[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#1302134, @Quuxplusone wrote:

> Now that `[[clang::maybe_trivially_relocatable]]` is implemented, we can see if it does actually simplify the libc++ patch. I will try to get to that tonight.


Here is `deque` implemented with `[[trivially_relocatable]]`: https://github.com/Quuxplusone/libcxx/compare/0533994b3fcb115fdd89a38f80f737de8e76d023...1df78ab704b8b3d5a5e225a7624eb5fe10e3f401
And `deque` implemented with `[[maybe_trivially_relocatable]]`:
https://github.com/Quuxplusone/libcxx/compare/0533994b3fcb115fdd89a38f80f737de8e76d023...5945ccee2f7bb30fbb4e3a7cf9308ee8145c758b

Here is `unordered_set` implemented with `[[trivially_relocatable]]`: 
https://github.com/Quuxplusone/libcxx/compare/0533994b3fcb115fdd89a38f80f737de8e76d023...8ddd963c738cef0c3ad5b314746ac5ddc2415751
And `unordered_set` implemented with `[[maybe_trivially_relocatable]]`:
https://github.com/Quuxplusone/libcxx/compare/0533994b3fcb115fdd89a38f80f737de8e76d023...0e8ddfe99145fe69a18a3fd8929674d937f22b99

The `[[maybe_trivially_relocatable]]` patches are much shorter (18 and 30 lines) than the `[[trivially_relocatable]]` patches (78 and 145 lines). However, it's worth noting that 56 of the lines in the `unordered_set [[trivially_relocatable]]` patch are there only to deal with C++03's lack of `using`-constructor declarations, and would disappear if we were working on a C++11-only codebase.

In the `unordered_set [[trivially_relocatable]]` patch, I use the "triviality-forcing wrapper" pattern which would not be possible with `[[maybe_trivially_relocatable]]`. This saves me from having to pipe the attribute down through `unique_ptr` and `__bucket_list_deallocator`.

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.

In the `unordered_set [[maybe_trivially_relocatable]]` patch, I must either pipe the attribute down through `unique_ptr`, or else give up using `unique_ptr` in `__hash_table`. I chose to modify `unique_ptr`; but note that many working programmers wouldn't have this luxury. In the `unordered_set [[trivially_relocatable]]` patch, I don't need to modify `unique_ptr`; I just use the "triviality-forcing wrapper" pattern.

In the `unordered_set [[trivially_relocatable]]` patch, I chose to "cheat" and write `is_trivially_relocatable<__next_pointer>::value && is_trivially_relocatable<__pointer_allocator>::value` instead of piping trivial relocatability down through `__bucket_list_deallocator`. If I'd done that, the patch would have gotten even longer. (And for no benefit, since `__bucket_list_deallocator` is a private helper class never directly touched by the user.)

So, both ways have their "cheats" and both ways have their dangers-of-unsafe-behavior. The `[[maybe_trivially_relocatable]]` patches are significantly shorter. The `[[trivially_relocatable]]` patches touch fewer classes. I'm still more comfortable with `[[trivially_relocatable]]`, but I can see the appeal of terser code.

I still believe it is impossible to implement `std::optional` with only `[[maybe_trivially_relocatable]]`.


Repository:
  rC Clang

https://reviews.llvm.org/D50119