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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D50119