Revert "[libc++] Optimize num_put integral functions" #131613

alexfh · 2025-03-17T13:41:48Z

Reverts #120859

This change breaks formatting of 0 with std::showbase + std::hex or std::oct, as well as +0 with std::showpos. I believe the new behavior is violating the standard. See #120859 (comment) and later comments for details and explanation.

This reverts commit 15edf87.

llvmbot · 2025-03-17T13:42:22Z

@llvm/pr-subscribers-libcxx

Author: Alexander Kornienko (alexfh)

Changes

Reverts llvm/llvm-project#120859

This change breaks formatting of 0 with std::showbase + std::hex or std::oct, as well as +0 with std::showpos. I believe the new behavior is violating the standard. See #120859 (comment) and later comments for details and explanation.

Patch is 34.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/131613.diff

13 Files Affected:

(modified) libcxx/docs/ReleaseNotes/20.rst (-2)
(modified) libcxx/include/__charconv/tables.h (+12-8)
(modified) libcxx/include/__charconv/to_chars_base_10.h (+18-14)
(modified) libcxx/include/__charconv/to_chars_integral.h (+27-45)
(modified) libcxx/include/__charconv/to_chars_result.h (-9)
(modified) libcxx/include/__charconv/traits.h (+7-3)
(modified) libcxx/include/__format/formatter_floating_point.h (-1)
(modified) libcxx/include/__format/formatter_integral.h (+1-1)
(modified) libcxx/include/__format/formatter_output.h (+18)
(modified) libcxx/include/locale (+39-65)
(modified) libcxx/include/module.modulemap (+1-4)
(removed) libcxx/test/benchmarks/locale/num_put.bench.cpp (-39)
(modified) libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_pointer.pass.cpp (+26-18)

diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
index c7b59545b4fbc..57ab0c167544b 100644
--- a/libcxx/docs/ReleaseNotes/20.rst
+++ b/libcxx/docs/ReleaseNotes/20.rst
@@ -120,8 +120,6 @@ Improvements and New Features
 
 - Added :ref:`hardening mode <hardening>` support for ``forward_list`` and ``bitset``.
 
-- The ``num_get::do_put`` integral overloads have been optimized, resulting in a performance improvement of up to 2.4x.
-
 Deprecations and Removals
 -------------------------
 
