Hello,

On 03/12/2024 16:53, Giuseppe D'Angelo wrote:
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,

An amended patch (attached) has now been pushed to trunk as
r15-7908-g613f8ddbe3d7da.


Thank you,

--
Giuseppe D'Angelo
From 613f8ddbe3d7da63d827e588bf0333813c184b8a 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. In C++ >= 20 modes, 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.

In pre-C++20 modes, the constructor is left defaulted. This ensures
compatibility with C++11/14/17 behavior. In other words: we are not
backporting P0883R2 to earlier language modes here.

Amend an existing test to check that a std::atomic wrapping a
non-default constructible type is always non-default constructible:
from C++20, because of the constraint; before C++20, because we
are removing the NSDMI, and therefore [class.default.ctor]/2.5
applies.

Add another test that checks that std::atomic is trivially default
constructible in pre-C++20 modes, and it isn't afterwards.

libstdc++-v3/ChangeLog:

	* include/bits/version.def (atomic_value_initialization):
	Guard the FTM with the language concepts FTM.
	* include/bits/version.h: Regenerate.
	* include/std/atomic (atomic): When atomic value init is
	defined, change the defaulted default constructor to
	a non-defaulted one, constraining it as per LWG4169.
	Otherwise, keep the existing constructor.
	Remove the NSDMI for the _M_i member.
	(_GLIBCXX20_INIT): Drop the macro, as it is not needed any more.
	* testsuite/29_atomics/atomic/69301.cc: Test that
	an atomic wrapping a non-default-constructible type is
	always itself non-default-constructible (in all language
	modes).
	* testsuite/29_atomics/atomic/cons/trivial.cc: New test.

Signed-off-by: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
---
 libstdc++-v3/include/bits/version.def         |  1 +
 libstdc++-v3/include/bits/version.h           |  2 +-
 libstdc++-v3/include/std/atomic               | 22 +++++-----
 .../testsuite/29_atomics/atomic/69301.cc      |  2 +
 .../29_atomics/atomic/cons/trivial.cc         | 41 +++++++++++++++++++
 5 files changed, 57 insertions(+), 11 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/29_atomics/atomic/cons/trivial.cc

diff --git a/libstdc++-v3/include/bits/version.def b/libstdc++-v3/include/bits/version.def
index 2af5a54bff2..56638759539 100644
--- a/libstdc++-v3/include/bits/version.def
+++ b/libstdc++-v3/include/bits/version.def
@@ -770,6 +770,7 @@ ftms = {
   values = {
     v = 201911;
     cxxmin = 20;
+    extra_cond = "__cpp_concepts >= 201907L";
   };
 };
 
diff --git a/libstdc++-v3/include/bits/version.h b/libstdc++-v3/include/bits/version.h
index 9833023cfdc..29e1535298c 100644
--- a/libstdc++-v3/include/bits/version.h
+++ b/libstdc++-v3/include/bits/version.h
@@ -856,7 +856,7 @@
 #undef __glibcxx_want_atomic_ref
 
 #if !defined(__cpp_lib_atomic_value_initialization)
-# if (__cplusplus >= 202002L)
+# if (__cplusplus >= 202002L) && (__cpp_concepts >= 201907L)
 #  define __glibcxx_atomic_value_initialization 201911L
 #  if defined(__glibcxx_want_all) || defined(__glibcxx_want_atomic_value_initialization)
 #   define __cpp_lib_atomic_value_initialization 201911L
diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index cd08df34ba7..9b1aca0fc09 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,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 
     public:
+#if __cpp_lib_atomic_value_initialization
+      // _GLIBCXX_RESOLVE_LIB_DEFECTS
+      // 4169. std::atomic<T>'s default constructor should be constrained
+      constexpr atomic() noexcept(is_nothrow_default_constructible_v<_Tp>)
+	requires is_default_constructible_v<_Tp>
+	: _M_i()
+      {}
+#else
       atomic() = default;
+#endif
+
       ~atomic() noexcept = default;
       atomic(const atomic&) = delete;
       atomic& operator=(const atomic&) = delete;
@@ -414,7 +417,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 da54b0e259e..72bf4af6af3 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/69301.cc
@@ -29,6 +29,8 @@ struct NonDefaultConstructible
 
 template class std::atomic<NonDefaultConstructible>;
 
+static_assert(!std::is_default_constructible<std::atomic<NonDefaultConstructible>>::value);
+
 void
 test01()
 {
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/cons/trivial.cc b/libstdc++-v3/testsuite/29_atomics/atomic/cons/trivial.cc
new file mode 100644
index 00000000000..bb578db362d
--- /dev/null
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/cons/trivial.cc
@@ -0,0 +1,41 @@
+// { dg-do compile { target c++11 } }
+
+#include <atomic>
+#include <type_traits>
+
+// C++20 / P0883R2 makes std::atomic value-initialize, so it's no
+// longer going to be trivially default constructible; check that it
+// still is in earlier language modes.
+// (We are not applying that paper as a DR.)
+#if __cpp_lib_atomic_value_initialization
+constexpr bool atomic_default_ctor_is_trivial = false;
+#else
+constexpr bool atomic_default_ctor_is_trivial = true;
+#endif
+
+template<typename T>
+using isTDC = std::is_trivially_default_constructible<T>;
+
+static_assert(isTDC<std::atomic<bool>>::value == atomic_default_ctor_is_trivial);
+static_assert(isTDC<std::atomic<char>>::value == atomic_default_ctor_is_trivial);
+static_assert(isTDC<std::atomic<unsigned char>>::value == atomic_default_ctor_is_trivial);
+static_assert(isTDC<std::atomic<int>>::value == atomic_default_ctor_is_trivial);
+static_assert(isTDC<std::atomic<long>>::value == atomic_default_ctor_is_trivial);
+static_assert(isTDC<std::atomic<unsigned long long>>::value == atomic_default_ctor_is_trivial);
+static_assert(isTDC<std::atomic<int*>>::value == atomic_default_ctor_is_trivial);
+
+struct DefaultConstructible
+{
+  int a;
+  long long b;
+  char* p;
+};
+static_assert(isTDC<std::atomic<DefaultConstructible>>::value == atomic_default_ctor_is_trivial);
+
+struct NonDefaultConstructible
+{
+  NonDefaultConstructible(int i) : val(i) { }
+  int val;
+};
+// Not default constructible, therefore not trivially default constructible
+static_assert(isTDC<std::atomic<NonDefaultConstructible>>::value == false);
-- 
2.34.1

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to