[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#1297070, @dblaikie wrote:

> Hey @rjmccall - I'm trying to remember, but can't get it quite clear in my head. I seem to recall some discussion about trivial_abi not implying/being strong enough for all the cases that trivial_relocatable sounds like it covers? Do you happen to remember the distinction there (the summary in this patch ("the warranting attribute [[trivially_relocatable]], which is similar in spirit to [[trivial_abi]], in that it lets the programmer communicate back to the compiler that a certain user-defined type should be assumed to have this property even though it would not naturally have the property all else being equal.") doesn't sound like what I remember - but my memory is hazy)?


`trivial_abi` permits annotated types to be passed and returned in registers, which is ABI-breaking.  Skimming the blog post, it looks like `trivially_relocatable` does not permit this — it merely signifies that destruction is a no-op after a move construction or assignment.  This is usefully different in the design space, since it means you can safely add the attribute retroactively to e.g. `std::unique_ptr`, and other templates can then detect that `std::unique_ptr` is trivially-relocatable and optimize themselves to use `memcpy` or `realloc` or whatever it is that they want to do.  So in that sense `trivial_abi` is a *stronger* attribute, not a *weaker* one: the property it determines ought to imply `trivially_relocatable`.

The only interesting question in the language design that I know of is what happens if you put the attribute on a template that's instantiated to contain a sub-object that is definitely not trivially relocatable / trivial-ABI.  For `trivial_abi`, we decided that the attribute is simply ignored — it implicitly only applies to specializations where the attribute would be legal.  I haven't dug into the design enough to know what `trivially_relocatable` decides in this situation, but the three basic options are:

- the attribute always has effect and allows trivial relocation regardless of the subobject types; this is obviously unsafe, so it limits the safe applicability of the attribute to templates
- the attribute is ignored, like `trivial_abi` is
- the attribute is ill-formed, and you'll need to add a `[[trivially_relocatable(bool)]]` version to support templates

If there are corner-case differences beyond that, I have no particular objection to unifying the semantics so that `trivial_abi` is strictly stronger, although we should talk about it case-by-case.



================
Comment at: test/SemaCXX/trivially-relocatable.cpp:42
+struct A6;
+struct [[trivially_relocatable]] A6 {};
+// expected-error at -1{{type A6 declared 'trivially_relocatable' after its first declaration}}
----------------
Rakete1111 wrote:
> Quuxplusone wrote:
> > Rakete1111 wrote:
> > > Why does this restriction exist? None of the existing attributes have it and I don't see why it would make sense to disallow this.
> > `[[noreturn]]` has it, and I am pretty sure that `[[trivial_abi]]` *ought* to have it, even though it currently doesn't. The intent is to make it harder for people to create ODR violations by declaring a type, using it in some way, and then saying "oh by the way this type was xxxxx all along."
> > 
> > ```
> > struct S { S(S&&); ~S(); };
> > std::vector<S> vec;
> > struct [[trivially_relocatable]] S;  // ha ha, now you have to re-do all of vector's codegen!
> > ```
> > 
> > Does that make sense as a reason it would be (mildly) beneficial to have this diagnostic?
> > You could still reasonably object to my assumptions that (A) the diagnostic is worth implementing in Clang and/or (B) the diagnostic is worth mandating in the Standard wording; but FWIW I do think it's very cheap both to implement and to specify.
> > 
> > (I will try to adjust the patch so that after the error is given, we remove the attribute from the class so as to make consistent any subsequent cascading errors. https://godbolt.org/g/nVRiow )
> Didn't know `[[noreturn]]` had it. Ah I see. Sold :)
I don't see any good reason to allow either this or `[[trivial_abi]]` on an arbitrary declaration of the type; it should have to be written on the definition.  This also gives sensible semantics for class template specializations.

The intrinsic obviously has to require its operand type to be complete, and therefore there is a unique declaration which can have the attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D50119