[libc++] Refactor tests for trivially copyable atomics

[ldionne]

Summary

• Replace irrelevant synopsis by a comment
• Use a .verify.cpp test instead of .compile.fail.cpp
• Remove unnecessary includes in one of the tests (was a copy-paste error)

[philnik]

libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.pass.cpp

30

Could you fix the indentation in this patch as well?

[• Quuxplusone]

libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.pass.cpp

27

Bonus nit here and in the new test: explicit constructors :)

libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp

20

Consider exactly copy-pasting the comment wording from libcxx/test/std/utilities/any/any.nonmembers/any.cast/const_correctness.verify.cpp, just for hypothetical greppability.

[ldionne]

Will apply comments when committing.

libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp

20

Unfortunately, that comment is specific to any_cast, so it wouldn't apply here. However, for greppability, one could just look for ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error.

libcxx/include/atomic

template <typename _Tp,
                                                 __cxx_atomic_base_impl<_Tp>,
                                                 __cxx_atomic_lock_impl<_Tp> >::type>
 #else
 template <typename _Tp,
           typename _Base = __cxx_atomic_base_impl<_Tp> >
 #endif //_LIBCPP_ATOMIC_ONLY_USE_BUILTINS
 struct __cxx_atomic_impl : public _Base {
     static_assert(is_trivially_copyable<_Tp>::value,
-      "std::atomic<Tp> requires that 'Tp' be a trivially copyable type");
+      "std::atomic<T> requires that 'T' be a trivially copyable type");
 
   _LIBCPP_INLINE_VISIBILITY __cxx_atomic_impl() _NOEXCEPT _LIBCPP_DEFAULT
   _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR explicit __cxx_atomic_impl(_Tp value) _NOEXCEPT
     : _Base(value) {}
 };
 
 #ifdef __linux__
     using __cxx_contention_t = int32_t;

libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.compile.fail.cpp

-//===----------------------------------------------------------------------===//
-//
-// 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
-// {
-//     bool is_lock_free() const volatile noexcept;
-//     bool is_lock_free() const noexcept;
-//     void store(T desr, memory_order m = memory_order_seq_cst) volatile noexcept;
-//     void store(T desr, memory_order m = memory_order_seq_cst) noexcept;
-//     T load(memory_order m = memory_order_seq_cst) const volatile noexcept;
-//     T load(memory_order m = memory_order_seq_cst) const noexcept;
-//     operator T() const volatile noexcept;
-//     operator T() const noexcept;
-//     T exchange(T desr, memory_order m = memory_order_seq_cst) volatile noexcept;
-//     T exchange(T desr, memory_order m = memory_order_seq_cst) noexcept;
-//     bool compare_exchange_weak(T& expc, T desr,
-//                                memory_order s, memory_order f) volatile noexcept;
-//     bool compare_exchange_weak(T& expc, T desr, memory_order s, memory_order f) noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                  memory_order s, memory_order f) volatile noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                  memory_order s, memory_order f) noexcept;
-//     bool compare_exchange_weak(T& expc, T desr,
-//                                memory_order m = memory_order_seq_cst) volatile noexcept;
-//     bool compare_exchange_weak(T& expc, T desr,
-//                                memory_order m = memory_order_seq_cst) noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                 memory_order m = memory_order_seq_cst) volatile noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                  memory_order m = memory_order_seq_cst) noexcept;
-//
-//     atomic() noexcept = default;
-//     constexpr atomic(T desr) noexcept;
-//     atomic(const atomic&) = delete;
-//     atomic& operator=(const atomic&) = delete;
-//     atomic& operator=(const atomic&) volatile = delete;
-//     T operator=(T) volatile noexcept;
-//     T operator=(T) noexcept;
-// };
-
-#include <atomic>
-#include <new>
-#include <cassert>
-#include <thread> // for thread_id
-#include <chrono> // for nanoseconds
-
-struct NotTriviallyCopyable {
-    NotTriviallyCopyable ( int i ) : i_(i) {}
-    NotTriviallyCopyable ( const NotTriviallyCopyable &rhs) : i_(rhs.i_) {}
-    int i_;
-};
-
-template <class T, class >
-void test ( T t ) {
-    std::atomic<T> t0(t);
-}
-
-int main(int, char**)
-{
-    test(NotTriviallyCopyable(42));
-
-  return 0;
-}

libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
 
 // <atomic>
 
 // template <class T>
-// struct atomic
-// {
-//     bool is_lock_free() const volatile noexcept;
-//     bool is_lock_free() const noexcept;
-//     void store(T desr, memory_order m = memory_order_seq_cst) volatile noexcept;
-//     void store(T desr, memory_order m = memory_order_seq_cst) noexcept;
-//     T load(memory_order m = memory_order_seq_cst) const volatile noexcept;
-//     T load(memory_order m = memory_order_seq_cst) const noexcept;
-//     operator T() const volatile noexcept;
-//     operator T() const noexcept;
-//     T exchange(T desr, memory_order m = memory_order_seq_cst) volatile noexcept;
-//     T exchange(T desr, memory_order m = memory_order_seq_cst) noexcept;
-//     bool compare_exchange_weak(T& expc, T desr,
-//                                memory_order s, memory_order f) volatile noexcept;
-//     bool compare_exchange_weak(T& expc, T desr, memory_order s, memory_order f) noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                  memory_order s, memory_order f) volatile noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                  memory_order s, memory_order f) noexcept;
-//     bool compare_exchange_weak(T& expc, T desr,
-//                                memory_order m = memory_order_seq_cst) volatile noexcept;
-//     bool compare_exchange_weak(T& expc, T desr,
-//                                memory_order m = memory_order_seq_cst) noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                 memory_order m = memory_order_seq_cst) volatile noexcept;
-//     bool compare_exchange_strong(T& expc, T desr,
-//                                  memory_order m = memory_order_seq_cst) noexcept;
-//
-//     atomic() noexcept = default;
-//     constexpr atomic(T desr) noexcept;
-//     atomic(const atomic&) = delete;
-//     atomic& operator=(const atomic&) = delete;
-//     atomic& operator=(const atomic&) volatile = delete;
-//     T operator=(T) volatile noexcept;
-//     T operator=(T) noexcept;
-// };
+// struct atomic;
+
+// Make sure atomic<TriviallyCopyable> can be instantiated.
 
 #include <atomic>
 #include <new>
 #include <cassert>
 #include <thread> // for thread_id
 #include <chrono> // for nanoseconds
 
 #include "test_macros.h"
 
 struct TriviallyCopyable {
-    TriviallyCopyable ( int i ) : i_(i) {}
+    TriviallyCopyable(int i) : i_(i) { }
     int i_;
-    };
 
 template <class T>
-void test ( T t ) {
+void test(T t) {
     std::atomic<T> t0(t);
-    }
 
-int main(int, char**)
-{
+int main(int, char**) {
     test(TriviallyCopyable(42));
     test(std::this_thread::get_id());
     test(std::chrono::nanoseconds(2));
 
     return 0;
 }

libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: libcpp-has-no-threads
+
+// <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 {
+    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@atomic:* {{std::atomic<T> requires that 'T' be a trivially copyable type}}
+}