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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
