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

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

Quuxplusone added a comment.

In D50119#1308869 <https://reviews.llvm.org/D50119#1308869>, @rjmccall wrote:

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


Submitted for your consideration:

  template<class T>
  class HeapResource {
      std::unique_ptr<T> ptr_;
      HeapResource(const HeapResource& rhs) : ptr_(rhs.ptr_->clone()) {}  // not defaulted
      HeapResource(HeapResource&&) = default;  // defaulted
      ~HeapResource() = default;
  };

This type is isomorphic to a real-world implementation of `std::function`. When you copy a HeapResource, you copy a T (which is observable); when you move a HeapResource, you do not touch T at all. Therefore copy-and-destroy is observably different from move-and-destroy. But (A) as a library vendor, I don't think I would care about clients who pathologically try to observe the difference; and (B) I don't think I grasp exactly how `[[trivial_abi]]` would care about this case either. (But remember: I still think of `[[trivial_abi]]` as completely orthogonal. You cut that debate short. ;) I would see no intrinsic difficulty with marking `HeapResource` as `[[trivial_abi]]`, for example.)

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

To clarify: you'd be completely comfortable with the user's //having the ability// to "use the stronger attribute"? That is, there seems to be a reasonable argument in favor of the existence of the stronger attribute, even in the presence of the weaker attribute?

> In practice types like this either won't exist or won't actually satisfy the requirements.

I think we probably agree on the nature of reality here, but I don't understand the way you phrased this sentence. I mean, `DestroyBase` and `MoveBase` absolutely //do// exist, in libc++'s implementation of `optional`. If you just mean that "in practice all such types will be private implementation details, and we'll never have to worry about the implications of these types' advertising themselves as trivially relocatable because they'll only ever be used in highly controlled circumstances where we can trust the programmer not to fall into any pits of their own making," then I agree, I think.

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

No worries. I didn't have last week off, and I still didn't actually get around to adding the boolean argument yet.

Speaking of the boolean argument... The usual usage will be something like `[[trivially_relocatable(is_trivially_relocatable_v<allocator_type> && is_trivially_relocatable_v<pointer> && is_trivially_relocatable_v<size_type>)]]`, basically listing out the types of all the class's non-static data members. This has suddenly suggested to me that maybe the compiler could generate this kind of thing automatically... which we'd naturally spell `[[trivially_relocatable(auto)]]`! That is, maybe the right spelling for `[[maybe_trivially_relocatable]]` is `[[trivially_relocatable(auto)]]`!
...Or, on second thought, maybe not. That seems super prone to overuse and thus abuse, like, if someone blindly applied it to their codebase's equivalent of `std::list`. Yeah, I keep coming back to: the best way to avoid accidental knife injuries is to keep the knife sharp.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50119/new/

https://reviews.llvm.org/D50119