diff --git a/libcxx/include/__charconv/tables.h b/libcxx/include/__charconv/tables.h
index b8c6fd8af0a0f..9568bf841cd02 100644
--- a/libcxx/include/__charconv/tables.h
+++ b/libcxx/include/__charconv/tables.h
@@ -19,14 +19,16 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+#if _LIBCPP_STD_VER >= 17
+
 namespace __itoa {
 
-inline _LIBCPP_CONSTEXPR const char __base_2_lut[64] = {
+inline constexpr char __base_2_lut[64] = {
     '0', '0', '0', '0', '0', '0', '0', '1', '0', '0', '1', '0', '0', '0', '1', '1', '0', '1', '0', '0', '0', '1',
     '0', '1', '0', '1', '1', '0', '0', '1', '1', '1', '1', '0', '0', '0', '1', '0', '0', '1', '1', '0', '1', '0',
     '1', '0', '1', '1', '1', '1', '0', '0', '1', '1', '0', '1', '1', '1', '1', '0', '1', '1', '1', '1'};
 
-inline _LIBCPP_CONSTEXPR const char __base_8_lut[128] = {
+inline constexpr char __base_8_lut[128] = {
     '0', '0', '0', '1', '0', '2', '0', '3', '0', '4', '0', '5', '0', '6', '0', '7', '1', '0', '1', '1', '1', '2',
     '1', '3', '1', '4', '1', '5', '1', '6', '1', '7', '2', '0', '2', '1', '2', '2', '2', '3', '2', '4', '2', '5',
     '2', '6', '2', '7', '3', '0', '3', '1', '3', '2', '3', '3', '3', '4', '3', '5', '3', '6', '3', '7', '4', '0',
@@ -34,7 +36,7 @@ inline _LIBCPP_CONSTEXPR const char __base_8_lut[128] = {
     '5', '4', '5', '5', '5', '6', '5', '7', '6', '0', '6', '1', '6', '2', '6', '3', '6', '4', '6', '5', '6', '6',
     '6', '7', '7', '0', '7', '1', '7', '2', '7', '3', '7', '4', '7', '5', '7', '6', '7', '7'};
 
-inline _LIBCPP_CONSTEXPR const char __base_16_lut[512] = {
+inline constexpr char __base_16_lut[512] = {
     '0', '0', '0', '1', '0', '2', '0', '3', '0', '4', '0', '5', '0', '6', '0', '7', '0', '8', '0', '9', '0', 'a', '0',
     'b', '0', 'c', '0', 'd', '0', 'e', '0', 'f', '1', '0', '1', '1', '1', '2', '1', '3', '1', '4', '1', '5', '1', '6',
     '1', '7', '1', '8', '1', '9', '1', 'a', '1', 'b', '1', 'c', '1', 'd', '1', 'e', '1', 'f', '2', '0', '2', '1', '2',
@@ -59,7 +61,7 @@ inline _LIBCPP_CONSTEXPR const char __base_16_lut[512] = {
     '1', 'f', '2', 'f', '3', 'f', '4', 'f', '5', 'f', '6', 'f', '7', 'f', '8', 'f', '9', 'f', 'a', 'f', 'b', 'f', 'c',
     'f', 'd', 'f', 'e', 'f', 'f'};
 
-inline _LIBCPP_CONSTEXPR const uint32_t __pow10_32[10] = {
+inline constexpr uint32_t __pow10_32[10] = {
     UINT32_C(0),
     UINT32_C(10),
     UINT32_C(100),
@@ -71,7 +73,7 @@ inline _LIBCPP_CONSTEXPR const uint32_t __pow10_32[10] = {
     UINT32_C(100000000),
     UINT32_C(1000000000)};
 
-inline _LIBCPP_CONSTEXPR const uint64_t __pow10_64[20] = {
+inline constexpr uint64_t __pow10_64[20] = {
     UINT64_C(0),
     UINT64_C(10),
     UINT64_C(100),
@@ -94,8 +96,8 @@ inline _LIBCPP_CONSTEXPR const uint64_t __pow10_64[20] = {
     UINT64_C(10000000000000000000)};
 
 #  if _LIBCPP_HAS_INT128
-inline _LIBCPP_CONSTEXPR const int __pow10_128_offset      = 0;
-inline _LIBCPP_CONSTEXPR const __uint128_t __pow10_128[40] = {
+inline constexpr int __pow10_128_offset      = 0;
+inline constexpr __uint128_t __pow10_128[40] = {
     UINT64_C(0),
     UINT64_C(10),
     UINT64_C(100),
@@ -138,7 +140,7 @@ inline _LIBCPP_CONSTEXPR const __uint128_t __pow10_128[40] = {
     (__uint128_t(UINT64_C(10000000000000000000)) * UINT64_C(10000000000000000000)) * 10};
 #  endif
 
-inline _LIBCPP_CONSTEXPR const char __digits_base_10[200] = {
+inline constexpr char __digits_base_10[200] = {
     // clang-format off
     '0', '0', '0', '1', '0', '2', '0', '3', '0', '4', '0', '5', '0', '6', '0', '7', '0', '8', '0', '9',
     '1', '0', '1', '1', '1', '2', '1', '3', '1', '4', '1', '5', '1', '6', '1', '7', '1', '8', '1', '9',
@@ -154,6 +156,8 @@ inline _LIBCPP_CONSTEXPR const char __digits_base_10[200] = {
 
 } // namespace __itoa
 
+#endif // _LIBCPP_STD_VER >= 17
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___CHARCONV_TABLES
diff --git a/libcxx/include/__charconv/to_chars_base_10.h b/libcxx/include/__charconv/to_chars_base_10.h
index d90952ea71f35..06e4e692337df 100644
--- a/libcxx/include/__charconv/to_chars_base_10.h
+++ b/libcxx/include/__charconv/to_chars_base_10.h
@@ -26,53 +26,55 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+#if _LIBCPP_STD_VER >= 17
+
 namespace __itoa {
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append1(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append1(char* __first, uint32_t __value) noexcept {
   *__first = '0' + static_cast<char>(__value);
   return __first + 1;
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append2(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append2(char* __first, uint32_t __value) noexcept {
   return std::copy_n(&__digits_base_10[__value * 2], 2, __first);
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append3(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append3(char* __first, uint32_t __value) noexcept {
   return __itoa::__append2(__itoa::__append1(__first, __value / 100), __value % 100);
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append4(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append4(char* __first, uint32_t __value) noexcept {
   return __itoa::__append2(__itoa::__append2(__first, __value / 100), __value % 100);
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append5(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append5(char* __first, uint32_t __value) noexcept {
   return __itoa::__append4(__itoa::__append1(__first, __value / 10000), __value % 10000);
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append6(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append6(char* __first, uint32_t __value) noexcept {
   return __itoa::__append4(__itoa::__append2(__first, __value / 10000), __value % 10000);
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append7(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append7(char* __first, uint32_t __value) noexcept {
   return __itoa::__append6(__itoa::__append1(__first, __value / 1000000), __value % 1000000);
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append8(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append8(char* __first, uint32_t __value) noexcept {
   return __itoa::__append6(__itoa::__append2(__first, __value / 1000000), __value % 1000000);
 }
 
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append9(char* __first, uint32_t __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char* __append9(char* __first, uint32_t __value) noexcept {
   return __itoa::__append8(__itoa::__append1(__first, __value / 100000000), __value % 100000000);
 }
 
 template <class _Tp>
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI char* __append10(char* __first, _Tp __value) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI char* __append10(char* __first, _Tp __value) noexcept {
   return __itoa::__append8(__itoa::__append2(__first, static_cast<uint32_t>(__value / 100000000)),
                            static_cast<uint32_t>(__value % 100000000));
 }
 
 _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char*
-__base_10_u32(char* __first, uint32_t __value) _NOEXCEPT {
+__base_10_u32(char* __first, uint32_t __value) noexcept {
   if (__value < 1000000) {
     if (__value < 10000) {
       if (__value < 100) {
@@ -108,7 +110,7 @@ __base_10_u32(char* __first, uint32_t __value) _NOEXCEPT {
 }
 
 _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char*
-__base_10_u64(char* __buffer, uint64_t __value) _NOEXCEPT {
+__base_10_u64(char* __buffer, uint64_t __value) noexcept {
   if (__value <= UINT32_MAX)
     return __itoa::__base_10_u32(__buffer, static_cast<uint32_t>(__value));
 
@@ -130,13 +132,13 @@ __base_10_u64(char* __buffer, uint64_t __value) _NOEXCEPT {
 /// \note The lookup table contains a partial set of exponents limiting the
 /// range that can be used. However the range is sufficient for
 /// \ref __base_10_u128.
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline __uint128_t __pow_10(int __exp) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline __uint128_t __pow_10(int __exp) noexcept {
   _LIBCPP_ASSERT_INTERNAL(__exp >= __pow10_128_offset, "Index out of bounds");
   return __pow10_128[__exp - __pow10_128_offset];
 }
 
 _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI inline char*
-__base_10_u128(char* __buffer, __uint128_t __value) _NOEXCEPT {
+__base_10_u128(char* __buffer, __uint128_t __value) noexcept {
   _LIBCPP_ASSERT_INTERNAL(
       __value > numeric_limits<uint64_t>::max(), "The optimizations for this algorithm fails when this isn't true.");
 
@@ -177,6 +179,8 @@ __base_10_u128(char* __buffer, __uint128_t __value) _NOEXCEPT {
 #  endif
 } // namespace __itoa
 
+#endif // _LIBCPP_STD_VER >= 17
+
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
diff --git a/libcxx/include/__charconv/to_chars_integral.h b/libcxx/include/__charconv/to_chars_integral.h
index 238c96d7c7a04..710299df9b4da 100644
--- a/libcxx/include/__charconv/to_chars_integral.h
+++ b/libcxx/include/__charconv/to_chars_integral.h
@@ -39,12 +39,16 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+#if _LIBCPP_STD_VER >= 17
+
+to_chars_result to_chars(char*, char*, bool, int = 10) = delete;
+
 template <typename _Tp>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
 __to_chars_itoa(char* __first, char* __last, _Tp __value, false_type);
 
 template <typename _Tp>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
 __to_chars_itoa(char* __first, char* __last, _Tp __value, true_type) {
   auto __x = std::__to_unsigned_like(__value);
   if (__value < 0 && __first != __last) {
@@ -56,7 +60,7 @@ __to_chars_itoa(char* __first, char* __last, _Tp __value, true_type) {
 }
 
 template <typename _Tp>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
 __to_chars_itoa(char* __first, char* __last, _Tp __value, false_type) {
   using __tx  = __itoa::__traits<_Tp>;
   auto __diff = __last - __first;
@@ -69,7 +73,7 @@ __to_chars_itoa(char* __first, char* __last, _Tp __value, false_type) {
 
 #  if _LIBCPP_HAS_INT128
 template <>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
 __to_chars_itoa(char* __first, char* __last, __uint128_t __value, false_type) {
   // When the value fits in 64-bits use the 64-bit code path. This reduces
   // the number of expensive calculations on 128-bit values.
@@ -88,20 +92,20 @@ __to_chars_itoa(char* __first, char* __last, __uint128_t __value, false_type) {
 }
 #  endif
 
-template <class _Tp, __enable_if_t<!is_signed<_Tp>::value, int> = 0>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
-__to_chars_integral(char* __first, char* __last, _Tp __value, int __base);
+template <class _Tp>
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
+__to_chars_integral(char* __first, char* __last, _Tp __value, int __base, false_type);
 
-template <class _Tp, __enable_if_t<is_signed<_Tp>::value, int> = 0>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
-__to_chars_integral(char* __first, char* __last, _Tp __value, int __base) {
+template <typename _Tp>
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
+__to_chars_integral(char* __first, char* __last, _Tp __value, int __base, true_type) {
   auto __x = std::__to_unsigned_like(__value);
   if (__value < 0 && __first != __last) {
     *__first++ = '-';
     __x        = std::__complement(__x);
   }
 
-  return std::__to_chars_integral(__first, __last, __x, __base);
+  return std::__to_chars_integral(__first, __last, __x, __base, false_type());
 }
 
 namespace __itoa {
@@ -112,7 +116,7 @@ struct _LIBCPP_HIDDEN __integral;
 template <>
 struct _LIBCPP_HIDDEN __integral<2> {
   template <typename _Tp>
-  _LIBCPP_HIDE_FROM_ABI static _LIBCPP_CONSTEXPR int __width(_Tp __value) _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI static constexpr int __width(_Tp __value) noexcept {
     // If value == 0 still need one digit. If the value != this has no
     // effect since the code scans for the most significant bit set. (Note
     // that __libcpp_clz doesn't work for 0.)
@@ -120,7 +124,7 @@ struct _LIBCPP_HIDDEN __integral<2> {
   }
 
   template <typename _Tp>
-  _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI static __to_chars_result
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI static to_chars_result
   __to_chars(char* __first, char* __last, _Tp __value) {
     ptrdiff_t __cap = __last - __first;
     int __n         = __width(__value);
@@ -148,7 +152,7 @@ struct _LIBCPP_HIDDEN __integral<2> {
 template <>
 struct _LIBCPP_HIDDEN __integral<8> {
   template <typename _Tp>
-  _LIBCPP_HIDE_FROM_ABI static _LIBCPP_CONSTEXPR int __width(_Tp __value) _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI static constexpr int __width(_Tp __value) noexcept {
     // If value == 0 still need one digit. If the value != this has no
     // effect since the code scans for the most significat bit set. (Note
     // that __libcpp_clz doesn't work for 0.)
@@ -156,7 +160,7 @@ struct _LIBCPP_HIDDEN __integral<8> {
   }
 
   template <typename _Tp>
-  _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI static __to_chars_result
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI static to_chars_result
   __to_chars(char* __first, char* __last, _Tp __value) {
     ptrdiff_t __cap = __last - __first;
     int __n         = __width(__value);
@@ -184,7 +188,7 @@ struct _LIBCPP_HIDDEN __integral<8> {
 template <>
 struct _LIBCPP_HIDDEN __integral<16> {
   template <typename _Tp>
-  _LIBCPP_HIDE_FROM_ABI static _LIBCPP_CONSTEXPR int __width(_Tp __value) _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI static constexpr int __width(_Tp __value) noexcept {
     // If value == 0 still need one digit. If the value != this has no
     // effect since the code scans for the most significat bit set. (Note
     // that __libcpp_clz doesn't work for 0.)
@@ -192,7 +196,7 @@ struct _LIBCPP_HIDDEN __integral<16> {
   }
 
   template <typename _Tp>
-  _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI static __to_chars_result
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI static to_chars_result
   __to_chars(char* __first, char* __last, _Tp __value) {
     ptrdiff_t __cap = __last - __first;
     int __n         = __width(__value);
@@ -231,13 +235,13 @@ _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI int __to_chars_integral_widt
 }
 
 template <unsigned _Base, typename _Tp, __enable_if_t<(sizeof(_Tp) >= sizeof(unsigned)), int> = 0>
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
 __to_chars_integral(char* __first, char* __last, _Tp __value) {
   return __itoa::__integral<_Base>::__to_chars(__first, __last, __value);
 }
 
 template <unsigned _Base, typename _Tp, __enable_if_t<(sizeof(_Tp) < sizeof(unsigned)), int> = 0>
-_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
+_LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
 __to_chars_integral(char* __first, char* __last, _Tp __value) {
   return std::__to_chars_integral<_Base>(__first, __last, static_cast<unsigned>(__value));
 }
@@ -268,9 +272,9 @@ _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI int __to_chars_integral_widt
   __libcpp_unreachable();
 }
 
-template <class _Tp, __enable_if_t<!is_signed<_Tp>::value, int> >
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI __to_chars_result
-__to_chars_integral(char* __first, char* __last, _Tp __value, int __base) {
+template <typename _Tp>
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
+__to_chars_integral(char* __first, char* __last, _Tp __value, int __base, false_type) {
   if (__base == 10) [[likely]]
     return std::__to_chars_itoa(__first, __last, __value, false_type());
 
@@ -298,28 +302,6 @@ __to_chars_integral(char* __first, char* __last, _Tp __value, int __base) {
   return {__last, errc(0)};
 }
 
-_LIBCPP_HIDE_FROM_ABI inline _LIBCPP_CONSTEXPR_SINCE_CXX14 char __hex_to_upper(char __c) {
-  switch (__c) {
-  case 'a':
-    return 'A';
-  case 'b':
-    return 'B';
-  case 'c':
-    return 'C';
-  case 'd':
-    return 'D';
-  case 'e':
-    return 'E';
-  case 'f':
-    return 'F';
-  }
-  return __c;
-}
-
-#if _LIBCPP_STD_VER >= 17
-
-to_chars_result to_chars(char*, char*, bool, int = 10) = delete;
-
 template <typename _Tp, __enable_if_t<is_integral<_Tp>::value, int> = 0>
 inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
 to_chars(char* __first, char* __last, _Tp __value) {
@@ -334,7 +316,7 @@ to_chars(char* __first, char* __last, _Tp __value, int __base) {
   _LIBCPP_ASSERT_UNCATEGORIZED(2 <= __base && __base <= 36, "base not in [2, 36]");
 
   using _Type = __make_32_64_or_128_bit_t<_Tp>;
-  return std::__to_chars_integral(__first, __last, static_cast<_Type>(__value), __base);
+  return std::__to_chars_integral(__first, __last, static_cast<_Type>(__value), __base, is_signed<_Tp>());
 }
 
 #endif // _LIBCPP_STD_VER >= 17
diff --git a/libcxx/include/__charconv/to_chars_result.h b/libcxx/include/__charconv/to_chars_result.h
index 41dea4ab14723..8df0897a49fbb 100644
--- a/libcxx/include/__charconv/to_chars_result.h
+++ b/libcxx/include/__charconv/to_chars_result.h
@@ -34,15 +34,6 @@ struct _LIBCPP_EXPORTED_FROM_ABI to_chars_result {
 
 #endif // _LIBCPP_STD_VER >= 17
 
-struct __to_chars_result {
-  char* __ptr;
-  errc __ec;
-
-#if _LIBCPP_STD_VER >= 17
-  _LIBCPP_HIDE_FROM_ABI constexpr operator to_chars_result() { return {__ptr, __ec}; }
-#endif
-};
-
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___CHARCONV_TO_CHARS_RESULT_H
diff --git a/libcxx/include/__charconv/traits.h b/libcxx/include/__charconv/traits.h
index 085a3f7886f31..dd1fa2354436a 100644
--- a/libcxx/include/__charconv/traits.h
+++ b/libcxx/include/__charconv/traits.h
@@ -30,13 +30,15 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+#if _LIBCPP_STD_VER >= 17
+
 namespace __itoa {
 
 template <typename _Tp, typename = void>
 struct _LIBCPP_HIDDEN __traits_base;
 
 template <typename _Tp>
-struct _LIBCPP_HIDDEN __traits_base<_Tp, __enable_if_t<sizeof(_Tp) <= sizeof(uint32_t)> > {
+struct _LIBCPP_HIDDEN __traits_base<_Tp, __enable_if_t<sizeof(_Tp) <= sizeof(uint32_t)>> {
   using type = uint32_t;
 
   /// The width estimation using a log10 algorithm.
@@ -62,7 +64,7 @@ struct _LIBCPP_HIDDEN __traits_base<_Tp, __enable_if_t<sizeof(_Tp) <= sizeof(uin
 };
 
 template <typename _Tp>
-struct _LIBCPP_HIDDEN __traits_base<_Tp, __enable_if_t<sizeof(_Tp) == sizeof(uint64_t)> > {
+struct _LIBCPP_HIDDEN __traits_base<_Tp, __enable_if_t<sizeof(_Tp) == sizeof(uint64_t)>> {
   using type = uint64_t;
 
   /// The width estimation using a log10 algorithm.
@@ -151,7 +153,7 @@ inline _LIBCPP_HIDE_FROM_ABI bool _LIBCPP_CONSTEXPR_SINCE_CXX23 __mul_overflowed
 
 template <typename _Tp>
 struct _LIBCPP_HIDDEN __traits : __traits_base<_Tp> {
-  static _LIBCPP_CONSTEXPR const ...
[truncated]

github-actions · 2025-03-17T13:46:17Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 1e89a76a0490b6c55a3e46ecf967da3e30c9112b 5c1188dfb27c3499f58849bde211ac03026d90d1 --extensions ,h,cpp -- libcxx/include/__charconv/tables.h libcxx/include/__charconv/to_chars_base_10.h libcxx/include/__charconv/to_chars_integral.h libcxx/include/__charconv/to_chars_result.h libcxx/include/__charconv/traits.h libcxx/include/__format/formatter_floating_point.h libcxx/include/__format/formatter_integral.h libcxx/include/__format/formatter_output.h libcxx/include/locale libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_pointer.pass.cpp

View the diff from clang-format here.

diff --git a/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_pointer.pass.cpp b/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_pointer.pass.cpp
index 13152742d5..df2ed100e2 100644
--- a/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_pointer.pass.cpp
+++ b/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_pointer.pass.cpp
@@ -21,31 +21,27 @@
 
 typedef std::num_put<char, cpp17_output_iterator<char*> > F;
 
-class my_facet
-    : public F
-{
+class my_facet : public F {
 public:
-    explicit my_facet(std::size_t refs = 0)
-        : F(refs) {}
+  explicit my_facet(std::size_t refs = 0) : F(refs) {}
 };
 
-int main(int, char**)
-{
-    const my_facet f(1);
-    {
-        std::ios ios(0);
-        void* v = 0;
-        char str[50];
-        cpp17_output_iterator<char*> iter = f.put(cpp17_output_iterator<char*>(str), ios, '*', v);
-        std::string ex(str, base(iter));
-        char expected_str[32] = {};
-        // num_put::put uses %p for pointer types, but the exact format of %p is
-        // implementation defined behavior for the C library. Compare output to
-        // snprintf for portability.
-        int rc = snprintf(expected_str, sizeof(expected_str), "%p", v);
-        assert(rc > 0);
-        assert(ex == expected_str);
-    }
+int main(int, char**) {
+  const my_facet f(1);
+  {
+    std::ios ios(0);
+    void* v = 0;
+    char str[50];
+    cpp17_output_iterator<char*> iter = f.put(cpp17_output_iterator<char*>(str), ios, '*', v);
+    std::string ex(str, base(iter));
+    char expected_str[32] = {};
+    // num_put::put uses %p for pointer types, but the exact format of %p is
+    // implementation defined behavior for the C library. Compare output to
+    // snprintf for portability.
+    int rc = snprintf(expected_str, sizeof(expected_str), "%p", v);
+    assert(rc > 0);
+    assert(ex == expected_str);
+  }
 
   return 0;
 }

mordante

LGTM when the CI passes.

mordante · 2025-03-17T17:12:41Z

@philnik777 before re-attempting this patch it would be good to first add tests to validate the existing behaviour.

This reverts commit d1156fc.

…3572) This reverts commit d1156fc. This patch fixes the reported incorrect formatting changes and adds tests for them. The performance should be unaffected, since there are no significant changes required to fix the bugs. Specifically, a `>` was changed to a `>=` to also add a `+` in the zero case, and we're checking for zero now before printing the octal and hexadecimal prefixes. Closes #131710

After 8fc2538 "Reapply '[libc++] Optimize num_put integral functions' (#131613) (#133572)", including <locale> in C++03 mode with Debug hardening enabled fails. This patch fixes that by applying a workaround used elsewhere to construct errc in C++03 mode.

Revert "[libc++] Optimize num_put integral functions (#120859)"

5c1188d

This reverts commit 15edf87.

alexfh requested a review from a team as a code owner March 17, 2025 13:41

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 17, 2025

alexfh assigned nico, ldionne and philnik777 Mar 17, 2025

alexfh mentioned this pull request Mar 17, 2025

[libc++] Optimize num_put integral functions #120859

Merged

alexfh requested review from ldionne and philnik777 March 17, 2025 13:45

mordante approved these changes Mar 17, 2025

View reviewed changes

alexfh merged commit d1156fc into main Mar 18, 2025
85 of 86 checks passed

alexfh deleted the revert-120859-optimize_num_put branch March 18, 2025 01:05

ldionne mentioned this pull request Mar 18, 2025

[libc++] Look into the revert in #131613 #131710

Closed

philnik777 added a commit to philnik777/llvm-project that referenced this pull request Mar 29, 2025

Reapply "[libc++] Optimize num_put integral functions" (llvm#131613)

0df8080

This reverts commit d1156fc.

philnik777 added a commit to philnik777/llvm-project that referenced this pull request Apr 11, 2025

Reapply "[libc++] Optimize num_put integral functions" (llvm#131613)

ad6cbd6

This reverts commit d1156fc.

stbergmann mentioned this pull request May 5, 2025

Fix use of std::errc() for C++ < 11 #138522

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert "[libc++] Optimize num_put integral functions" #131613

Revert "[libc++] Optimize num_put integral functions" #131613

Uh oh!

alexfh commented Mar 17, 2025

Uh oh!

llvmbot commented Mar 17, 2025

Uh oh!

github-actions bot commented Mar 17, 2025

Uh oh!

mordante left a comment

Uh oh!

mordante commented Mar 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Revert "[libc++] Optimize num_put integral functions" #131613

Revert "[libc++] Optimize num_put integral functions" #131613

Uh oh!

Conversation

alexfh commented Mar 17, 2025

Uh oh!

llvmbot commented Mar 17, 2025

Uh oh!

github-actions bot commented Mar 17, 2025

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

mordante commented Mar 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants