rjmccall added a comment. In D50119#1305503 <https://reviews.llvm.org/D50119#1305503>, @Quuxplusone wrote:
> In D50119#1303720 <https://reviews.llvm.org/D50119#1303720>, @rjmccall wrote: > > > In D50119#1303662 <https://reviews.llvm.org/D50119#1303662>, @Quuxplusone > > wrote: > > > > > >> I still believe it is impossible to implement `std::optional` with > > > >> only `[[maybe_trivially_relocatable]]`. > > > > > > > > This might also need elaboration. > > > > > > Because `__optional_destruct_base` contains an anonymous union member, > > > and that union type is not destructible; therefore that union type is not > > > trivially relocatable; therefore `__optional_destruct_base` contains a > > > member of non-trivially-destructible type. However, I'm working on > > > changing my patch to make anonymous unions "see-through" in this respect, > > > so that that union's non-destructibility doesn't interfere with the > > > `[[maybe_trivially_relocatable]]`ity of its enclosing class type, as long > > > as all the members of the //union// are trivially relocatable. This might > > > fix `optional`. I'm not sure yet. > > > > > > Ah, that makes sense. And yeah, that seems like the right rule for unions. > > I really appreciate you putting the effort into exploring this part of the > > design space. > > > I think it's part of the right rule, but it's not enough, either. Consider: > > struct [[clang::maybe_trivially_relocatable]] DestroyBase { > int i; > DestroyBase(DestroyBase&&) = delete; > ~DestroyBase(); > }; > struct [[clang::maybe_trivially_relocatable]] MoveBase : DestroyBase { > MoveBase(MoveBase&&); > }; > > static_assert(not std::is_move_constructible_v<DestroyBase>); //, therefore > static_assert(not std::is_relocatable_v<DestroyBase>); //, therefore > static_assert(not std::is_trivially_relocatable_v<DestroyBase>); //; and > since MoveBase now has a base class of non-trivially-relocatable type, > static_assert(not std::is_trivially_relocatable_v<MoveBase>); > Hmm. I don't remember what we do in this case for `trivial_abi` (which does need to address it because it's possible to return types like `DestroyBase`). It looks like we currently make it non-trivial. But trivial-ABI needs to consider copy constructors as well as move constructors, which would undermine my case that it should be a subset of trivially-relocatability, at least in the fantasy world where there are correct types which default their move constructors but not their copy constructors or vice-versa. > So maybe you need a notion like "a class is BlahBlah if all of its direct > members are BlahBlah, //and// either it is annotated > `[[maybe_trivially_relocatable]]` or else it has no non-defaulted move or > destroy operations." And then "a move-constructible, destructible class is > trivially relocatable if it is BlahBlah, //or// if it is annotated > `[[trivially_relocatable]]`." And then a class like `DestroyBase` can be > "BlahBlah but not move-constructible," and if it's placed inside `MoveBase`, > and `MoveBase` is //both// annotated and move-constructible, then `MoveBase` > becomes trivially relocatable. I'm completely comfortable with saying that `MoveBase` just isn't trivially-relocatable under `maybe_trivially_relocatable` attribute, and if you've somehow arranged for it to be, you need to use the stronger attribute. In practice types like this either won't exist or won't actually satisfy the requirements. That should allow you to avoid distinguishing between `BlahBlah` and `trivially-relocatable`. As long as the analysis jumps directly down to variant members instead of treating anonymous unions as formal subobjects, I don't think this will cause serious limitations or usability problems. Variant members are important as a special case only because anonymous unions can't really be fixed otherwise, and it's consistent with basically every other language rule around construction and destruction. > The benefit of my original "second-level" `[[trivially_relocatable]]` > attribute is that it puts all the responsibility in a single very high-level > place. `optional` is the thing that knows exactly when it's trivially > relocatable (or at least can make a conservative guess, which might boil down > to "I'm never trivially relocatable"), so `optional` is the thing that should > get the attribute. Yes, it's definitely unambiguous what happens with your stronger attribute. > If I get time today I'll try adding the boolean argument to the attribute, > and do the libc++ patches that way for comparison. Okay. Sorry for the delayed response, I had last week off. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50119/new/ https://reviews.llvm.org/D50119 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
