From 613f8ddbe3d7da63d827e588bf0333813c184b8a Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo 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's default constructor based on whether T itself is default constructible. At the moment, std::atomic'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. --- 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 #include +#include 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'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 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; +static_assert(!std::is_default_constructible>::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 +#include + +// 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 +using isTDC = std::is_trivially_default_constructible; + +static_assert(isTDC>::value == atomic_default_ctor_is_trivial); +static_assert(isTDC>::value == atomic_default_ctor_is_trivial); +static_assert(isTDC>::value == atomic_default_ctor_is_trivial); +static_assert(isTDC>::value == atomic_default_ctor_is_trivial); +static_assert(isTDC>::value == atomic_default_ctor_is_trivial); +static_assert(isTDC>::value == atomic_default_ctor_is_trivial); +static_assert(isTDC>::value == atomic_default_ctor_is_trivial); + +struct DefaultConstructible +{ + int a; + long long b; + char* p; +}; +static_assert(isTDC>::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>::value == false);