[libcxx-commits] [libcxx] [libc++] Improve diagnostic when violating `std::atomic` trivially copyable mandates (PR #131754)
Damien L-G via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Apr 2 19:13:34 PDT 2025
https://github.com/dalg24 updated https://github.com/llvm/llvm-project/pull/131754
>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/8] 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/8] 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/8] 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/8] 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
>From 41150c80ac0da10149b8e72e9ff7087b0ce5ad5f Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Tue, 18 Mar 2025 09:37:01 +0100
Subject: [PATCH 5/8] Move mandates check __atomic/{support.h -> atomic.h}
---
libcxx/include/__atomic/atomic.h | 8 +++++++-
libcxx/include/__atomic/support.h | 8 +-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h
index c65f9afe4f390..07cd0a5d936f4 100644
--- a/libcxx/include/__atomic/atomic.h
+++ b/libcxx/include/__atomic/atomic.h
@@ -230,8 +230,14 @@ struct __atomic_waitable_traits<__atomic_base<_Tp, _IsIntegral> > {
}
};
+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 <class _Tp>
-struct atomic : public __atomic_base<_Tp> {
+struct atomic : public __atomic_base<typename __check_atomic_mandates<_Tp>::type> {
using __base _LIBCPP_NODEBUG = __atomic_base<_Tp>;
#if _LIBCPP_STD_VER >= 20
diff --git a/libcxx/include/__atomic/support.h b/libcxx/include/__atomic/support.h
index 232ee23ea1db2..ca9273b5b7249 100644
--- a/libcxx/include/__atomic/support.h
+++ b/libcxx/include/__atomic/support.h
@@ -111,13 +111,7 @@
_LIBCPP_BEGIN_NAMESPACE_STD
-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> >
+template <typename _Tp, typename _Base = __cxx_atomic_base_impl<_Tp> >
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 cb91be24e4e7525c7a5e6c14eea53d2a99f7246b Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Tue, 18 Mar 2025 09:39:45 +0100
Subject: [PATCH 6/8] Add `// XFAIL: FROZEN-CXX03-HEADERS-FIXME` directive in
verify test
---
.../atomics/atomics.types.generic/trivially_copyable.verify.cpp | 2 ++
1 file changed, 2 insertions(+)
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
index c869e1fe76841..452e65d569230 100644
--- a/libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp
@@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
// <atomic>
// template <class T>
>From 1c9de5a4071092d974bd4685967fc6c0c5c252b2 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Tue, 18 Mar 2025 10:34:15 +0100
Subject: [PATCH 7/8] Fixup is_trivially_copyable header
---
libcxx/include/__atomic/atomic.h | 1 +
libcxx/include/__atomic/support.h | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h
index 07cd0a5d936f4..c630ccc72ed47 100644
--- a/libcxx/include/__atomic/atomic.h
+++ b/libcxx/include/__atomic/atomic.h
@@ -23,6 +23,7 @@
#include <__type_traits/is_integral.h>
#include <__type_traits/is_nothrow_constructible.h>
#include <__type_traits/is_same.h>
+#include <__type_traits/is_trivially_copyable.h>
#include <__type_traits/remove_const.h>
#include <__type_traits/remove_pointer.h>
#include <__type_traits/remove_volatile.h>
diff --git a/libcxx/include/__atomic/support.h b/libcxx/include/__atomic/support.h
index ca9273b5b7249..99d0f6aa543ca 100644
--- a/libcxx/include/__atomic/support.h
+++ b/libcxx/include/__atomic/support.h
@@ -10,7 +10,6 @@
#define _LIBCPP___ATOMIC_SUPPORT_H
#include <__config>
-#include <__type_traits/is_trivially_copyable.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
>From 3e564f133c0b495d681394d68d70af7d8efd5c60 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Mon, 31 Mar 2025 16:04:21 -0400
Subject: [PATCH 8/8] Per review move the test back to libcxx/test/std
---
.../atomics/atomics.types.generic/trivially_copyable.verify.cpp | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename libcxx/test/{libcxx => std}/atomics/atomics.types.generic/trivially_copyable.verify.cpp (100%)
diff --git a/libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp b/libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
similarity index 100%
rename from libcxx/test/libcxx/atomics/atomics.types.generic/trivially_copyable.verify.cpp
rename to libcxx/test/std/atomics/atomics.types.generic/trivially_copyable.verify.cpp
More information about the libcxx-commits
mailing list