On Tue, 10 Dec 2024 at 10:48, Giuseppe D'Angelo <giuseppe.dang...@kdab.com> wrote: > > Hi, > > On 10/12/2024 10:56, Jonathan Wakely wrote: > > We need this to depend on is_trivially_copyable too, so we can use memcpy. > > > > I'm testing a fix now to fix bootstrap. > > There's a broader question I think, which is how much we want to "bend" > the language rules.
That's the subject of https://gcc.gnu.org/PR86023 And see also https://gcc.gnu.org/PR109976 > [basic] isn't really super-explicit at giving us the rights to bit-blast > bytes into uninitialized storage, and pretend there are objects there. > There are a couple of provisions that deal with trivially copyable types: > > https://eel.is/c++draft/basic.types.general#2 > https://eel.is/c++draft/basic.types.general#3 > > which guarantee that one can memcpy the representation of an live object > onto another live object, possibly itself. However, they don't > necessarily talk about creating/starting the lifetime of new objects > elsewhere. > > That could be achieved through implicit-lifetime, but trivially copyable > doesn't imply implicit-lifetime, AFAICS; consider the corner case of a > type with deleted constructors but only trivial assignments and > destruction, which is TC but not IL. This is why I think the facilities > in P2590R2 talk about "trivially-copyable implicit-lifetime" types. > > Maybe however the relocate functions are only called on types that we > know are move constructible (e.g. from std::vector) so if they're TC we > also know they're IL? The relocate functions are called on nothrow-move types, and then we dispatch to doing move+destroy or memcpy based on triviality. > > (The only thing missing is possibly a call to an abstract machine "magic > function", such as start_lifetime_as?) > > Explicitly checking for a trivial move constructor + trivial destructor > (that is, the code after my patch) implies that the type is IL, but that > doesn't really gives us the rights to assume that the object in the new > storage have the same values than the old ones, as that's only > guaranteed for TC types. That's partly why __is_bitwise_relocatable was intended as a customization point. We can specialize it for std::pair<T,U> because we know that if T and U are trivially copyable, then memcpy on std::pair<T,U> gives the right values (even though technically std::pair is never trivially copyable). More pragmatically, without checking for trivially copyable or disabling the warnings, there are bootstrap failures. So we do need a quick fix. > > In short it looks like a bit of a lose-lose situation. I hope P3279 will > bring clarity in the area. > > > Then, if the problem is just suppressing the warning, this does it: > > > diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h > > b/libstdc++-v3/include/bits/stl_uninitialized.h > > index 916288352d7..374f30ee645 100644 > > --- a/libstdc++-v3/include/bits/stl_uninitialized.h > > +++ b/libstdc++-v3/include/bits/stl_uninitialized.h > > @@ -1294,7 +1294,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > return __out.base(); > > } > > #endif > > - __builtin_memcpy(__result, __first, __count * sizeof(_Tp)); > > + // Cast to void* in order to suppress -Wclass-memaccess. > > + __builtin_memcpy(static_cast<void *>(__result), static_cast<const > > void *>(__first), __count * sizeof(_Tp)); > > } > > return __result + __count; > > } > > It's the trick we use in Qt; or similarly just suppressing the warning > via #pragmas, however I'm not sure if Clang likes that. > > What do you think? I think this should be considered for GCC 16, rather than now. So I've reverted the change to __is_bitwise_relocatable for now.
