On Fri, 11 Oct 2024 at 20:33, Jonathan Wakely <jwak...@redhat.com> wrote: > > On Sat, 21 Sept 2024 at 10:43, Giuseppe D'Angelo > <giuseppe.dang...@kdab.com> wrote: > > > > 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/xcr4zK8hx > > > > PR116769 argues that Clang's behavior is the correct one here, so this > > patch improves compat with Clang by removing the defaulted default > > constructor. > > GCC's behaviour seems much more useful. > > > 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. > > Good, that's the correct answer :-) > I don't understand why anybody would want the NSDMI to be ignored and > give the wrong answer, or be instantiated and give a hard error. > > > > Thoughts? > > Your patch changes the value of > is_trivially_default_constructible_v<std::atomic<int>> for > C++11/14/17. > > Currently that is true for <= 17 and true for >= 20. You patch makes > it always false. > > If we did this instead, I think all compilers would handle it > correctly and we wouldn't change any behaviour for C++17 down: > > atomic() = default; > #ifdef __cpp_concepts >= 202002
We might not require 202002 here, that's the value for P0848R3 and we might not need that to conditionally delete the default ctor. I don't remember the details. It might be sufficient to just check that __cpp_concepts is defined. > atomic() requires (!std::is_constructible_v<_Tp>) = delete; > #endif > > For C++17 there's no NSDMI and the default constructor does the right > thing (getting deleted if T is not default constructible). > For C++20 the default constructor is deleted if the NSDMI would be > ill-formed, which is consistent with the C++17 behaviour. > The triviality of the constructor is unchanged.
