<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rjmccall added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D41039#951648" rel="noreferrer" target="_blank">https://reviews.llvm.org/D41039#951648</a>, @ahatanak wrote:<br>
<br>
> 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.<br>
><br>
> Should we error out or drop "trivial_abi" from struct Outer when the following code is compiled?<br>
><br>
> struct Inner1 {<br>
> ~Inner1(); // non-trivial<br>
> int x;<br>
> };<br>
><br>
> struct __attribute__((trivial_abi)) Outer {<br>
> ~Outer();<br>
> Inner1 x;<br>
> };<br>
><br>
><br>
> 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.<br>
<br>
<br>
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:<br>
<br>
template <class T> struct __attribute__((trivial_abi)) holder {<br>
T value;<br>
~holder() {}<br>
};<br>
holder<std::string> hs; // this instantiation should be legal despite the fact that holder<std::string> cannot be trivial-ABI.<br>
<br>
But we should still be able to emit the diagnostic in template definitions, e.g.:<br>
<br>
template <class T> struct __attribute__((trivial_abi)) named_holder {<br>
std::string name; // there are no instantiations of this template that could ever be trivial-ABI<br>
T value;<br>
~named_holder() {}<br>
};<br>
<br>
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.<br>
<br>
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.<br></blockquote><div><br>Agreed on both counts (would love a better name, don't have any stand-out candidates off the top of my head).<br><br>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.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D41039" rel="noreferrer" target="_blank">https://reviews.llvm.org/D41039</a><br>
<br>
<br>
<br>
</blockquote></div></div>