[libc++] Suppress unsigned-integer-overflow warning in <charconv>

[hchataing]

[libc++] Suppress unsigned-integer-overflow warning in <charconv>

Invoking __complement with 0 as parameter triggers this runtime error:

include/c++/v1/__charconv/traits.h:181:19: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior include/c++/v1/__charconv/traits.h:181:19

[github-actions]

Hello @hchataing 👋

Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.

• All contributions to LLVM must follow our LLVM AI Tool Use Policy. In particular, if you used AI while working on this PR, remember to add a note to the PR description.
• The LLVM Code-Review Policy and Practices document contains practical information about the PR process, including how patches are reviewed and accepted, and who can review a PR.
• Our LLVM Developer Policy describes our expectations for code quality, commit summaries and contains notes on our CI system.

Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description.

---

Frequently asked questions

How do I add reviewers?

This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically.

You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using @ followed by their GitHub username.

What if there are no comments?

If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers.

Are any special GitHub settings required to contribute to LLVM?

We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details.

---

If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse.

Thank you,
The LLVM Community

[llvmorg-github-actions]

@llvm/pr-subscribers-libcxx

Author: hchataing

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/225801.diff

1 Files Affected:

• (modified) libcxx/include/__charconv/traits.h (+2-1)

diff --git a/libcxx/include/__charconv/traits.h b/libcxx/include/__charconv/traits.h
index b8c840d1ebe32..2f08dac4c2afe 100644
--- a/libcxx/include/__charconv/traits.h
+++ b/libcxx/include/__charconv/traits.h
@@ -155,7 +155,8 @@ struct _LIBCPP_HIDDEN __traits : __traits_base<_Tp> {
 } // namespace __itoa
 
 template <typename _Tp>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _Tp __complement(_Tp __x) {
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI
+    _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK _Tp __complement(_Tp __x) {
   static_assert(is_unsigned<_Tp>::value, "cast to unsigned first");
   return _Tp(~__x + 1);
 }

[nickdesaulniers]

cc @llvm/android-maintainers

[nickdesaulniers]

Can you please include the text from the sanitizer diagnostic in the PR (and commit) description? This helps reviewers better understand your patch.

[hchataing]

This is the exact error.

prebuilts/clang/host/linux-x86/clang-r596125/include/c++/v1/__charconv/traits.h:181:19: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior prebuilts/clang/host/linux-x86/clang-r596125/include/c++/v1/__charconv/traits.h:181:19 

I updated the commit message with it.

[nickdesaulniers]

Thanks, can you please:

1. remove prebuilts/clang/host/linux-x86/clang-r596125/ from both lines of the commit message? Those are very specific to Android's build of llvm, so they are unnecessary information in the commit message.
2. duplicate the commit message in the PR description? (ctrl+f, No description provided.)

[hchataing]

Thanks, can you please:

1. remove prebuilts/clang/host/linux-x86/clang-r596125/ from both lines of the commit message? Those are very specific to Android's build of llvm, so they are unnecessary information in the commit message.
2. duplicate the commit message in the PR description? (ctrl+f, No description provided.)

Done 👍

[philnik777]

This needs testing.

[hchataing]

@philnik777 can you please indicate where and how ?
This is my first time uploading to llvm-project, I don't know the ins and outs yet.

[philnik777]

@philnik777 can you please indicate where and how ? This is my first time uploading to llvm-project, I don't know the ins and outs yet.

Well, you picked one where that's not super easy. You might be able to get away with simply adding it to the normal UBSan run (i.e. update params.py). We'd really want to fix all failures if we do that though. Depending on how many there are that might or might not be feasible.

[hchataing]

I uploaded a separate commit to test adding -fsanitize=unsigned-integer-overflow to the UBSan configuration. I will let the CI run to see if that is bringing out any failures.

@philnik777 is this the change that you were expecting ?

[hchataing]

@philnik777 I spent some more time on this (aided by AI I must admit).

The result is a new test case added to libcxx/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp, with the additional compilation to enable unsigned-integer-overflow. The change in libcxx/test/support/charconv_test_helpers.h is necessary to enforce runtime execution of the test cases... I verified that the test does fail without the fix.

[ldionne]

This makes sense to me, I have a few comments and a small question about a change, but non-blocking.

[github-actions]

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

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions h,cpp -- libcxx/include/__charconv/traits.h libcxx/test/std/utilities/charconv/charconv.from.chars/floating_point.pass.cpp libcxx/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp libcxx/test/std/utilities/charconv/charconv.from.chars/integral.roundtrip.pass.cpp libcxx/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp libcxx/test/support/charconv_test_helpers.h --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp b/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
index ab6f6724c..0ece8bdc8 100644
--- a/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
+++ b/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
@@ -24,137 +24,132 @@
 
 struct test_basics
 {
-    template <typename T>
-    TEST_CONSTEXPR_CXX23 void operator()()
+  template <typename T>
+  TEST_CONSTEXPR_CXX23 void operator()() {
+    std::from_chars_result r;
+    T x;
+
     {
-        std::from_chars_result r;
-        T x;
-
-        {
-            char s[] = "001x";
-
-            // the expected form of the subject sequence is a sequence of
-            // letters and digits representing an integer with the radix
-            // specified by base (C11 7.22.1.4/3)
-            r = std::from_chars(s, s + sizeof(s), x);
-            assert(r.ec == std::errc{});
-            assert(r.ptr == s + 3);
-            assert(x == 1);
-        }
-
-        {
-            // The string has more characters than valid in an 128-bit value.
-            char s[] = "0X7BAtSGHDkEIXZgQRfYChLpOzRnM ";
-
-            // The letters from a (or A) through z (or Z) are ascribed the
-            // values 10 through 35; (C11 7.22.1.4/3)
-            r = std::from_chars(s, s + sizeof(s), x, 36);
-            assert(r.ec == std::errc::result_out_of_range);
-            // The member ptr of the return value points to the first character
-            // not matching the pattern
-            assert(r.ptr == s + sizeof(s) - 2);
-            assert(x == 1);
-
-            // no "0x" or "0X" prefix shall appear if the value of base is 16
-            r = std::from_chars(s, s + sizeof(s), x, 16);
-            assert(r.ec == std::errc{});
-            assert(r.ptr == s + 1);
-            assert(x == 0);
-
-            // only letters and digits whose ascribed values are less than that
-            // of base are permitted. (C11 7.22.1.4/3)
-            r = std::from_chars(s + 2, s + sizeof(s), x, 12);
-            // If the parsed value is not in the range representable by the type
-            // of value,
-            if (!fits_in<T>(1150))
-            {
-                // value is unmodified and
-                assert(x == 0);
-                // the member ec of the return value is equal to
-                // errc::result_out_of_range
-                assert(r.ec == std::errc::result_out_of_range);
-            }
-            else
-            {
-                // Otherwise, value is set to the parsed value,
-                assert(x == 1150);
-                // and the member ec is value-initialized.
-                assert(r.ec == std::errc{});
-            }
-            assert(r.ptr == s + 5);
-        }
+      char s[] = "001x";
+
+      // the expected form of the subject sequence is a sequence of
+      // letters and digits representing an integer with the radix
+      // specified by base (C11 7.22.1.4/3)
+      r = std::from_chars(s, s + sizeof(s), x);
+      assert(r.ec == std::errc{});
+      assert(r.ptr == s + 3);
+      assert(x == 1);
     }
+
+    {
+      // The string has more characters than valid in an 128-bit value.
+      char s[] = "0X7BAtSGHDkEIXZgQRfYChLpOzRnM ";
+
+      // The letters from a (or A) through z (or Z) are ascribed the
+      // values 10 through 35; (C11 7.22.1.4/3)
+      r = std::from_chars(s, s + sizeof(s), x, 36);
+      assert(r.ec == std::errc::result_out_of_range);
+      // The member ptr of the return value points to the first character
+      // not matching the pattern
+      assert(r.ptr == s + sizeof(s) - 2);
+      assert(x == 1);
+
+      // no "0x" or "0X" prefix shall appear if the value of base is 16
+      r = std::from_chars(s, s + sizeof(s), x, 16);
+      assert(r.ec == std::errc{});
+      assert(r.ptr == s + 1);
+      assert(x == 0);
+
+      // only letters and digits whose ascribed values are less than that
+      // of base are permitted. (C11 7.22.1.4/3)
+      r = std::from_chars(s + 2, s + sizeof(s), x, 12);
+      // If the parsed value is not in the range representable by the type
+      // of value,
+      if (!fits_in<T>(1150)) {
+        // value is unmodified and
+        assert(x == 0);
+        // the member ec of the return value is equal to
+        // errc::result_out_of_range
+        assert(r.ec == std::errc::result_out_of_range);
+      } else {
+        // Otherwise, value is set to the parsed value,
+        assert(x == 1150);
+        // and the member ec is value-initialized.
+        assert(r.ec == std::errc{});
+      }
+      assert(r.ptr == s + 5);
+    }
+  }
 };
 
 struct test_signed
 {
-    template <typename T>
-    TEST_CONSTEXPR_CXX23 void operator()()
+  template <typename T>
+  TEST_CONSTEXPR_CXX23 void operator()() {
+    std::from_chars_result r;
+    T x = 42;
+
+    {
+      // If the pattern allows for an optional sign,
+      // but the string has no digit characters following the sign,
+      char s[] = "- 9+12";
+      r        = std::from_chars(s, s + sizeof(s), x);
+      // value is unmodified,
+      assert(x == 42);
+      // no characters match the pattern.
+      assert(r.ptr == s);
+      assert(r.ec == std::errc::invalid_argument);
+    }
+
+    {
+      char s[] = "9+12";
+      r        = std::from_chars(s, s + sizeof(s), x);
+      assert(r.ec == std::errc{});
+      // The member ptr of the return value points to the first character
+      // not matching the pattern,
+      assert(r.ptr == s + 1);
+      assert(x == 9);
+    }
+
+    {
+      char s[] = "12";
+      r        = std::from_chars(s, s + 2, x);
+      assert(r.ec == std::errc{});
+      // or has the value last if all characters match.
+      assert(r.ptr == s + 2);
+      assert(x == 12);
+    }
+
+    {
+      // '-' is the only sign that may appear
+      char s[] = "+30";
+      // If no characters match the pattern,
+      r = std::from_chars(s, s + sizeof(s), x);
+      // value is unmodified,
+      assert(x == 12);
+      // the member ptr of the return value is first and
+      assert(r.ptr == s);
+      // the member ec is equal to errc::invalid_argument.
+      assert(r.ec == std::errc::invalid_argument);
+    }
+
     {
-        std::from_chars_result r;
-        T x = 42;
-
-        {
-            // If the pattern allows for an optional sign,
-            // but the string has no digit characters following the sign,
-            char s[] = "- 9+12";
-            r = std::from_chars(s, s + sizeof(s), x);
-            // value is unmodified,
-            assert(x == 42);
-            // no characters match the pattern.
-            assert(r.ptr == s);
-            assert(r.ec == std::errc::invalid_argument);
-        }
-
-        {
-            char s[] = "9+12";
-            r = std::from_chars(s, s + sizeof(s), x);
-            assert(r.ec == std::errc{});
-            // The member ptr of the return value points to the first character
-            // not matching the pattern,
-            assert(r.ptr == s + 1);
-            assert(x == 9);
-        }
-
-        {
-            char s[] = "12";
-            r = std::from_chars(s, s + 2, x);
-            assert(r.ec == std::errc{});
-            // or has the value last if all characters match.
-            assert(r.ptr == s + 2);
-            assert(x == 12);
-        }
-
-        {
-            // '-' is the only sign that may appear
-            char s[] = "+30";
-            // If no characters match the pattern,
-            r = std::from_chars(s, s + sizeof(s), x);
-            // value is unmodified,
-            assert(x == 12);
-            // the member ptr of the return value is first and
-            assert(r.ptr == s);
-            // the member ec is equal to errc::invalid_argument.
-            assert(r.ec == std::errc::invalid_argument);
-        }
-
-        {
-            // Ensure "-0" does not trigger unsigned integer overflow.
-            char s[] = "-0";
-            r = std::from_chars(s, s + sizeof(s), x);
-            assert(r.ec == std::errc{});
-            assert(r.ptr == s + 2);
-            assert(x == 0);
-        }
+      // Ensure "-0" does not trigger unsigned integer overflow.
+      char s[] = "-0";
+      r        = std::from_chars(s, s + sizeof(s), x);
+      assert(r.ec == std::errc{});
+      assert(r.ptr == s + 2);
+      assert(x == 0);
     }
+  }
 };
 
 TEST_CONSTEXPR_CXX23 bool test()
 {
-    types::for_each(integrals(), test_basics());
-    types::for_each(types::signed_integer_types(), test_signed());
+  types::for_each(integrals(), test_basics());
+  types::for_each(types::signed_integer_types(), test_signed());
 
-    return true;
+  return true;
 }
 
 int main(int, char**) {
diff --git a/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.roundtrip.pass.cpp b/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.roundtrip.pass.cpp
index a9681202b..a4f1a467f 100644
--- a/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.roundtrip.pass.cpp
+++ b/libcxx/test/std/utilities/charconv/charconv.from.chars/integral.roundtrip.pass.cpp
@@ -21,64 +21,58 @@
 #include "test_macros.h"
 #include "charconv_test_helpers.h"
 
-struct test_basics : roundtrip_test_base
-{
-    template <typename T>
-    TEST_CONSTEXPR_CXX23 void operator()()
-    {
-        test<T>(0);
-        test<T>(42);
-        test<T>(32768);
-        test<T>(0, 10);
-        test<T>(42, 10);
-        test<T>(32768, 10);
-        test<T>(0xf, 16);
-        test<T>(0xdeadbeaf, 16);
-        test<T>(0755, 8);
-
-        for (int b = 2; b < 37; ++b)
-        {
-            using xl = std::numeric_limits<T>;
-
-            test<T>(1, b);
-            test<T>(-1, b);
-            test<T>(xl::lowest(), b);
-            test<T>((xl::max)(), b);
-            test<T>((xl::max)() / 2, b);
-        }
+struct test_basics : roundtrip_test_base {
+  template <typename T>
+  TEST_CONSTEXPR_CXX23 void operator()() {
+    test<T>(0);
+    test<T>(42);
+    test<T>(32768);
+    test<T>(0, 10);
+    test<T>(42, 10);
+    test<T>(32768, 10);
+    test<T>(0xf, 16);
+    test<T>(0xdeadbeaf, 16);
+    test<T>(0755, 8);
+
+    for (int b = 2; b < 37; ++b) {
+      using xl = std::numeric_limits<T>;
+
+      test<T>(1, b);
+      test<T>(-1, b);
+      test<T>(xl::lowest(), b);
+      test<T>((xl::max)(), b);
+      test<T>((xl::max)() / 2, b);
     }
+  }
 };
 
-struct test_signed : roundtrip_test_base
-{
-    template <typename T>
-    TEST_CONSTEXPR_CXX23 void operator()()
-    {
-        test<T>(-1);
-        test<T>(-12);
-        test<T>(-1, 10);
-        test<T>(-12, 10);
-        test<T>(-21734634, 10);
-        test<T>(-2647, 2);
-        test<T>(-0xcc1, 16);
-
-        for (int b = 2; b < 37; ++b)
-        {
-            using xl = std::numeric_limits<T>;
-
-            test<T>(0, b);
-            test<T>(xl::lowest(), b);
-            test<T>((xl::max)(), b);
-        }
+struct test_signed : roundtrip_test_base {
+  template <typename T>
+  TEST_CONSTEXPR_CXX23 void operator()() {
+    test<T>(-1);
+    test<T>(-12);
+    test<T>(-1, 10);
+    test<T>(-12, 10);
+    test<T>(-21734634, 10);
+    test<T>(-2647, 2);
+    test<T>(-0xcc1, 16);
+
+    for (int b = 2; b < 37; ++b) {
+      using xl = std::numeric_limits<T>;
+
+      test<T>(0, b);
+      test<T>(xl::lowest(), b);
+      test<T>((xl::max)(), b);
     }
+  }
 };
 
 TEST_CONSTEXPR_CXX23 bool test()
 {
-    types::for_each(integrals(), test_basics());
-    types::for_each(types::signed_integer_types(), test_signed());
+  types::for_each(integrals(), test_basics());
+  types::for_each(types::signed_integer_types(), test_signed());
 
-    return true;
+  return true;
 }
 
 int main(int, char**) {
diff --git a/libcxx/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp b/libcxx/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
index 4d07ad78d..d92911405 100644
--- a/libcxx/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
+++ b/libcxx/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
@@ -51,65 +51,63 @@ TEST_CONSTEXPR_CXX23 __int128_t make_i128(__int128_t a, __int128_t b, std::int64
 }
 #endif
 
-struct test_basics : to_chars_test_base
-{
-    template <typename T>
-    TEST_CONSTEXPR_CXX23 void operator()()
-    {
-        test<T>(0, "0");
-        test<T>(42, "42");
-        test<T>(32768, "32768");
-        test<T>(0, "0", 10);
-        test<T>(42, "42", 10);
-        test<T>(32768, "32768", 10);
-        test<T>(0xf, "f", 16);
-        test<T>(0xdeadbeaf, "deadbeaf", 16);
-        test<T>(0755, "755", 8);
+struct test_basics : to_chars_test_base {
+  template <typename T>
+  TEST_CONSTEXPR_CXX23 void operator()() {
+    test<T>(0, "0");
+    test<T>(42, "42");
+    test<T>(32768, "32768");
+    test<T>(0, "0", 10);
+    test<T>(42, "42", 10);
+    test<T>(32768, "32768", 10);
+    test<T>(0xf, "f", 16);
+    test<T>(0xdeadbeaf, "deadbeaf", 16);
+    test<T>(0755, "755", 8);
 
-        // Test each len till len of UINT64_MAX = 20 because to_chars algorithm
-        // makes branches based on decimal digits count in the value string
-        // representation.
-        // Test driver automatically skips values not fitting into source type.
-        test<T>(1UL, "1");
-        test<T>(12UL, "12");
-        test<T>(123UL, "123");
-        test<T>(1234UL, "1234");
-        test<T>(12345UL, "12345");
-        test<T>(123456UL, "123456");
-        test<T>(1234567UL, "1234567");
-        test<T>(12345678UL, "12345678");
-        test<T>(123456789UL, "123456789");
-        test<T>(1234567890UL, "1234567890");
-        test<T>(12345678901UL, "12345678901");
-        test<T>(123456789012UL, "123456789012");
-        test<T>(1234567890123UL, "1234567890123");
-        test<T>(12345678901234UL, "12345678901234");
-        test<T>(123456789012345UL, "123456789012345");
-        test<T>(1234567890123456UL, "1234567890123456");
-        test<T>(12345678901234567UL, "12345678901234567");
-        test<T>(123456789012345678UL, "123456789012345678");
-        test<T>(1234567890123456789UL, "1234567890123456789");
-        test<T>(12345678901234567890UL, "12345678901234567890");
+    // Test each len till len of UINT64_MAX = 20 because to_chars algorithm
+    // makes branches based on decimal digits count in the value string
+    // representation.
+    // Test driver automatically skips values not fitting into source type.
+    test<T>(1UL, "1");
+    test<T>(12UL, "12");
+    test<T>(123UL, "123");
+    test<T>(1234UL, "1234");
+    test<T>(12345UL, "12345");
+    test<T>(123456UL, "123456");
+    test<T>(1234567UL, "1234567");
+    test<T>(12345678UL, "12345678");
+    test<T>(123456789UL, "123456789");
+    test<T>(1234567890UL, "1234567890");
+    test<T>(12345678901UL, "12345678901");
+    test<T>(123456789012UL, "123456789012");
+    test<T>(1234567890123UL, "1234567890123");
+    test<T>(12345678901234UL, "12345678901234");
+    test<T>(123456789012345UL, "123456789012345");
+    test<T>(1234567890123456UL, "1234567890123456");
+    test<T>(12345678901234567UL, "12345678901234567");
+    test<T>(123456789012345678UL, "123456789012345678");
+    test<T>(1234567890123456789UL, "1234567890123456789");
+    test<T>(12345678901234567890UL, "12345678901234567890");
 #ifndef TEST_HAS_NO_INT128
-        test<T>(make_u128(12UL, 3456789012345678901UL), "123456789012345678901");
-        test<T>(make_u128(123UL, 4567890123456789012UL), "1234567890123456789012");
-        test<T>(make_u128(1234UL, 5678901234567890123UL), "12345678901234567890123");
-        test<T>(make_u128(12345UL, 6789012345678901234UL), "123456789012345678901234");
-        test<T>(make_u128(123456UL, 7890123456789012345UL), "1234567890123456789012345");
-        test<T>(make_u128(1234567UL, 8901234567890123456UL), "12345678901234567890123456");
-        test<T>(make_u128(12345678UL, 9012345678901234567UL), "123456789012345678901234567");
-        test<T>(make_u128(123456789UL, 123456789012345678UL), "1234567890123456789012345678");
-        test<T>(make_u128(123UL, 4567890123456UL, 7890123456789UL), "12345678901234567890123456789");
-        test<T>(make_u128(1234UL, 5678901234567UL, 8901234567890UL), "123456789012345678901234567890");
-        test<T>(make_u128(12345UL, 6789012345678UL, 9012345678901UL), "1234567890123456789012345678901");
-        test<T>(make_u128(123456UL, 7890123456789UL, 123456789012UL), "12345678901234567890123456789012");
-        test<T>(make_u128(1234567UL, 8901234567890UL, 1234567890123UL), "123456789012345678901234567890123");
-        test<T>(make_u128(12345678UL, 9012345678901UL, 2345678901234UL), "1234567890123456789012345678901234");
-        test<T>(make_u128(123456789UL, 123456789012UL, 3456789012345UL), "12345678901234567890123456789012345");
-        test<T>(make_u128(1234567890UL, 1234567890123UL, 4567890123456UL), "123456789012345678901234567890123456");
-        test<T>(make_u128(12345678901UL, 2345678901234UL, 5678901234567UL), "1234567890123456789012345678901234567");
-        test<T>(make_u128(123456789012UL, 3456789012345UL, 6789012345678UL), "12345678901234567890123456789012345678");
-        test<T>(make_u128(1234567890123UL, 4567890123456UL, 7890123456789UL), "123456789012345678901234567890123456789");
+    test<T>(make_u128(12UL, 3456789012345678901UL), "123456789012345678901");
+    test<T>(make_u128(123UL, 4567890123456789012UL), "1234567890123456789012");
+    test<T>(make_u128(1234UL, 5678901234567890123UL), "12345678901234567890123");
+    test<T>(make_u128(12345UL, 6789012345678901234UL), "123456789012345678901234");
+    test<T>(make_u128(123456UL, 7890123456789012345UL), "1234567890123456789012345");
+    test<T>(make_u128(1234567UL, 8901234567890123456UL), "12345678901234567890123456");
+    test<T>(make_u128(12345678UL, 9012345678901234567UL), "123456789012345678901234567");
+    test<T>(make_u128(123456789UL, 123456789012345678UL), "1234567890123456789012345678");
+    test<T>(make_u128(123UL, 4567890123456UL, 7890123456789UL), "12345678901234567890123456789");
+    test<T>(make_u128(1234UL, 5678901234567UL, 8901234567890UL), "123456789012345678901234567890");
+    test<T>(make_u128(12345UL, 6789012345678UL, 9012345678901UL), "1234567890123456789012345678901");
+    test<T>(make_u128(123456UL, 7890123456789UL, 123456789012UL), "12345678901234567890123456789012");
+    test<T>(make_u128(1234567UL, 8901234567890UL, 1234567890123UL), "123456789012345678901234567890123");
+    test<T>(make_u128(12345678UL, 9012345678901UL, 2345678901234UL), "1234567890123456789012345678901234");
+    test<T>(make_u128(123456789UL, 123456789012UL, 3456789012345UL), "12345678901234567890123456789012345");
+    test<T>(make_u128(1234567890UL, 1234567890123UL, 4567890123456UL), "123456789012345678901234567890123456");
+    test<T>(make_u128(12345678901UL, 2345678901234UL, 5678901234567UL), "1234567890123456789012345678901234567");
+    test<T>(make_u128(123456789012UL, 3456789012345UL, 6789012345678UL), "12345678901234567890123456789012345678");
+    test<T>(make_u128(1234567890123UL, 4567890123456UL, 7890123456789UL), "123456789012345678901234567890123456789");
 #endif
 
         // Test special cases with zeros inside a value string representation,
@@ -167,67 +165,65 @@ struct test_basics : to_chars_test_base
             test_value<T>((xl::max)(), b);
             test_value<T>((xl::max)() / 2, b);
         }
-    }
+  }
 };
 
-struct test_signed : to_chars_test_base
-{
-    template <typename T>
-    TEST_CONSTEXPR_CXX23 void operator()()
-    {
-        test<T>(-1, "-1");
-        test<T>(-12, "-12");
-        test<T>(-1, "-1", 10);
-        test<T>(-12, "-12", 10);
-        test<T>(-21734634, "-21734634", 10);
-        test<T>(-2647, "-101001010111", 2);
-        test<T>(-0xcc1, "-cc1", 16);
+struct test_signed : to_chars_test_base {
+  template <typename T>
+  TEST_CONSTEXPR_CXX23 void operator()() {
+    test<T>(-1, "-1");
+    test<T>(-12, "-12");
+    test<T>(-1, "-1", 10);
+    test<T>(-12, "-12", 10);
+    test<T>(-21734634, "-21734634", 10);
+    test<T>(-2647, "-101001010111", 2);
+    test<T>(-0xcc1, "-cc1", 16);
 
-        // Test each len till len of INT64_MAX = 19 because to_chars algorithm
-        // makes branches based on decimal digits count in the value string
-        // representation.
-        // Test driver automatically skips values not fitting into source type.
-        test<T>(-1L, "-1");
-        test<T>(-12L, "-12");
-        test<T>(-123L, "-123");
-        test<T>(-1234L, "-1234");
-        test<T>(-12345L, "-12345");
-        test<T>(-123456L, "-123456");
-        test<T>(-1234567L, "-1234567");
-        test<T>(-12345678L, "-12345678");
-        test<T>(-123456789L, "-123456789");
-        test<T>(-1234567890L, "-1234567890");
-        test<T>(-12345678901L, "-12345678901");
-        test<T>(-123456789012L, "-123456789012");
-        test<T>(-1234567890123L, "-1234567890123");
-        test<T>(-12345678901234L, "-12345678901234");
-        test<T>(-123456789012345L, "-123456789012345");
-        test<T>(-1234567890123456L, "-1234567890123456");
-        test<T>(-12345678901234567L, "-12345678901234567");
-        test<T>(-123456789012345678L, "-123456789012345678");
-        test<T>(-1234567890123456789L, "-1234567890123456789");
+    // Test each len till len of INT64_MAX = 19 because to_chars algorithm
+    // makes branches based on decimal digits count in the value string
+    // representation.
+    // Test driver automatically skips values not fitting into source type.
+    test<T>(-1L, "-1");
+    test<T>(-12L, "-12");
+    test<T>(-123L, "-123");
+    test<T>(-1234L, "-1234");
+    test<T>(-12345L, "-12345");
+    test<T>(-123456L, "-123456");
+    test<T>(-1234567L, "-1234567");
+    test<T>(-12345678L, "-12345678");
+    test<T>(-123456789L, "-123456789");
+    test<T>(-1234567890L, "-1234567890");
+    test<T>(-12345678901L, "-12345678901");
+    test<T>(-123456789012L, "-123456789012");
+    test<T>(-1234567890123L, "-1234567890123");
+    test<T>(-12345678901234L, "-12345678901234");
+    test<T>(-123456789012345L, "-123456789012345");
+    test<T>(-1234567890123456L, "-1234567890123456");
+    test<T>(-12345678901234567L, "-12345678901234567");
+    test<T>(-123456789012345678L, "-123456789012345678");
+    test<T>(-1234567890123456789L, "-1234567890123456789");
 #ifndef TEST_HAS_NO_INT128
-        test<T>(make_i128(-1L, 2345678901234567890L), "-12345678901234567890");
-        test<T>(make_i128(-12L, 3456789012345678901L), "-123456789012345678901");
-        test<T>(make_i128(-123L, 4567890123456789012L), "-1234567890123456789012");
-        test<T>(make_i128(-1234L, 5678901234567890123L), "-12345678901234567890123");
-        test<T>(make_i128(-12345L, 6789012345678901234L), "-123456789012345678901234");
-        test<T>(make_i128(-123456L, 7890123456789012345L), "-1234567890123456789012345");
-        test<T>(make_i128(-1234567L, 8901234567890123456L), "-12345678901234567890123456");
-        test<T>(make_i128(-12345678L, 9012345678901234567L), "-123456789012345678901234567");
-        test<T>(make_i128(-123456789L, 123456789012345678L), "-1234567890123456789012345678");
-        test<T>(make_i128(-1234567890L, 1234567890123456789L), "-12345678901234567890123456789");
-        test<T>(make_i128(-123L, 4567890123456L, 7890123456789L), "-12345678901234567890123456789");
-        test<T>(make_i128(-1234L, 5678901234567L, 8901234567890L), "-123456789012345678901234567890");
-        test<T>(make_i128(-12345L, 6789012345678L, 9012345678901L), "-1234567890123456789012345678901");
-        test<T>(make_i128(-123456L, 7890123456789L, 123456789012L), "-12345678901234567890123456789012");
-        test<T>(make_i128(-1234567L, 8901234567890L, 1234567890123L), "-123456789012345678901234567890123");
-        test<T>(make_i128(-12345678L, 9012345678901L, 2345678901234L), "-1234567890123456789012345678901234");
-        test<T>(make_i128(-123456789L, 123456789012L, 3456789012345L), "-12345678901234567890123456789012345");
-        test<T>(make_i128(-1234567890L, 1234567890123L, 4567890123456L), "-123456789012345678901234567890123456");
-        test<T>(make_i128(-12345678901L, 2345678901234L, 5678901234567L), "-1234567890123456789012345678901234567");
-        test<T>(make_i128(-123456789012L, 3456789012345L, 6789012345678L), "-12345678901234567890123456789012345678");
-        test<T>(make_i128(-1234567890123L, 4567890123456L, 7890123456789L), "-123456789012345678901234567890123456789");
+    test<T>(make_i128(-1L, 2345678901234567890L), "-12345678901234567890");
+    test<T>(make_i128(-12L, 3456789012345678901L), "-123456789012345678901");
+    test<T>(make_i128(-123L, 4567890123456789012L), "-1234567890123456789012");
+    test<T>(make_i128(-1234L, 5678901234567890123L), "-12345678901234567890123");
+    test<T>(make_i128(-12345L, 6789012345678901234L), "-123456789012345678901234");
+    test<T>(make_i128(-123456L, 7890123456789012345L), "-1234567890123456789012345");
+    test<T>(make_i128(-1234567L, 8901234567890123456L), "-12345678901234567890123456");
+    test<T>(make_i128(-12345678L, 9012345678901234567L), "-123456789012345678901234567");
+    test<T>(make_i128(-123456789L, 123456789012345678L), "-1234567890123456789012345678");
+    test<T>(make_i128(-1234567890L, 1234567890123456789L), "-12345678901234567890123456789");
+    test<T>(make_i128(-123L, 4567890123456L, 7890123456789L), "-12345678901234567890123456789");
+    test<T>(make_i128(-1234L, 5678901234567L, 8901234567890L), "-123456789012345678901234567890");
+    test<T>(make_i128(-12345L, 6789012345678L, 9012345678901L), "-1234567890123456789012345678901");
+    test<T>(make_i128(-123456L, 7890123456789L, 123456789012L), "-12345678901234567890123456789012");
+    test<T>(make_i128(-1234567L, 8901234567890L, 1234567890123L), "-123456789012345678901234567890123");
+    test<T>(make_i128(-12345678L, 9012345678901L, 2345678901234L), "-1234567890123456789012345678901234");
+    test<T>(make_i128(-123456789L, 123456789012L, 3456789012345L), "-12345678901234567890123456789012345");
+    test<T>(make_i128(-1234567890L, 1234567890123L, 4567890123456L), "-123456789012345678901234567890123456");
+    test<T>(make_i128(-12345678901L, 2345678901234L, 5678901234567L), "-1234567890123456789012345678901234567");
+    test<T>(make_i128(-123456789012L, 3456789012345L, 6789012345678L), "-12345678901234567890123456789012345678");
+    test<T>(make_i128(-1234567890123L, 4567890123456L, 7890123456789L), "-123456789012345678901234567890123456789");
 #endif
 
         // Test special cases with zeros inside a value string representation,
@@ -284,15 +280,15 @@ struct test_signed : to_chars_test_base
             test_value<T>(xl::lowest(), b);
             test_value<T>((xl::max)(), b);
         }
-    }
+  }
 };
 
 TEST_CONSTEXPR_CXX23 bool test()
 {
-    types::for_each(integrals(), test_basics());
-    types::for_each(types::signed_integer_types(), test_signed());
+  types::for_each(integrals(), test_basics());
+  types::for_each(types::signed_integer_types(), test_signed());
 
-    return true;
+  return true;
 }
 
 int main(int, char**)
diff --git a/libcxx/test/support/charconv_test_helpers.h b/libcxx/test/support/charconv_test_helpers.h
index 3d163322a..7cd14204d 100644
--- a/libcxx/test/support/charconv_test_helpers.h
+++ b/libcxx/test/support/charconv_test_helpers.h
@@ -81,58 +81,56 @@ fits_in(T v)
 
 struct to_chars_test_base
 {
-    template <typename X, typename T, std::size_t N, typename... Ts>
-    TEST_CONSTEXPR_CXX23 void test(T v, char const (&expect)[N], Ts... args)
-    {
-        std::to_chars_result r;
-
-        constexpr std::size_t len = N - 1;
-        static_assert(len > 0, "expected output won't be empty");
-
-        if (!fits_in<X>(v))
-            return;
-
-        r = std::to_chars(buf, buf + len - 1, X(v), args...);
-        assert(r.ptr == buf + len - 1);
-        assert(r.ec == std::errc::value_too_large);
-
-        r = std::to_chars(buf, buf + sizeof(buf), X(v), args...);
-        assert(r.ptr == buf + len);
-        assert(r.ec == std::errc{});
-        assert(std::equal(buf, buf + len, expect));
-    }
-
-    template <typename X, typename... Ts>
-    TEST_CONSTEXPR_CXX23 void test_value(X v, Ts... args)
-    {
-        std::to_chars_result r;
-
-        // Poison the buffer for testing whether a successful std::to_chars
-        // doesn't modify data beyond r.ptr. Use unsigned values to avoid
-        // overflowing char when it's signed.
-        std::iota(buf, buf + sizeof(buf), static_cast<unsigned char>(1));
-        r = std::to_chars(buf, buf + sizeof(buf), v, args...);
-        assert(r.ec == std::errc{});
-        for (std::size_t i = r.ptr - buf; i < sizeof(buf); ++i)
-            assert(static_cast<unsigned char>(buf[i]) == i + 1);
-        *r.ptr = '\0';
+  template <typename X, typename T, std::size_t N, typename... Ts>
+  TEST_CONSTEXPR_CXX23 void test(T v, char const (&expect)[N], Ts... args) {
+    std::to_chars_result r;
+
+    constexpr std::size_t len = N - 1;
+    static_assert(len > 0, "expected output won't be empty");
+
+    if (!fits_in<X>(v))
+      return;
+
+    r = std::to_chars(buf, buf + len - 1, X(v), args...);
+    assert(r.ptr == buf + len - 1);
+    assert(r.ec == std::errc::value_too_large);
+
+    r = std::to_chars(buf, buf + sizeof(buf), X(v), args...);
+    assert(r.ptr == buf + len);
+    assert(r.ec == std::errc{});
+    assert(std::equal(buf, buf + len, expect));
+  }
+
+  template <typename X, typename... Ts>
+  TEST_CONSTEXPR_CXX23 void test_value(X v, Ts... args) {
+    std::to_chars_result r;
+
+    // Poison the buffer for testing whether a successful std::to_chars
+    // doesn't modify data beyond r.ptr. Use unsigned values to avoid
+    // overflowing char when it's signed.
+    std::iota(buf, buf + sizeof(buf), static_cast<unsigned char>(1));
+    r = std::to_chars(buf, buf + sizeof(buf), v, args...);
+    assert(r.ec == std::errc{});
+    for (std::size_t i = r.ptr - buf; i < sizeof(buf); ++i)
+      assert(static_cast<unsigned char>(buf[i]) == i + 1);
+    *r.ptr = '\0';
 
 #ifndef TEST_HAS_NO_INT128
         if (sizeof(X) == sizeof(__int128_t)) {
-            auto a = fromchars128_impl<X>(buf, r.ptr, args...);
-            assert(v == a);
+          auto a = fromchars128_impl<X>(buf, r.ptr, args...);
+          assert(v == a);
         } else
 #endif
         {
-            auto a = fromchars_impl<X>(buf, r.ptr, args...);
-            assert(v == a);
+          auto a = fromchars_impl<X>(buf, r.ptr, args...);
+          assert(v == a);
         }
 
         auto ep = r.ptr - 1;
         r = std::to_chars(buf, ep, v, args...);
         assert(r.ptr == ep);
         assert(r.ec == std::errc::value_too_large);
-    }
+  }
 
 private:
     static TEST_CONSTEXPR_CXX23 long long fromchars_impl(char const* p, char const* ep, int base, true_type)
@@ -204,18 +202,16 @@ private:
 
     template <typename X>
     static TEST_CONSTEXPR_CXX23 auto fromchars128_impl(char const* p, char const* ep, int base = 10)
-    -> decltype(fromchars128_impl(p, ep, base, std::is_signed<X>()))
-    {
-        return fromchars128_impl(p, ep, base, std::is_signed<X>());
+        -> decltype(fromchars128_impl(p, ep, base, std::is_signed<X>())) {
+      return fromchars128_impl(p, ep, base, std::is_signed<X>());
     }
 
 #endif
 
     template <typename X>
     static TEST_CONSTEXPR_CXX23 auto fromchars_impl(char const* p, char const* ep, int base = 10)
-    -> decltype(fromchars_impl(p, ep, base, std::is_signed<X>()))
-    {
-        return fromchars_impl(p, ep, base, std::is_signed<X>());
+        -> decltype(fromchars_impl(p, ep, base, std::is_signed<X>())) {
+      return fromchars_impl(p, ep, base, std::is_signed<X>());
     }
 
     char buf[150];
@@ -223,48 +219,41 @@ private:
 
 struct roundtrip_test_base
 {
-    template <typename X, typename T, typename... Ts>
-    TEST_CONSTEXPR_CXX23 void test(T v, Ts... args)
-    {
-        std::from_chars_result r2;
-        std::to_chars_result r;
-        X x = 0xc;
-
-        if (fits_in<X>(v))
-        {
-            r = std::to_chars(buf, buf + sizeof(buf), v, args...);
-            assert(r.ec == std::errc{});
-
-            r2 = std::from_chars(buf, r.ptr, x, args...);
-            assert(r2.ptr == r.ptr);
-            assert(x == X(v));
-        }
-        else
-        {
-            r = std::to_chars(buf, buf + sizeof(buf), v, args...);
-            assert(r.ec == std::errc{});
-
-            r2 = std::from_chars(buf, r.ptr, x, args...);
-
-            TEST_DIAGNOSTIC_PUSH
-            TEST_MSVC_DIAGNOSTIC_IGNORED(4127) // conditional expression is constant
-
-            if (std::is_signed<T>::value && v < 0 && std::is_unsigned<X>::value)
-            {
-                assert(x == 0xc);
-                assert(r2.ptr == buf);
-                assert(r2.ec == std::errc::invalid_argument);
-            }
-            else
-            {
-                assert(x == 0xc);
-                assert(r2.ptr == r.ptr);
-                assert(r2.ec == std::errc::result_out_of_range);
-            }
-
-            TEST_DIAGNOSTIC_POP
-        }
+  template <typename X, typename T, typename... Ts>
+  TEST_CONSTEXPR_CXX23 void test(T v, Ts... args) {
+    std::from_chars_result r2;
+    std::to_chars_result r;
+    X x = 0xc;
+
+    if (fits_in<X>(v)) {
+      r = std::to_chars(buf, buf + sizeof(buf), v, args...);
+      assert(r.ec == std::errc{});
+
+      r2 = std::from_chars(buf, r.ptr, x, args...);
+      assert(r2.ptr == r.ptr);
+      assert(x == X(v));
+    } else {
+      r = std::to_chars(buf, buf + sizeof(buf), v, args...);
+      assert(r.ec == std::errc{});
+
+      r2 = std::from_chars(buf, r.ptr, x, args...);
+
+      TEST_DIAGNOSTIC_PUSH
+      TEST_MSVC_DIAGNOSTIC_IGNORED(4127) // conditional expression is constant
+
+      if (std::is_signed<T>::value && v < 0 && std::is_unsigned<X>::value) {
+        assert(x == 0xc);
+        assert(r2.ptr == buf);
+        assert(r2.ec == std::errc::invalid_argument);
+      } else {
+        assert(x == 0xc);
+        assert(r2.ptr == r.ptr);
+        assert(r2.ec == std::errc::result_out_of_range);
+      }
+
+      TEST_DIAGNOSTIC_POP
     }
+  }
 
 private:
     char buf[150];

[ldionne]

Can be merged once the CI is green (modulo code formatting in the test suite). Thanks!

[nickdesaulniers]

Looks like you need to run git clang-format HEAD~2 @hchataing

[hchataing]

Formatting applied 👍

[ldionne]

As requested in #225801 (comment), we'd rather not reformat unrelated test code, so I undid the formatting-only changes to make the PR's diff reviewable.

[hchataing]

As requested in #225801 (comment), we'd rather not reformat unrelated test code, so I undid the formatting-only changes to make the PR's diff reviewable.

That was the output of git clang-format ..

[ldionne]

As requested in #225801 (comment), we'd rather not reformat unrelated test code, so I undid the formatting-only changes to make the PR's diff reviewable.

That was the output of git clang-format ..

That's fine, but what I am saying is that it makes the PR unreviewable, so we'd rather land it with a red formatting job than make the PR unreviewable. That's what we do for changes in the test suite since we have not reformatted our whole test suite yet.

[hchataing]

As requested in #225801 (comment), we'd rather not reformat unrelated test code, so I undid the formatting-only changes to make the PR's diff reviewable.

That was the output of git clang-format ..

That's fine, but what I am saying is that it makes the PR unreviewable, so we'd rather land it with a red formatting job than make the PR unreviewable. That's what we do for changes in the test suite since we have not reformatted our whole test suite yet.

I have nothing to add on my side then 👍