[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D50119#1303662, @Quuxplusone wrote:

> In https://reviews.llvm.org/D50119#1303577, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D50119#1303423, @Quuxplusone wrote:
> >
> > > In the `unordered_set [[maybe_trivially_relocatable]]` patch, I must wrap the attribute in a macro `_LIBCPP_MAYBE_TRIVIALLY_RELOCATABLE_UNLESS_DEBUG`. Without this caveat, I would have ended up with //unsafe behavior// in debug mode. The `unordered_set [[trivially_relocatable]]` patch does not have this danger; the fact that we break the Rule of Zero in debug mode is sufficient to disable trivial relocatability.
> >
> >
> > Can you elaborate?  Providing a non-defaulted copy/move constructor or destructor should make an unannotated class non-trivially-relocatable under both rules.
>
>
> In the patch I was describing, I //had// annotated `unordered_set`. However, your comment made me realize that after annotating `__hash_table`, I could completely drop the `[[maybe_trivially_relocatable]]` annotation on `unordered_set`, and hide its move-constructor behind `#if _LIBCPP_DEBUG_LEVEL >= 2` the same way I do in the `[[trivially_relocatable]]` patch. So I retract that complaint.
>
> Here is `unordered_set` implemented with `[[trivially_relocatable]]`: 
>  https://github.com/Quuxplusone/libcxx/compare/0533994b3fcb115fdd89a38f80f737de8e76d023...8ddd963c738cef0c3ad5b314746ac5ddc2415751
>  And here's the new and improved version of `unordered_set` implemented with `[[maybe_trivially_relocatable]]`:
>  https://github.com/Quuxplusone/libcxx/compare/0533994b3fcb115fdd89a38f80f737de8e76d023...0e8ddfe99145fe69a18a3fd8929674d937f22b99


Okay, thank you.

>>> 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.


Repository:
  rC Clang

https://reviews.llvm.org/D50119