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

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 2 09:42:16 PST 2018


On Tue, Dec 19, 2017 at 9:43 PM Akira Hatanaka <ahatanak at gmail.com> wrote:

> On Tue, Dec 12, 2017 at 12:12 PM, John McCall <rjmccall at gmail.com> wrote:
>
>> On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>> On Mon, Dec 11, 2017 at 5:38 PM John McCall <rjmccall at gmail.com> wrote:
>>>
>>>> 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.
>>>>
>>>
>>> Yeah, Phab doesn't do a great job looking on the mailing list for
>>> interesting replies - I forget, maybe it only catches bottom post or top
>>> post but not infix replies or something...
>>>
>>
>> Well, fortunately the mailing list is also archived. :)
>>
>>
>>> 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.
>>>>
>>>
>>> Fair point.
>>>
>>>
>>>> "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.
>>>>
>>>
>>> *nod* I think if you want to slice this that way (ensuring there's space
>>> to support describing a similar property for use only in non-ABI-breaking
>>> contexts as well) it seems like ABI is the term to use somewhere in the
>>> name.
>>>
>>
>> Yeah.  We just had a little internal conversation about it, and the idea
>> of "address_invariant_abi" came up, but I'm not sure it has enough to
>> recommend it over "trivial_abi" to justify the longer name.
>>
>>
>>> 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.
>>>>
>>>
>>> Fair point, much like the quirky but useful behavior of "= default".
>>> Good point about non-dependent contexts still being relevant to this
>>> subjective behavior...
>>>
>>> I was already leaning towards this being a warning rather than an error
>>> - this situation leans me moreso that way & possibly suppressing the
>>> warning when the types are split between system and non-system headers (if
>>> the attribute's on a type in a non-system header, but the type that's
>>> blocking it from being active is in a system header, don't warn).
>>>
>>
>> That's an interesting idea.
>>
>>
> OK, so if a trivial_abi class has a component that is not trivial, we drop
> the trivial_abi from the containing class. Also, we may decide not to error
> out or warn if the attribute is on a template or the non-trivial class
> comes from a system header.
>
> What would the rules be for propagating the trivial_abi property outwards?
> I tried to extend the existing standard rules for determining whether a
> special function is trivial to trivial_abi or whether a type should be
> forced to be passed indirectly in my first patch, but it looks like that
> approach is too complicated.
>

What makes it too complicated? That would seem unfortunate to diverge here,
I would think..


> I'm thinking about using the following rules instead. What do you think?
>
> trivial_abi=true for a class if it meets the following conditions:
>
> 1. it has attribute "trivial_abi" or
> 2. if it doesn't have attribute "trivial_abi", it must meet all of the
> following conditions:
>  2-1. it doesn't have any virtual functions or virtual bases and
>  2-2. it doesn't have any members that would cause the type to be
> non-trivial (e.g., __weak objc pointers) and
>  2-3. it doesn't have any user-provided copy constructors, move
> constructors, or destructors (explicitly defaulted methods are OK) and
>  2-4. trivial_abi=true for all of its base classes and members
>
> These rules apply only when the class or the type of one of its members or
> base classes has attribute "trivial_abi". The existing rules in the
> standard will be used to determine whether clang should force a type to be
> passed indirectly if none of the classes in the class hierarchy have
> attribute "trivial_abi".
>
>
>> John.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180102/8ed08765/attachment.html>


More information about the cfe-commits mailing list