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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
