Hello,The attached patch modifies std::atomic's primary template. The goal is to improve compatibility with Clang, while also possibly making it more complaint with the changes introduced by P0883 / C++20.
Simplifying, std::atomic has a `T t = T()` NDSMI and a defaulted default constructor. The crux of the problem is that Clang seems to be stricter than GCC when that constructor is considered / instantiated.
Given a non-default constructible type NDC, doing something like constexpr bool b = std::is_default_constructible_v<std::atomic<NDC>>;causes a hard error on Clang because it will "see" the call to `NDC()` in the NDSMI. The code is instead accepted by GCC. This hard error will happen anywhere one "mentions" std::atomic<NDC>'s default constructor, for instance in libstdc++'s C++20 std::pair implementation (uses them in the explicit(bool) bits). You can play with this here:
https://gcc.godbolt.org/z/xcr4zK8hxPR116769 argues that Clang's behavior is the correct one here, so this patch improves compat with Clang by removing the defaulted default constructor.
A related issue is: what's the value of `b` above? std::atomic's default constructor is not constrained, so it should be `true`. Right now we're reporting `false` instead.
Thoughts? Thank you, -- Giuseppe D'Angelo
From 274543f82ac4e77aaf9c8f5158a44b98538537e5 Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
Date: Sat, 21 Sep 2024 10:36:20 +0200
Subject: [PATCH] libstdc++: improve std::atomic compatibility with Clang
std::atomic<T>'s primary template in libstdc++ has a defaulted default
constructor. Value-initialization of the T member (since C++20 /
P0883R2) is done via a NSDMI (= T()).
There is implementation divergence on the behavior of the defaulted
default constructor in case T isn't default constructible. In particular
GCC seems to effectively treat the default constructor as deleted /
constrained away; whereas Clang triggers a hard error due to the NDSMI.
It seems that GCC's behavior is a non-standard extension here, see the
discussion in PR116769; the presence of a NDSMI shouldn't make the
constructor deleted ([class.default.ctor]/2.5 does not apply) and the
interpretation of [class.default.ctor]/4 seems to match Clang's behavior
instead.
Therefore, this commit removes std::atomic's defaulted default
constructor and changes it to a non-defaulted one; value-initialization
of the data member is moved from the NDSMI to the member init list.
The new signature matches the one in the Standard as per
[atomics.types.operations]/1.
Finally: std::atomic<T>'s default constructor is not constrained on T
being default constructible; it instead Mandates that condition. I'm
not sure why that's the case, but this requires removing a testcase
where there's an explicit class instantiation of std::atomic with a
non-default constructible type. Such an instantiation would instantiate
all member functions (as per [temp.explicit]/9), including the default
constructor, which now triggers an hard error.
In its place I've instead added a static_assert checking that
std::atomic is default constructible even with a non-default
constructible T.
libstdc++-v3/ChangeLog:
* include/std/atomic: Turn the defaulted default constructor
in a non-defaulted one; remove the NSDMI for the _M_i member.
Drop the _GLIBCXX20_INIT macro as it is not needed any more.
* testsuite/29_atomics/atomic/69301.cc: Remove the explicit
class instantation, which would cause a hard error. Add a new
test.
Signed-off-by: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
---
libstdc++-v3/include/std/atomic | 18 +++++++-----------
.../testsuite/29_atomics/atomic/69301.cc | 6 +++++-
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index 36ff89a146c..85b25ec4e8d 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -187,14 +187,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif // __cpp_lib_atomic_wait
};
-/// @cond undocumented
-#if __cpp_lib_atomic_value_initialization
-# define _GLIBCXX20_INIT(I) = I
-#else
-# define _GLIBCXX20_INIT(I)
-#endif
-/// @endcond
-
/**
* @brief Generic atomic type, primary class template.
*
@@ -214,7 +206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static constexpr int _S_alignment
= _S_min_alignment > alignof(_Tp) ? _S_min_alignment : alignof(_Tp);
- alignas(_S_alignment) _Tp _M_i _GLIBCXX20_INIT(_Tp());
+ alignas(_S_alignment) _Tp _M_i;
static_assert(__is_trivially_copyable(_Tp),
"std::atomic requires a trivially copyable type");
@@ -230,7 +222,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif
public:
- atomic() = default;
+ constexpr atomic() noexcept(__is_nothrow_constructible(_Tp))
+#if __cpp_lib_atomic_value_initialization
+ : _M_i()
+#endif
+ {}
+
~atomic() noexcept = default;
atomic(const atomic&) = delete;
atomic& operator=(const atomic&) = delete;
@@ -412,7 +409,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif // __cpp_lib_atomic_wait
};
-#undef _GLIBCXX20_INIT
/// Partial specialization for pointer types.
template<typename _Tp>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc b/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc
index 875e3344a2c..1007c2a4da5 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc
@@ -27,7 +27,11 @@ struct NonDefaultConstructible
int val;
};
-template class std::atomic<NonDefaultConstructible>;
+static_assert(
+ std::is_default_constructible<std::atomic<NonDefaultConstructible>>::value,
+ "std::atomic should always default constructible as its default "
+ "constructor is not constrained"
+);
void
test01()
--
2.34.1
smime.p7s
Description: S/MIME Cryptographic Signature
