[PATCH] D41039: Add support for attribute "trivial_abi"

[John McCall via cfe-commits]

On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> rjmccall added a comment.
>>
>> In https://reviews.llvm.org/D41039#951648, @ahatanak wrote:
>>
>> > I had a discussion with Duncan today and he pointed out that perhaps we
>> shouldn't allow users to annotate a struct with "trivial_abi" if one of its
>> subobjects is non-trivial and is not annotated with "trivial_abi" since
>> that gives users too much power.
>> >
>> > Should we error out or drop "trivial_abi" from struct Outer when the
>> following code is compiled?
>> >
>> >   struct Inner1 {
>> >     ~Inner1(); // non-trivial
>> >     int x;
>> >   };
>> >
>> >   struct __attribute__((trivial_abi)) Outer {
>> >     ~Outer();
>> >     Inner1 x;
>> >   };
>> >
>> >
>> > The current patch doesn't error out or drop the attribute, but the
>> patch would probably be much simpler if we didn't allow it.
>>
>>
>> I think it makes sense to emit an error if there is provably a
>> non-trivial-ABI component.  However, for class temploids I think that
>> diagnostic should only fire on the definition, not on instantiations; for
>> example:
>>
>>   template <class T> struct __attribute__((trivial_abi)) holder {
>>      T value;
>>      ~holder() {}
>>   };
>>   holder<std::string> hs; // this instantiation should be legal despite
>> the fact that holder<std::string> cannot be trivial-ABI.
>>
>> But we should still be able to emit the diagnostic in template
>> definitions, e.g.:
>>
>>   template <class T> struct __attribute__((trivial_abi)) named_holder {
>>      std::string name; // there are no instantiations of this template
>> that could ever be trivial-ABI
>>      T value;
>>      ~named_holder() {}
>>   };
>>
>> The wording should be something akin to the standard template rule that a
>> template is ill-formed if it has no valid instantiations, no diagnostic
>> required.
>>
>> I would definitely like to open the conversation about the name of the
>> attribute.  I don't think we've used "abi" in an existing attribute name;
>> usually it's more descriptive.  And "trivial" is a weighty word in the
>> standard.  I'm not sure I have a great counter-proposal off the top of my
>> head, though.
>>
>
> Agreed on both counts (would love a better name, don't have any stand-out
> candidates off the top of my head).
>
> I feel like a more descriptive term about the property of the object would
> make me happier - something like "address_independent_identity"
> (s/identity/value/?) though, yeah, that's not spectacular by any stretch.
>

Incidentally, your comments are not showing up on Phabricator for some
reason.

The term "trivially movable" suggests itself, with two caveats:
  - What we're talking about is trivial *destructive* movability, i.e. that
the combination of moving the value to a new object and not destroying the
old object can be done trivially, which is not quite the same as trivial
movability in the normal C++ sense, which I guess could be a property that
someone theoretically might care about (if the type is trivially
destructed, but it isn't copyable for semantic reasons?).
  - Trivial destructive movability is a really common property, and it's
something that a compiler would really like to optimize based on even in
cases where trivial_abi can't be adopted for binary-compatibility reasons.
Stealing the term for the stronger property that the type is trivially
destructively movable *and its ABI should reflect that in a specific way*
would be unfortunate.

"trivially_movable" is a long attribute anyway, and
"trivially_destructively_movable" is even worse.

Maybe that second point is telling us that this isn't purely descriptive —
it's inherently talking about the ABI, not just the semantics of the type.
I might be talking myself into accepting trivial_abi if we don't end up
with a better suggestion.

Random thing that occurred to me: is it actually reasonable to enforce
trivial_abi correctness in a non-template context?  Templates aren't the
only case where a user could reasonably want to add trivial_abi and just
have it be suppressed if it's wrong.  Imagine if some stdlib made
std::string trivial_abi; someone might reasonably want to make my
named_holder example above trivial_abi as well, with the expectation that
it would only have an effect on targets where std::string was trivial_abi.
At the very least, I'm concerned that we might be opening ourselves up to a
need to add supporting features, like a way to be conditionally trivial_abi
based on context.

https://reviews.llvm.org/D41039
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171211/a7a424dc/attachment.html>