[libcxx-commits] [libcxx] [libc++] Improve diagnostic when violating `std::atomic` trivially copyable mandates (PR #131754)

[Damien L-G via libcxx-commits]

https://github.com/dalg24 created https://github.com/llvm/llvm-project/pull/131754

When attempting to instantiate `std::atomic` with a non trivially copyable type, one gets errors from instantiating internals before the actual static assertion that check the template parameter type requirements.

The `verify` test for it had a `// ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error` directive to work around this issue.  The changes I propose enable us to drop that directive.
As I understand it, the `verify` test was misplaced so I moved it to `test/{std -> libcxx}/atomics`.

(I ran into this while working on #121414 in which we would add another static assertion in `__check_atomic_mandates`)

>From 67baf5e4b4648c62757557420ca401a7ac644abc Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Tue, 18 Mar 2025 08:51:36 +0100
Subject: [PATCH 1/4] Apply clang-format

---
 .../atomics.types.generic/trivially_copyable.verify.cpp    | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp b/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
index 0955707cdcf38..132c969668f24 100644
--- a/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
+++ b/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
@@ -20,12 +20,13 @@
 #include <atomic>
 
 struct NotTriviallyCopyable {
-  explicit NotTriviallyCopyable(int i) : i_(i) { }
-  NotTriviallyCopyable(const NotTriviallyCopyable &rhs) : i_(rhs.i_) { }
+  explicit NotTriviallyCopyable(int i) : i_(i) {}
+  NotTriviallyCopyable(const NotTriviallyCopyable& rhs) : i_(rhs.i_) {}
   int i_;
 };
 
 void f() {
   NotTriviallyCopyable x(42);
-  std::atomic<NotTriviallyCopyable> a(x); // expected-error@*:* {{std::atomic<T> requires that 'T' be a trivially copyable type}}
+  std::atomic<NotTriviallyCopyable> a(
+      x); // expected-error@*:* {{std::atomic<T> requires that 'T' be a trivially copyable type}}
 }

>From 06a40cfcdb89131ca4afde47520e690842218303 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Tue, 18 Mar 2025 08:53:41 +0100
Subject: [PATCH 2/4] Do not silent additional errors from instantiating
 std::atomic

---
 .../atomics.types.generic/trivially_copyable.verify.cpp     | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp b/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
index 132c969668f24..c869e1fe76841 100644
--- a/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
+++ b/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
@@ -11,12 +11,6 @@
 // template <class T>
 // struct atomic;
 
-// This test checks that we static_assert inside std::atomic<T> when T
-// is not trivially copyable, however Clang will sometimes emit additional
-// errors while trying to instantiate the rest of std::atomic<T>.
-// We silence those to make the test more robust.
-// ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error
-
 #include <atomic>
 
 struct NotTriviallyCopyable {

>From a3e63b11a57afb0c86fdeeaaa4b706c7a4436143 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Tue, 18 Mar 2025 09:00:32 +0100
Subject: [PATCH 3/4] Make sure that std:atomic mandates are checked first

---
 libcxx/include/__atomic/support.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__atomic/support.h b/libcxx/include/__atomic/support.h
index 4b555ab483ca0..232ee23ea1db2 100644
--- a/libcxx/include/__atomic/support.h
+++ b/libcxx/include/__atomic/support.h
@@ -111,10 +111,14 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <typename _Tp, typename _Base = __cxx_atomic_base_impl<_Tp> >
-struct __cxx_atomic_impl : public _Base {
+template <typename _Tp>
+struct __check_atomic_mandates {
+  using type = _Tp;
   static_assert(is_trivially_copyable<_Tp>::value, "std::atomic<T> requires that 'T' be a trivially copyable type");
+};
 
+template <typename _Tp, typename _Base = __cxx_atomic_base_impl<typename __check_atomic_mandates<_Tp>::type> >
+struct __cxx_atomic_impl : public _Base {
   _LIBCPP_HIDE_FROM_ABI __cxx_atomic_impl() _NOEXCEPT = default;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR explicit __cxx_atomic_impl(_Tp __value) _NOEXCEPT : _Base(__value) {}
 };

>From d5796458ab25a956eb4230fff95f290705f91bd6 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Tue, 18 Mar 2025 09:03:36 +0100
Subject: [PATCH 4/4] Move misplaced verify test std -> libcxx

---
 .../atomics/atomics.types.generic/trivially_copyable.verify.cpp   | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename libcxx/test/{std => libcxx}/atomics/atomics.types.generic/trivially_copyable.verify.cpp (100%)

diff --git a/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp b/libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp
similarity index 100%
rename from libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
rename to libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp