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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)
----------------
Quuxplusone wrote:
> @rjmccall wrote:
> > 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.
> 
> Not necessarily a "no-op"; my canonical example is a CopyOnlyCXX03SharedPtr which increments a refcount on construction and decrements on destruction. But move-construction plus destruction should "balance out" and result in no observable side effects.
> 
> > 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.
> 
> `trivial_abi` is an "orthogonal" attribute: you can have `trivial_abi` types with non-trivial constructors and destructors, which can have observable side effects. For example,
> ```
> struct [[clang::trivial_abi]] DestructionAnnouncer {
>     ~DestructionAnnouncer() { puts("hello!"); }
> };
> ```
> is `trivial_abi` (because of the annotation) yet not trivially relocatable, because its "move plus destroy" operation has observable side effects.
> 
> > 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
> 
> What happens is basically the first thing you said, except that I disagree that it's "obviously unsafe." Right now, conditionally trivial relocation is possible via template metaprogramming; see the libcxx patch at e.g.
> https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> Since the attribute is an opt-in mechanism, it makes perfect sense to me that if you put it on a class (or class template), then it applies to the class, without any further sanity-checking by the compiler. The compiler has no reason to second-guess the programmer here.
> 
> However, there's one more interesting case. Suppose the programmer puts the attribute on a class that isn't relocatable at all! (For example, the union case @erichkeane mentioned, or a class type with a deleted destructor.) In that case, this patch *does* give an error... *unless* the class was produced by instantiating a template, in which case we *don't* give an error, because it's not the template-writer's fault.
> https://p1144.godbolt.org/z/wSZPba
> trivial_abi is an "orthogonal" attribute: you can have trivial_abi types with non-trivial constructors and destructors, which can have observable side effects. 

Let me cut this conversation short.  `trivial_abi` is not such an old and widely-established attribute that we are unable to revise its definition.  I am comfortable making the same semantic guarantees for `trivial_abi` that you're making for `trivially_relocatable`, because I think it is in the language's interest for `trivial_abi` to be strictly stronger than `trivially_relocatable`.

> What happens is basically the first thing you said, except that I disagree that it's "obviously unsafe." 

Under your semantics, the attribute is an unchecked assertion about all of a class's subobjects.  A class template which fails to correctly apply the template metaprogramming trick to all of its dependently-typed subobjects — which can be quite awkward because it creates an extra dimension of partial specialization, and which breaks ABI by adding extra template parameters — will be silently miscompiled to allow objects to be memcpy'ed when they're potentially not legal to memcpy.  That is a footgun, and it is indeed "obviously unsafe".

Now, it's fair to say that it's unsafe in a useful way: because the attribute isn't checked, you can wrap a type you don't control in a `trivially_relocatable` struct and thereby get the advantages of triviality on the wrapper.  The model used by `trivial_abi` doesn't allow that.  But I feel pretty strongly that that is not the right default behavior for the language.


Repository:
  rC Clang

https://reviews.llvm.org/D50119