Hello,Following the outcome of the previous discussion, the attached revised patch implements the proposed resolution to LWG 4169, moved to WP in Wrocław.
https://cplusplus.github.io/LWG/issue4169 Thanks, -- Giuseppe D'Angelo
From 3dc64d830c61a16c2c4c0722a3983054aad1b04e 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++: constrain std::atomic's default constructor
This commit implements the proposed resolution to LWG4169, which is
to constrain std::atomic<T>'s default constructor based on whether
T itself is default constructible.
At the moment, 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()).
GCC already considers the defaulted constructor constrained/deleted,
however this behavior is non-standard (see the discussion in PR116769):
the presence of a NSDMI should not make the constructor unavailable to
overload resolution/deleted ([class.default.ctor]/2.5 does not apply).
When using libstdc++ on Clang, this causes build issues as the
constructor is *not* deleted there -- the interpretation of
[class.default.ctor]/4 seems to match Clang's behavior.
Therefore, although there would be "nothing to do" with GCC+libstdc++,
this commit changes the code as to stop relying on the GCC language
extension. std::atomic's defaulted default constructor is changed to be
a non-defaulted one, with a constraint added as per LWG4169;
value-initialization of the data member is moved from the NSDMI to the
member init list. The new signature matches the one in the Standard as
per [atomics.types.operations]/1.
A test that explictly instantiated std::atomic with a non-default
constructible type needed to be amended. Such an instantiation would
instantiate all member functions (as per [temp.explicit]/9), including
the default constructor, which now triggers a hard error when built in
pre-C++20 modes.
In its place I've instead added a couple of static_asserts checking that
std::atomic is default constructible even with a non-default
constructible T (in pre-C++20 modes), or is not default constructible
at all (since C++20).
libstdc++-v3/ChangeLog:
* include/std/atomic: Turn the defaulted default constructor
in a non-defaulted one, constraining it as per LWG4169;
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: Restrict the explicit
class instantation only to C++ >= 20 modes; it would otherwise
cause a hard error. Add a new test.
* testsuite/29_atomics/atomic/cons/value_init.cc: Add a new test.
Signed-off-by: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
---
libstdc++-v3/include/std/atomic | 20 +++++++++----------
.../testsuite/29_atomics/atomic/69301.cc | 8 ++++++++
.../29_atomics/atomic/cons/value_init.cc | 8 ++++++++
3 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index c0568d3320b..503dca945d3 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -51,6 +51,7 @@
#include <bits/atomic_base.h>
#include <cstdint>
+#include <type_traits>
namespace std _GLIBCXX_VISIBILITY(default)
{
@@ -189,14 +190,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.
*
@@ -216,7 +209,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");
@@ -232,7 +225,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif
public:
- atomic() = default;
+ constexpr atomic() noexcept(is_nothrow_default_constructible<_Tp>::value)
+#if __cpp_lib_atomic_value_initialization
+ requires is_default_constructible<_Tp>::value
+ : _M_i()
+#endif
+ {}
+
~atomic() noexcept = default;
atomic(const atomic&) = delete;
atomic& operator=(const atomic&) = delete;
@@ -414,7 +413,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..005e7d53939 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc
@@ -27,7 +27,15 @@ struct NonDefaultConstructible
int val;
};
+#if __cpp_lib_atomic_value_initialization // C++ >= 20
template class std::atomic<NonDefaultConstructible>;
+static_assert(!std::is_default_constructible_v<std::atomic<NonDefaultConstructible>>);
+#else
+static_assert(
+ std::is_default_constructible<std::atomic<NonDefaultConstructible>>::value,
+ "Before C++20 / LWG4169 std::atomic should be default constructible"
+);
+#endif
void
test01()
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/cons/value_init.cc b/libstdc++-v3/testsuite/29_atomics/atomic/cons/value_init.cc
index 82648e4247b..9a508099b39 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/cons/value_init.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/cons/value_init.cc
@@ -68,6 +68,14 @@ test03()
static_assert(std::is_nothrow_default_constructible_v<std::atomic<int*>>);
}
+// LWG4169
+struct NDC
+{
+ constexpr NDC(int) {}
+};
+
+static_assert(!std::is_default_constructible_v<std::atomic<NDC>>);
+
int
main()
{
--
2.34.1
smime.p7s
Description: S/MIME Cryptographic Signature
