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

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Damien L-G (dalg24)

<details>
<summary>Changes</summary>

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`)

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


3 Files Affected:

- (modified) libcxx/include/__atomic/support.h (+6-2) 
- (added) libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp (+26) 
- (removed) libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp (-31) 


``````````diff
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) {}
 };
diff --git a/libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp b/libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp
new file mode 100644
index 0000000000000..c869e1fe76841
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp
@@ -0,0 +1,26 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <atomic>
+
+// template <class T>
+// struct atomic;
+
+#include <atomic>
+
+struct NotTriviallyCopyable {
+  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}}
+}
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
deleted file mode 100644
index 0955707cdcf38..0000000000000
--- a/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
+++ /dev/null
@@ -1,31 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// <atomic>
-
-// 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 {
-  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}}
-}

``````````

</details>


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