[libcxx][libc] Hand in Hand PoC with from_chars #91651
Merged
+2,365
−21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for working on this! Let's see what CI has to say about its prospects, and then kick off a design conversation on Discourse? |
ldionne
reviewed
Jul 10, 2024
libcxx/test/std/utilities/charconv/charconv.from.chars/float.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/utilities/charconv/charconv.from.chars/float.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/src/CMakeLists.txt
Outdated
| @@ -197,7 +198,7 @@ split_list(LIBCXX_LINK_FLAGS) | |||
| # Build the shared library. | |||
| if (LIBCXX_ENABLE_SHARED) | |||
| add_library(cxx_shared SHARED ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS}) | |||
| target_include_directories(cxx_shared PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) | |||
| target_include_directories(cxx_shared PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/../../libc) #TODO: Do this properly | |||
There was a problem hiding this comment.
We could do something like this instead:
#
# runtimes/cmake/Modules/FindLibcUtils.cmake
#
# This file sets up an interface target called e.g. libc::shared-utilities that we can link against from any of the runtimes
# to get the functionality implemented in it. Concretely, we'd use it basically from libcxx/src/CMakeLists.txt as:
include(FindLibcUtils)
target_link_libraries(cxx_shared PRIVATE libc::shared-utilities) # This probably/hopefully only adds a `-I` flag.
# Possible implementation of this file:
add_library(libc::shared-utilities INTERFACE)
target_include_directories(libc::shared-utilities INTERFACE <path-to-shared-utilities>)
target_compile_features(libc::shared-utilities INTERFACE cxx_std_11) # for exampleThere was a problem hiding this comment.
I've created a file that does this, it's not the cleanest solution but it's the best I've got for the moment.
|
Thanks for the review @ldionne. I've just spoken with @michaelrj-google and I'll work on the libc++ part of this patch. |
michaelrj-google
force-pushed
the
libcHandInHandExample
branch
from
3267f34
to
ae9736e
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
michaelrj-google
commented
Aug 5, 2024
mordante
force-pushed
the
libcHandInHandExample
branch
from
83ed161
to
f2072fc
Compare
ldionne
requested changes
Aug 15, 2024
|
|
||
| const char* src = __ptr; // rename to match the libc code copied for this section. | ||
| ptrdiff_t length = __last - src; | ||
| _LIBCPP_ASSERT_INTERNAL(length > 0, ""); |
There was a problem hiding this comment.
Can we add a nicer error message?
There was a problem hiding this comment.
I've added an error message, though it may still need improvement.
| @@ -317,6 +317,8 @@ auto all_unsigned = type_list< | |||
| >(); | |||
| auto integrals = concat(all_signed, all_unsigned); | |||
|
|
|||
| auto all_floats = type_list< float, double >(); //TODO: Add long double | |||
There was a problem hiding this comment.
We should refactor this to use test/support/type_algorithms.h instead. Doesn't have to be in this patch.
ldionne
added
the
libc++
|
@llvm/pr-subscribers-libc @llvm/pr-subscribers-libcxx Author: Michael Jones (michaelrj-google) ChangesThis patch aims to demonstrate the utility of sharing code between libc Patch is 78.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91651.diff 24 Files Affected:
diff --git a/libc/shared/str_to_float.h b/libc/shared/str_to_float.h
new file mode 100644
index 00000000000000..da70db11f6b82b
--- /dev/null
+++ b/libc/shared/str_to_float.h
@@ -0,0 +1,29 @@
+//===-- String to float conversion utils ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SHARED_STR_TO_FLOAT_H
+#define LLVM_LIBC_SHARED_STR_TO_FLOAT_H
+
+#include "src/__support/str_to_float.h"
+
+namespace LIBC_NAMESPACE::shared {
+
+// WARNING: This is a proof of concept. In future the interface point for libcxx
+// won't be using libc internal classes.
+
+template <class T>
+inline internal::FloatConvertReturn<T> decimal_exp_to_float(
+ internal::ExpandedFloat<T> init_num, bool truncated,
+ internal::RoundDirection round, const char *__restrict num_start,
+ const size_t num_len = cpp::numeric_limits<size_t>::max()) {
+ return internal::decimal_exp_to_float(init_num, truncated, round, num_start,
+ num_len);
+}
+} // namespace LIBC_NAMESPACE::shared
+
+#endif // LLVM_LIBC_SHARED_STR_TO_FLOAT_H
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 32579272858a8e..43f9e5071f3b02 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -235,6 +235,7 @@ set(files
__bit/rotate.h
__bit_reference
__charconv/chars_format.h
+ __charconv/from_chars_floating_point.h
__charconv/from_chars_integral.h
__charconv/from_chars_result.h
__charconv/tables.h
diff --git a/libcxx/include/__charconv/from_chars_floating_point.h b/libcxx/include/__charconv/from_chars_floating_point.h
new file mode 100644
index 00000000000000..33ecf7012cfb6a
--- /dev/null
+++ b/libcxx/include/__charconv/from_chars_floating_point.h
@@ -0,0 +1,56 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___CHARCONV_FROM_CHARS_FLOATING_POINT_H
+#define _LIBCPP___CHARCONV_FROM_CHARS_FLOATING_POINT_H
+
+#include <__assert>
+#include <__charconv/chars_format.h>
+#include <__charconv/from_chars_result.h>
+#include <__charconv/traits.h>
+#include <__config>
+#include <__system_error/errc.h>
+#include <__type_traits/enable_if.h>
+#include <__type_traits/integral_constant.h>
+#include <__type_traits/is_floating_point.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER >= 17
+
+_LIBCPP_AVAILABILITY_FROM_CHARS_FLOATING_POINT _LIBCPP_EXPORTED_FROM_ABI from_chars_result
+__from_chars_floating_point(const char* __first, const char* __last, float& __value, chars_format __fmt);
+
+_LIBCPP_AVAILABILITY_FROM_CHARS_FLOATING_POINT _LIBCPP_EXPORTED_FROM_ABI from_chars_result
+__from_chars_floating_point(const char* __first, const char* __last, double& __value, chars_format __fmt);
+
+_LIBCPP_HIDE_FROM_ABI inline from_chars_result
+from_chars(const char* __first, const char* __last, float& __value, chars_format __fmt = chars_format::general) {
+ return std::__from_chars_floating_point(__first, __last, __value, __fmt);
+}
+
+_LIBCPP_HIDE_FROM_ABI inline from_chars_result
+from_chars(const char* __first, const char* __last, double& __value, chars_format __fmt = chars_format::general) {
+ return std::__from_chars_floating_point(__first, __last, __value, __fmt);
+}
+
+#endif // _LIBCPP_STD_VER >= 17
+
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif // _LIBCPP___CHARCONV_FROM_CHARS_FLOATING_POINT_H
diff --git a/libcxx/include/__configuration/availability.h b/libcxx/include/__configuration/availability.h
index ab483a07c9c137..edd9df87ebca99 100644
--- a/libcxx/include/__configuration/availability.h
+++ b/libcxx/include/__configuration/availability.h
@@ -87,6 +87,9 @@
// in all versions of the library are available.
#if defined(_LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS)
+# define _LIBCPP_INTRODUCED_IN_LLVM_20 1
+# define _LIBCPP_INTRODUCED_IN_LLVM_20_ATTRIBUTE /* nothing */
+
# define _LIBCPP_INTRODUCED_IN_LLVM_19 1
# define _LIBCPP_INTRODUCED_IN_LLVM_19_ATTRIBUTE /* nothing */
@@ -132,6 +135,11 @@
// clang-format off
+// LLVM 20
+// TODO: Fill this in
+# define _LIBCPP_INTRODUCED_IN_LLVM_20 0
+# define _LIBCPP_INTRODUCED_IN_LLVM_20_ATTRIBUTE __attribute__((unavailable))
+
// LLVM 19
// TODO: Fill this in
# define _LIBCPP_INTRODUCED_IN_LLVM_19 0
@@ -375,6 +383,11 @@
#define _LIBCPP_AVAILABILITY_HAS_BAD_EXPECTED_ACCESS_KEY_FUNCTION _LIBCPP_INTRODUCED_IN_LLVM_19
#define _LIBCPP_AVAILABILITY_BAD_EXPECTED_ACCESS_KEY_FUNCTION _LIBCPP_INTRODUCED_IN_LLVM_19_ATTRIBUTE
+// This controls the availability of floating-point std::from_chars functions.
+// These overloads were added later than the integer overloads.
+#define _LIBCPP_AVAILABILITY_HAS_FROM_CHARS_FLOATING_POINT _LIBCPP_INTRODUCED_IN_LLVM_20
+#define _LIBCPP_AVAILABILITY_FROM_CHARS_FLOATING_POINT _LIBCPP_INTRODUCED_IN_LLVM_20_ATTRIBUTE
+
// Define availability attributes that depend on _LIBCPP_HAS_NO_EXCEPTIONS.
// Those are defined in terms of the availability attributes above, and
// should not be vendor-specific.
diff --git a/libcxx/include/charconv b/libcxx/include/charconv
index a2e270e9316dc7..dfef8194a2a67b 100644
--- a/libcxx/include/charconv
+++ b/libcxx/include/charconv
@@ -65,6 +65,12 @@ namespace std {
constexpr from_chars_result from_chars(const char* first, const char* last,
see below& value, int base = 10); // constexpr since C++23
+ constexpr from_chars_result from_chars(const char* first, const char* last,
+ float& value, chars_format fmt);
+
+ constexpr from_chars_result from_chars(const char* first, const char* last,
+ double& value, chars_format fmt);
+
} // namespace std
*/
@@ -73,6 +79,7 @@ namespace std {
#if _LIBCPP_STD_VER >= 17
# include <__charconv/chars_format.h>
+# include <__charconv/from_chars_floating_point.h>
# include <__charconv/from_chars_integral.h>
# include <__charconv/from_chars_result.h>
# include <__charconv/tables.h>
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 13d0dce34d97e3..6c0ee59e2cc121 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -1075,18 +1075,19 @@ module std_private_bit_invert_if [system] { header "__bit/invert_if.h" }
module std_private_bit_popcount [system] { header "__bit/popcount.h" }
module std_private_bit_rotate [system] { header "__bit/rotate.h" }
-module std_private_charconv_chars_format [system] { header "__charconv/chars_format.h" }
-module std_private_charconv_from_chars_integral [system] { header "__charconv/from_chars_integral.h" }
-module std_private_charconv_from_chars_result [system] { header "__charconv/from_chars_result.h" }
-module std_private_charconv_tables [system] { header "__charconv/tables.h" }
-module std_private_charconv_to_chars [system] { header "__charconv/to_chars.h" }
-module std_private_charconv_to_chars_base_10 [system] { header "__charconv/to_chars_base_10.h" }
-module std_private_charconv_to_chars_floating_point [system] { header "__charconv/to_chars_floating_point.h" }
-module std_private_charconv_to_chars_integral [system] {
+module std_private_charconv_chars_format [system] { header "__charconv/chars_format.h" }
+module std_private_charconv_from_chars_integral [system] { header "__charconv/from_chars_integral.h" }
+module std_private_charconv_from_chars_result [system] { header "__charconv/from_chars_result.h" }
+module std_private_charconv_tables [system] { header "__charconv/tables.h" }
+module std_private_charconv_to_chars [system] { header "__charconv/to_chars.h" }
+module std_private_charconv_to_chars_base_10 [system] { header "__charconv/to_chars_base_10.h" }
+module std_private_charconv_to_chars_floating_point [system] { header "__charconv/to_chars_floating_point.h" }
+module std_private_charconv_from_chars_floating_point [system] { header "__charconv/from_chars_floating_point.h" }
+module std_private_charconv_to_chars_integral [system] {
header "__charconv/to_chars_integral.h"
export std_private_charconv_traits
}
-module std_private_charconv_to_chars_result [system] {
+module std_private_charconv_to_chars_result [system] {
header "__charconv/to_chars_result.h"
export *
}
diff --git a/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
index 917388f86811fe..7e227263ae10ae 100644
--- a/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1587,6 +1587,8 @@
{'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIxNS_22__cxx_atomic_base_implIxEEEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__131__arrive_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseEh', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__132__destroy_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__134__construct_barrier_algorithm_baseERl', 'type': 'FUNC'}
diff --git a/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
index 033d9f9987fa80..e0aca153ca0a2a 100644
--- a/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -572,6 +572,8 @@
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIiNS_22__cxx_atomic_base_implIiEEEE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKv', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
+{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
+{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__131__arrive_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseEh', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__132__destroy_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__134__construct_barrier_algorithm_baseERl', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
diff --git a/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
index 332d8abeb03e3a..d3c3012c443b0a 100644
--- a/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -572,6 +572,8 @@
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIlNS_22__cxx_atomic_base_implIlEEEE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKv', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
+{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
+{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__131__arrive_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseEh', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__132__destroy_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseE', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
{'import_export': 'EXP', 'is_defined': True, 'name': '_ZNSt3__134__construct_barrier_algorithm_baseERl', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
diff --git a/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
index defe235a283c21..a9dd2a5851f7f5 100644
--- a/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1587,6 +1587,8 @@
{'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIxNS_22__cxx_atomic_base_implIxEEEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__131__arrive_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseEh', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__132__destroy_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__134__construct_barrier_algorithm_baseERl', 'type': 'FUNC'}
diff --git a/libcxx/lib/abi/x86_64-linux-android21.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-linux-android21.libcxxabi.v1.stable.exceptions.nonew.abilist
index 6b77cda1e2866d..55c69232375b07 100644
--- a/libcxx/lib/abi/x86_64-linux-android21.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/x86_64-linux-android21.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1220,6 +1220,8 @@
{'is_defined': True, 'name': '_ZNSt6__ndk123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIiNS_22__cxx_atomic_base_implIiEEEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt6__ndk123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt6__ndk125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt6__ndk127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt6__ndk127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt6__ndk131__arrive_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseEh', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt6__ndk132__destroy_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt6__ndk134__construct_barrier_algorithm_baseERl', 'type': 'FUNC'}
diff --git a/libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.nonew.abilist
index 3458b333dd6a9b..356440b6cfd152 100644
--- a/libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1235,6 +1235,8 @@
{'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIlNS_22__cxx_atomic_base_implIlEEEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__131__arrive_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseEh', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__132__destroy_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__134__construct_barrier_algorithm_baseERl', 'type': 'FUNC'}
diff --git a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist
index bdf90ba25c7fd9..0fc3be345fa8d3 100644
--- a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1233,6 +1233,8 @@
{'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIiNS_22__cxx_atomic_base_implIiEEEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt3__127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__131__arrive_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseEh', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__132__destroy_barrier_algorithm_baseEPNS_24__barrier_algorithm_baseE', 'type': 'FUNC'}
{'is_defined': True, 'name': '_ZNSt3__134__construct_barrier_algorithm_baseERl', 'type': 'FUNC'}
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index fe9d2666fa4caa..6183f42252d82b 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -31,6 +31,7 @@ set(LIBCXX_SOURCES
include/ryu/f2s.h
include/ryu/ryu.h
include/to_chars_floating_point.h
+ include/from_chars_floating_point.h
legacy_pointer_safety.cpp
memory.cpp
memory_resource.cpp
@@ -176,11 +177,13 @@ endif()
split_list(LIBCXX_COMPILE_FLAGS)
split_list(LIBCXX_LINK_FLAGS)
+include(FindLibcUtils)
+
# Build the shared library.
if (LIBCXX_ENABLE_SHARED)
add_library(cxx_shared SHARED ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
target_include_directories(cxx_shared PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
- target_link_libraries(cxx_shared PUBLIC cxx-headers libcxx-...
[truncated]
|
mordante
reviewed
Aug 18, 2024
| @@ -121,6 +121,7 @@ inline constexpr FloatFromCharsTestCase float_from_chars_test_cases[] = { | |||
| "2222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222" | |||
| "2222222222222222222e-45", | |||
| chars_format::scientific, 1006, errc{}, 0x0.000004p-126f}, | |||
|
|
|||
There was a problem hiding this comment.
FYI we should not edit this file unless strictly required. This file is copied from the MSVC STL implementation. Avoiding unrelated changes makes it easier to keep the files in sync.
mordante
reviewed
Aug 18, 2024
ldionne
reviewed
Aug 20, 2024
michaelrj-google
commented
Sep 3, 2024
| if (__offset + 1 < __n && // an exponent always needs at least one digit. | ||
| std::tolower(__first[__offset]) == 'p') { | ||
| ++__offset; // assumes a valid exponent. | ||
| LIBC_NAMESPACE::shared::StrToNumResult<int32_t> __e = |
There was a problem hiding this comment.
nit:
Suggested change
| LIBC_NAMESPACE::shared::StrToNumResult<int32_t> __e = | |
| auto __e = |
Comment on lines
365
to
375
| int64_t temp_exponent = static_cast<int64_t>(exponent) + static_cast<int64_t>(add_to_exponent); | ||
|
|
||
| // If the result is in the valid range, then we use it. The valid range is | ||
| // also within the int32 range, so this prevents overflow issues. | ||
| if (temp_exponent > LIBC_NAMESPACE::shared::FPBits<_Fp>::MAX_BIASED_EXPONENT) { | ||
| exponent = LIBC_NAMESPACE::shared::FPBits<_Fp>::MAX_BIASED_EXPONENT; | ||
| } else if (temp_exponent < -LIBC_NAMESPACE::shared::FPBits<_Fp>::MAX_BIASED_EXPONENT) { | ||
| exponent = -LIBC_NAMESPACE::shared::FPBits<_Fp>::MAX_BIASED_EXPONENT; | ||
| } else { | ||
| exponent = static_cast<int32_t>(temp_exponent); | ||
| } |
There was a problem hiding this comment.
This should probably use __merge_exponents
philnik777
reviewed
Sep 11, 2024
h-vetinari
reviewed
Sep 14, 2024
libcxx/docs/Status/Cxx17.rst
Outdated
| @@ -40,7 +40,7 @@ Paper Status | |||
|
|
|||
| .. note:: | |||
|
|
|||
| .. [#note-P0067] P0067: ``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``. | |||
| .. [#note-P0067] P0067: ``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``. ``std::from_chars`` for ``float`` and ``double`` since version 20.0. | |||
There was a problem hiding this comment.
I think this comment would benefit a rewrite, to note what is actually still open. E.g.
Suggested change
| .. [#note-P0067] P0067: ``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``. ``std::from_chars`` for ``float`` and ``double`` since version 20.0. | |
| .. [#note-P0067] P0067: for integer types, ``std::(to|from)_chars`` has been available since version 7.0; for ``float`` and ``double``, ``std::to_chars`` since version 14.0 and ``std::from_chars`` since version 20.1. Support is complete except for ``long double``, which currently uses the implementation for ``double``. |
There was a problem hiding this comment.
I left a comment before, but forgot to publish it :-/
to_chars has float, double, and long double, where long double is double
from_chars only has float and double support, and no long double support at all.
(Adding proper long double support is on my todo list and using LLVM-libc that will be easier, however a lot of work will be to write proper tests for long double. So I feel your suggestion is not correct.
Feel free to propose different wording if you have a better suggestion.
ldionne
reviewed
Sep 25, 2024
| @@ -0,0 +1,1557 @@ | |||
| //===----------------------------------------------------------------------===// | |||
There was a problem hiding this comment.
Let's rename this test to floating_point.pass.cpp
libcxx/test/std/utilities/charconv/charconv.from.chars/float.pass.cpp
Outdated
Show resolved
Hide resolved
mordante
force-pushed
the
libcHandInHandExample
branch
from
fe23413
to
fbe296e
Compare
ldionne
reviewed
Oct 15, 2024
libcxx/docs/Status/Cxx2cIssues.csv
Outdated
| @@ -78,4 +78,5 @@ | |||
| "","","","","","" | |||
| "`LWG3343 <https://wg21.link/LWG3343>`__","Ordering of calls to ``unlock()`` and ``notify_all()`` in Effects element of ``notify_all_at_thread_exit()`` should be reversed","Not Adopted Yet","|Complete|","16.0","" | |||
| "`LWG4139 <https://wg21.link/LWG4139>`__","§[time.zone.leap] recursive constraint in <=>","Not Adopted Yet","|Complete|","20.0","" | |||
| "`LWG3456 <https://wg21.link/LWG3343>`__","Pattern used by std::from_chars is underspecified (option B)",,"Not Yet Adopted","|Complete|","20.0","" | |||
There was a problem hiding this comment.
This looks like the wrong LWG issue number.
There was a problem hiding this comment.
Actually this is a copy-paste error; the number is correct, the link is not updated.
libcxx/lib/abi/CHANGELOG.TXT
Outdated
Comment on lines
23
to
24
| Symbol added: _ZNSt6__ndk127__from_chars_floating_pointEPKcS1_RdNS_12chars_formatE | ||
| Symbol added: _ZNSt6__ndk127__from_chars_floating_pointEPKcS1_RfNS_12chars_formatE |
There was a problem hiding this comment.
These symbols seem a bit wrong: it looks like they are coming from the Android CI jobs, is that possible? Android uses a special namespace instead of __1, so it would be better to take the name generated from another Linux job since that represents better the name of the symbols being added.
libcxx/test/std/utilities/charconv/charconv.from.chars/floatint_point.pass.cpp
Outdated
Show resolved
Hide resolved
h-vetinari
reviewed
Oct 17, 2024
| @@ -71,7 +71,7 @@ | |||
| "`P0394R4 <https://wg21.link/P0394R4>`__","Hotel Parallelifornia: terminate() for Parallel Algorithms Exception Handling","2016-06 (Oulu)","|Complete|","17.0","" | |||
| "","","","","","" | |||
| "`P0003R5 <https://wg21.link/P0003R5>`__","Removing Deprecated Exception Specifications from C++17","2016-11 (Issaquah)","|Complete|","5.0","" | |||
| "`P0067R5 <https://wg21.link/P0067R5>`__","Elementary string conversions, revision 5","2016-11 (Issaquah)","|Partial|","","``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``." | |||
| "`P0067R5 <https://wg21.link/P0067R5>`__","Elementary string conversions, revision 5","2016-11 (Issaquah)","|Partial|","","``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``. ``std::from_chars`` for ``float`` and ``double`` since version 20.0." | |||
There was a problem hiding this comment.
Reattaching my suggestion (clarify what's done resp. still open) after the table format changed to include notes.
Suggested change
| "`P0067R5 <https://wg21.link/P0067R5>`__","Elementary string conversions, revision 5","2016-11 (Issaquah)","|Partial|","","``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``. ``std::from_chars`` for ``float`` and ``double`` since version 20.0." | |
| "`P0067R5 <https://wg21.link/P0067R5>`__","Elementary string conversions, revision 5","2016-11 (Issaquah)","|Partial|","","For integer types, ``std::(to|from)_chars`` has been available since version 7.0; for ``float`` and ``double``, ``std::to_chars`` since version 14.0 and ``std::from_chars`` since version 20.1. Support is complete except for ``long double``, which currently uses the implementation for ``double``. |
There was a problem hiding this comment.
I typed a comment before, but it seems I forgot to publish it :-/
The suggestion is not correct
14.0 has from_chars, where long double uses the double implementation
20.0 has to_chars and no long double support at all.
I'm open to suggestions to improve the wording, feel free to prove and updated wording.
There was a problem hiding this comment.
OK, thanks for the clarification - just to double-check though:
20.0 has
to_charsand nolong doublesupport at all.
This PR is adding from_chars, right (e.g. TEST_HAS_FROM_CHARS_FLOATING_POINT)?
mordante
force-pushed
the
libcHandInHandExample
branch
from
6f84901
to
01d5f45
Compare
ldionne
approved these changes
Oct 17, 2024
There was a problem hiding this comment.
LGTM with a few comments about the ABI boundary. I'm not overly attached to those, but I do think that avoiding the external template instantiation declaration and the explicit template instantiation in the .cpp file results in simpler code for the reader -- it's not always obvious how that stuff interacts with attributes.
@h-vetinari 's comment on the status page should also be addressed.
Thanks a lot to everyone involved in this patch! It's exciting to see this kind of collaboration between different subprojects.
|
Everything LGTM from the libc side. I will wait to merge until @mordante is done with his edits and approves. Once that's done I will try to merge either this friday morning or monday morning. |
mordante
force-pushed
the
libcHandInHandExample
branch
from
01d5f45
to
71b488a
Compare
philnik777
reviewed
Oct 20, 2024
Comment on lines
38
to
40
| template <class _Fp> | ||
| _LIBCPP_EXPORTED_FROM_ABI [[gnu::pure]] __from_chars_result<_Fp> __from_chars_floating_point( | ||
| [[clang::noescape]] const char* __first, [[clang::noescape]] const char* __last, chars_format __fmt); |
There was a problem hiding this comment.
Suggested change
| template <class _Fp> | |
| _LIBCPP_EXPORTED_FROM_ABI [[gnu::pure]] __from_chars_result<_Fp> __from_chars_floating_point( | |
| [[clang::noescape]] const char* __first, [[clang::noescape]] const char* __last, chars_format __fmt); | |
| template <class _Fp> | |
| _LIBCPP_EXPORTED_FROM_ABI [[__gnu__::__pure__]] __from_chars_result<_Fp> __from_chars_floating_point( | |
| [[_Clang::__noescape__]] const char* __first, [[clang::noescape]] const char* __last, chars_format __fmt); |
Also, [[clang::noescape]] should be behind a macro, since GCC doesn't support it.
@ldionne What's blocking re-enabling clang tidy? This would have been caught.
There was a problem hiding this comment.
I discussed using macros with Louis. He'll work on a patch to document how we want to use exported dylib functions; that patch should introduce that macro.
There was a problem hiding this comment.
For now I removed the pure, some compilers seem not to like it. I'll investigate this later.
mordante
force-pushed
the
libcHandInHandExample
branch
3 times, most recently
from
d79aabb
to
88ea875
Compare
ldionne
approved these changes
Oct 21, 2024
mordante
force-pushed
the
libcHandInHandExample
branch
from
88ea875
to
63884e3
Compare
Implements std::from_chars for float and double. The implementation uses LLVM-libc to do the real parsing. Since this is the first time libc++ uses LLVM-libc there is a bit of additional infrastructure code. The patch is based on the [RFC] Project Hand In Hand (LLVM-libc/libc++ code sharing) https://discourse.llvm.org/t/rfc-project-hand-in-hand-llvm-libc-libc-code-sharing/77701
michaelrj-google
force-pushed
the
libcHandInHandExample
branch
from
63884e3
to
967de3d
Compare
|
The force push was only to update the commit message. CI was green, so this should be safe to merge. Massive thanks to @ldionne and @mordante for pushing this along from the libc++ side, I really appreciate the work you did to make this project a success. This truly wouldn't have been possible without you. Thanks as well to everyone else who contributed to this patch to make it the best it can be. I truly hope this is the first step of many. 🤝 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/7370 Here is the relevant piece of the build log for the reference |
This buildbot is flakey, and the failure seems to be unrelated. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/4525 Here is the relevant piece of the build log for the reference |
michaelrj-google
added a commit
to michaelrj-google/llvm-project
that referenced
this pull request
Oct 22, 2024
Turns out for double double LDBL_MANT_DIG == 106. This patch fixes the constant. Should fix the ppc buildbot. Previously: llvm#113235 llvm#113237 llvm#91651
michaelrj-google
added a commit
that referenced
this pull request
Oct 22, 2024
EricWF
pushed a commit
to efcs/llvm-project
that referenced
this pull request
Oct 22, 2024
Implements std::from_chars for float and double. The implementation uses LLVM-libc to do the real parsing. Since this is the first time libc++ uses LLVM-libc there is a bit of additional infrastructure code. The patch is based on the [RFC] Project Hand In Hand (LLVM-libc/libc++ code sharing) https://discourse.llvm.org/t/rfc-project-hand-in-hand-llvm-libc-libc-code-sharing/77701
EricWF
pushed a commit
to efcs/llvm-project
that referenced
this pull request
Oct 22, 2024
Turns out for double double LDBL_MANT_DIG == 106. This patch fixes the constant. Should fix the ppc buildbot. Previously: llvm#113235 llvm#113237 llvm#91651
nico
added a commit
that referenced
this pull request
Oct 24, 2024
It's currently broken by #91651. Stop buildling libcxx src files until that is sorted out.
frederick-vs-ja
added a commit
that referenced
this pull request
Oct 25, 2024
Identifiers `clang` and `noescape` are not reserved by the C++ standard, so perhaps we need to use the equivalent reserved forms. Also changes the occurrences of that attribute to a macro, following the convention for `[[_Clang::__lifetimebound__]]`. Addresses #91651 (comment).
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements std::from_chars for float and double.
The implementation uses LLVM-libc to do the real parsing. Since this is the first time libc++
uses LLVM-libc there is a bit of additional infrastructure code. The patch is based on the
[RFC] Project Hand In Hand (LLVM-libc/libc++ code sharing)
https://discourse.llvm.org/t/rfc-project-hand-in-hand-llvm-libc-libc-code-sharing/77701