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

[John McCall via Phabricator via cfe-commits]

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.


https://reviews.llvm.org/D41039