[libcxx-commits] [libcxx] [libc++] std::atomic primary template should not have a `difference_type` member type (PR #123236)

Damien L-G via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jan 16 13:47:26 PST 2025


https://github.com/dalg24 updated https://github.com/llvm/llvm-project/pull/123236

>From 3695367ec94f55c5688a871886be9a7f9800beca Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Thu, 16 Jan 2025 16:39:58 -0500
Subject: [PATCH 1/5] Apply clang-format

---
 libcxx/test/std/atomics/types.pass.cpp | 200 ++++++++++++-------------
 1 file changed, 96 insertions(+), 104 deletions(-)

diff --git a/libcxx/test/std/atomics/types.pass.cpp b/libcxx/test/std/atomics/types.pass.cpp
index 33512c037804f8..e07a5bf5fe17b7 100644
--- a/libcxx/test/std/atomics/types.pass.cpp
+++ b/libcxx/test/std/atomics/types.pass.cpp
@@ -30,162 +30,154 @@
 #endif
 
 template <class A, bool Integral>
-struct test_atomic
-{
-    test_atomic()
-    {
-        A a; (void)a;
+struct test_atomic {
+  test_atomic() {
+    A a;
+    (void)a;
 #if TEST_STD_VER >= 17
     static_assert((std::is_same_v<typename A::value_type, decltype(a.load())>), "");
 #endif
-    }
+  }
 };
 
 template <class A>
-struct test_atomic<A, true>
-{
-    test_atomic()
-    {
-        A a; (void)a;
+struct test_atomic<A, true> {
+  test_atomic() {
+    A a;
+    (void)a;
 #if TEST_STD_VER >= 17
     static_assert((std::is_same_v<typename A::value_type, decltype(a.load())>), "");
     static_assert((std::is_same_v<typename A::value_type, typename A::difference_type>), "");
 #endif
-    }
+  }
 };
 
 template <class A>
-struct test_atomic<A*, false>
-{
-    test_atomic()
-    {
-        A a; (void)a;
+struct test_atomic<A*, false> {
+  test_atomic() {
+    A a;
+    (void)a;
 #if TEST_STD_VER >= 17
     static_assert((std::is_same_v<typename A::value_type, decltype(a.load())>), "");
     static_assert((std::is_same_v<typename A::difference_type, std::ptrdiff_t>), "");
 #endif
-    }
+  }
 };
 
 template <class T>
-void
-test()
-{
-    using A = std::atomic<T>;
+void test() {
+  using A = std::atomic<T>;
 #if TEST_STD_VER >= 17
-    static_assert((std::is_same_v<typename A::value_type, T>), "");
+  static_assert((std::is_same_v<typename A::value_type, T>), "");
 #endif
-    test_atomic<A, std::is_integral<T>::value && !std::is_same<T, bool>::value>();
+  test_atomic<A, std::is_integral<T>::value && !std::is_same<T, bool>::value>();
 }
 
 struct TriviallyCopyable {
-    int i_;
+  int i_;
 };
 
-struct WeirdTriviallyCopyable
-{
-    char i, j, k; /* the 3 chars of doom */
+struct WeirdTriviallyCopyable {
+  char i, j, k; /* the 3 chars of doom */
 };
 
-struct PaddedTriviallyCopyable
-{
-    char i; int j; /* probably lock-free? */
+struct PaddedTriviallyCopyable {
+  char i;
+  int j; /* probably lock-free? */
 };
 
-struct LargeTriviallyCopyable
-{
-    int i, j[127]; /* decidedly not lock-free */
+struct LargeTriviallyCopyable {
+  int i, j[127]; /* decidedly not lock-free */
 };
 
-int main(int, char**)
-{
-    test<bool>               ();
-    test<char>               ();
-    test<signed char>        ();
-    test<unsigned char>      ();
-    test<short>              ();
-    test<unsigned short>     ();
-    test<int>                ();
-    test<unsigned int>       ();
-    test<long>               ();
-    test<unsigned long>      ();
-    test<long long>          ();
-    test<unsigned long long> ();
+int main(int, char**) {
+  test<bool>();
+  test<char>();
+  test<signed char>();
+  test<unsigned char>();
+  test<short>();
+  test<unsigned short>();
+  test<int>();
+  test<unsigned int>();
+  test<long>();
+  test<unsigned long>();
+  test<long long>();
+  test<unsigned long long>();
 #if TEST_STD_VER > 17 && defined(__cpp_char8_t)
-    test<char8_t>            ();
+  test<char8_t>();
 #endif
-    test<char16_t>           ();
-    test<char32_t>           ();
+  test<char16_t>();
+  test<char32_t>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
-    test<wchar_t>            ();
+  test<wchar_t>();
 #endif
 
-    test<std::int_least8_t>   ();
-    test<std::uint_least8_t>  ();
-    test<std::int_least16_t>  ();
-    test<std::uint_least16_t> ();
-    test<std::int_least32_t>  ();
-    test<std::uint_least32_t> ();
-    test<std::int_least64_t>  ();
-    test<std::uint_least64_t> ();
-
-    test<std::int_fast8_t>   ();
-    test<std::uint_fast8_t>  ();
-    test<std::int_fast16_t>  ();
-    test<std::uint_fast16_t> ();
-    test<std::int_fast32_t>  ();
-    test<std::uint_fast32_t> ();
-    test<std::int_fast64_t>  ();
-    test<std::uint_fast64_t> ();
-
-    test< std::int8_t>  ();
-    test<std::uint8_t>  ();
-    test< std::int16_t> ();
-    test<std::uint16_t> ();
-    test< std::int32_t> ();
-    test<std::uint32_t> ();
-    test< std::int64_t> ();
-    test<std::uint64_t> ();
-
-    test<std::intptr_t>  ();
-    test<std::uintptr_t> ();
-    test<std::size_t>    ();
-    test<std::ptrdiff_t> ();
-    test<std::intmax_t>  ();
-    test<std::uintmax_t> ();
-
-    test<std::uintmax_t> ();
-    test<std::uintmax_t> ();
-
-    test<TriviallyCopyable>();
-    test<PaddedTriviallyCopyable>();
+  test<std::int_least8_t>();
+  test<std::uint_least8_t>();
+  test<std::int_least16_t>();
+  test<std::uint_least16_t>();
+  test<std::int_least32_t>();
+  test<std::uint_least32_t>();
+  test<std::int_least64_t>();
+  test<std::uint_least64_t>();
+
+  test<std::int_fast8_t>();
+  test<std::uint_fast8_t>();
+  test<std::int_fast16_t>();
+  test<std::uint_fast16_t>();
+  test<std::int_fast32_t>();
+  test<std::uint_fast32_t>();
+  test<std::int_fast64_t>();
+  test<std::uint_fast64_t>();
+
+  test< std::int8_t>();
+  test<std::uint8_t>();
+  test< std::int16_t>();
+  test<std::uint16_t>();
+  test< std::int32_t>();
+  test<std::uint32_t>();
+  test< std::int64_t>();
+  test<std::uint64_t>();
+
+  test<std::intptr_t>();
+  test<std::uintptr_t>();
+  test<std::size_t>();
+  test<std::ptrdiff_t>();
+  test<std::intmax_t>();
+  test<std::uintmax_t>();
+
+  test<std::uintmax_t>();
+  test<std::uintmax_t>();
+
+  test<TriviallyCopyable>();
+  test<PaddedTriviallyCopyable>();
 #ifndef __APPLE__ // Apple doesn't ship libatomic
-    /*
+  /*
         These aren't going to be lock-free,
         so some libatomic.a is necessary.
     */
-    test<WeirdTriviallyCopyable>();
-    test<LargeTriviallyCopyable>();
+  test<WeirdTriviallyCopyable>();
+  test<LargeTriviallyCopyable>();
 #endif
 
 #ifndef TEST_HAS_NO_THREADS
-    test<std::thread::id>();
+  test<std::thread::id>();
 #endif
-    test<std::chrono::nanoseconds>();
-    test<float>();
+  test<std::chrono::nanoseconds>();
+  test<float>();
 
 #if TEST_STD_VER >= 20
-    test<std::atomic_signed_lock_free::value_type>();
-    static_assert(std::is_signed_v<std::atomic_signed_lock_free::value_type>);
-    static_assert(std::is_integral_v<std::atomic_signed_lock_free::value_type>);
+  test<std::atomic_signed_lock_free::value_type>();
+  static_assert(std::is_signed_v<std::atomic_signed_lock_free::value_type>);
+  static_assert(std::is_integral_v<std::atomic_signed_lock_free::value_type>);
 
-    test<std::atomic_unsigned_lock_free::value_type>();
-    static_assert(std::is_unsigned_v<std::atomic_unsigned_lock_free::value_type>);
-    static_assert(std::is_integral_v<std::atomic_unsigned_lock_free::value_type>);
+  test<std::atomic_unsigned_lock_free::value_type>();
+  static_assert(std::is_unsigned_v<std::atomic_unsigned_lock_free::value_type>);
+  static_assert(std::is_integral_v<std::atomic_unsigned_lock_free::value_type>);
 /*
     test<std::shared_ptr<int>>();
 */
 #endif
 
-    return 0;
+  return 0;
 }

>From 857e15f045a6f146e42e40e5bbd887d06e34a22d Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Thu, 16 Jan 2025 15:12:58 -0500
Subject: [PATCH 2/5] Fix tests for std::atomic member types

---
 libcxx/test/std/atomics/types.pass.cpp | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/libcxx/test/std/atomics/types.pass.cpp b/libcxx/test/std/atomics/types.pass.cpp
index e07a5bf5fe17b7..dd8f770ac45434 100644
--- a/libcxx/test/std/atomics/types.pass.cpp
+++ b/libcxx/test/std/atomics/types.pass.cpp
@@ -29,19 +29,29 @@
 #  include <thread>
 #endif
 
-template <class A, bool Integral>
+// detect existence of the difference_type member type
+template <class...>
+using myvoid_t = void;
+template <typename T, typename = void>
+struct has_difference_type : std::false_type {};
+template <typename T>
+struct has_difference_type<T, myvoid_t<typename T::difference_type> > : std::true_type {};
+
+template <class A, bool IntegralOrFloating, bool Pointer>
 struct test_atomic {
   test_atomic() {
+    static_assert(!IntegralOrFloating || !Pointer, "");
     A a;
     (void)a;
 #if TEST_STD_VER >= 17
     static_assert((std::is_same_v<typename A::value_type, decltype(a.load())>), "");
+    static_assert(!has_difference_type<A>::value, "");
 #endif
   }
 };
 
 template <class A>
-struct test_atomic<A, true> {
+struct test_atomic<A, true, false> {
   test_atomic() {
     A a;
     (void)a;
@@ -53,7 +63,7 @@ struct test_atomic<A, true> {
 };
 
 template <class A>
-struct test_atomic<A*, false> {
+struct test_atomic<A, false, true> {
   test_atomic() {
     A a;
     (void)a;
@@ -70,7 +80,10 @@ void test() {
 #if TEST_STD_VER >= 17
   static_assert((std::is_same_v<typename A::value_type, T>), "");
 #endif
-  test_atomic<A, std::is_integral<T>::value && !std::is_same<T, bool>::value>();
+  constexpr bool IntegralOrFloating =
+      (std::is_integral<T>::value && !std::is_same<T, bool>::value) || std::is_floating_point<T>::value;
+  constexpr bool Pointer = std::is_pointer<T>::value;
+  test_atomic<A, IntegralOrFloating, Pointer>();
 }
 
 struct TriviallyCopyable {
@@ -149,6 +162,11 @@ int main(int, char**) {
   test<std::uintmax_t>();
   test<std::uintmax_t>();
 
+  test<void*>();
+  test<const void*>();
+  test<int*>();
+  test<const int*>();
+
   test<TriviallyCopyable>();
   test<PaddedTriviallyCopyable>();
 #ifndef __APPLE__ // Apple doesn't ship libatomic

>From c1ea96634d166b13d7ac6ecc4ec68a02d304be59 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Thu, 16 Jan 2025 15:13:11 -0500
Subject: [PATCH 3/5] Fix std::atomic primary template should not have
 difference_type

---
 libcxx/include/__atomic/atomic.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h
index 975a479e204004..1ed206ab2ff197 100644
--- a/libcxx/include/__atomic/atomic.h
+++ b/libcxx/include/__atomic/atomic.h
@@ -40,6 +40,8 @@ struct __atomic_base // false
 {
   mutable __cxx_atomic_impl<_Tp> __a_;
 
+  using value_type = _Tp;
+
 #if _LIBCPP_STD_VER >= 17
   static constexpr bool is_always_lock_free = __libcpp_is_always_lock_free<__cxx_atomic_impl<_Tp> >::__value;
 #endif
@@ -145,6 +147,8 @@ template <class _Tp>
 struct __atomic_base<_Tp, true> : public __atomic_base<_Tp, false> {
   using __base _LIBCPP_NODEBUG = __atomic_base<_Tp, false>;
 
+  using difference_type = __base::value_type;
+
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __atomic_base() _NOEXCEPT = default;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : __base(__d) {}
@@ -229,8 +233,6 @@ struct __atomic_waitable_traits<__atomic_base<_Tp, _IsIntegral> > {
 template <class _Tp>
 struct atomic : public __atomic_base<_Tp> {
   using __base _LIBCPP_NODEBUG = __atomic_base<_Tp>;
-  using value_type             = _Tp;
-  using difference_type        = value_type;
 
 #if _LIBCPP_STD_VER >= 20
   _LIBCPP_HIDE_FROM_ABI atomic() = default;
@@ -258,8 +260,8 @@ struct atomic : public __atomic_base<_Tp> {
 template <class _Tp>
 struct atomic<_Tp*> : public __atomic_base<_Tp*> {
   using __base _LIBCPP_NODEBUG = __atomic_base<_Tp*>;
-  using value_type             = _Tp*;
-  using difference_type        = ptrdiff_t;
+
+  using difference_type = ptrdiff_t;
 
   _LIBCPP_HIDE_FROM_ABI atomic() _NOEXCEPT = default;
 

>From af4d5f53a535406ec9842392c3c4ab9e596e2936 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Thu, 16 Jan 2025 16:35:40 -0500
Subject: [PATCH 4/5] Update expected error for
 atomic_fetch_{add,sub}[_explicit] with pointers to member function

---
 .../atomics.types.operations.req/atomic_fetch_add.verify.cpp  | 4 ++--
 .../atomic_fetch_add_explicit.verify.cpp                      | 4 ++--
 .../atomics.types.operations.req/atomic_fetch_sub.verify.cpp  | 4 ++--
 .../atomic_fetch_sub_explicit.verify.cpp                      | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.verify.cpp b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.verify.cpp
index fbe7dbbd6d9f04..f114efe4e64da4 100644
--- a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.verify.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.verify.cpp
@@ -63,12 +63,12 @@ struct S {
 void member_function_pointer() {
   {
     volatile std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_add' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_add'}}
     std::atomic_fetch_add(&fun, 0);
   }
   {
     std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_add' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_add'}}
     std::atomic_fetch_add(&fun, 0);
   }
 }
diff --git a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add_explicit.verify.cpp b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add_explicit.verify.cpp
index 176a38c7c0f7f1..04e9c458321df2 100644
--- a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add_explicit.verify.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add_explicit.verify.cpp
@@ -66,12 +66,12 @@ struct S {
 void member_function_pointer() {
   {
     volatile std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_add' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_add_explicit'}}
     std::atomic_fetch_add_explicit(&fun, 0, std::memory_order_relaxed);
   }
   {
     std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_add' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_add_explicit'}}
     std::atomic_fetch_add_explicit(&fun, 0, std::memory_order_relaxed);
   }
 }
diff --git a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub.verify.cpp b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub.verify.cpp
index 6f3039dab66974..dd95c232737337 100644
--- a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub.verify.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub.verify.cpp
@@ -63,12 +63,12 @@ struct S {
 void member_function_pointer() {
   {
     volatile std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_sub' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_sub'}}
     std::atomic_fetch_sub(&fun, 0);
   }
   {
     std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_sub' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_sub'}}
     std::atomic_fetch_sub(&fun, 0);
   }
 }
diff --git a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp
index 1b3c58fa87551c..b4d4f0a1fc70dd 100644
--- a/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp
@@ -66,12 +66,12 @@ struct S {
 void member_function_pointer() {
   {
     volatile std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_sub' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_sub_explicit'}}
     std::atomic_fetch_sub_explicit(&fun, 0, std::memory_order_relaxed);
   }
   {
     std::atomic<void (S::*)(int)> fun;
-    // expected-error@*:* {{no member named 'fetch_sub' in}}
+    // expected-error@*:* {{no matching function for call to 'atomic_fetch_sub_explicit'}}
     std::atomic_fetch_sub_explicit(&fun, 0, std::memory_order_relaxed);
   }
 }

>From 95a84d452dd29ea47ae44b09e1b5abe31d550e82 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Thu, 16 Jan 2025 16:47:05 -0500
Subject: [PATCH 5/5] Add test with function pointer

---
 libcxx/test/std/atomics/types.pass.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libcxx/test/std/atomics/types.pass.cpp b/libcxx/test/std/atomics/types.pass.cpp
index dd8f770ac45434..ef01e792cf0079 100644
--- a/libcxx/test/std/atomics/types.pass.cpp
+++ b/libcxx/test/std/atomics/types.pass.cpp
@@ -162,6 +162,7 @@ int main(int, char**) {
   test<std::uintmax_t>();
   test<std::uintmax_t>();
 
+  test<void (*)(int)>();
   test<void*>();
   test<const void*>();
   test<int*>();



More information about the libcxx-commits mailing list