<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Dec 12, 2017 at 12:12 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@gmail.com" target="_blank">rjmccall@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div class="gmail-m_-8969288968174730138gmail-h5">On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br></div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="gmail-m_-8969288968174730138gmail-h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><span style="color:rgb(80,0,80)">On Mon, Dec 11, 2017 at 5:38 PM John McCall <</span><a href="mailto:rjmccall@gmail.com" target="_blank">rjmccall@gmail.com</a><span style="color:rgb(80,0,80)">> wrote:</span><br><div class="gmail_quote"><div><div class="gmail-m_-8969288968174730138gmail-m_-2329063236051570055h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br><div class="gmail_quote"><span class="gmail-m_-8969288968174730138gmail-m_-2329063236051570055m_6617752451251688076m_-4185080464891481584gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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/D4103<wbr>9#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></span><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></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Incidentally, your comments are not showing up on Phabricator for some reason.</div></div></div></div></blockquote></div></div><div><br>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... <br></div></div></div></blockquote><div><br></div></div></div><div>Well, fortunately the mailing list is also archived. :)</div><span class="gmail-m_-8969288968174730138gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>The term "trivially movable" suggests itself, with two caveats:<br></div><div> - 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?).</div><div> - 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.</div></div></div></div></blockquote></span><div><br>Fair point.<br> </div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>"trivially_movable" is a long attribute anyway, and "trivially_destructively_movab<wbr>le" is even worse.<br></div><div><br></div><div>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.</div></div></div></div></blockquote></span><div><br>*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.<br></div></div></div></blockquote><div><br></div></span><div>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.</div><span class="gmail-m_-8969288968174730138gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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.<br></div></div></div></div></blockquote></span><div><br>Fair point, much like the quirky but useful behavior of "= default". Good point about non-dependent contexts still being relevant to this subjective behavior... <br><br>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).<br></div></div></div></blockquote><div><br></div></span><div>That's an interesting idea.</div><span class="gmail-m_-8969288968174730138gmail-HOEnZb"><font color="#888888"><div><br></div></font></span></div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>I'm thinking about using the following rules instead. What do you think?</div><div><br></div><div>trivial_abi=true for a class if it meets the following conditions:<br></div><div><br></div><div><div>1. it has attribute "trivial_abi" or<br></div></div><div>2. if it doesn't have attribute "trivial_abi", it must meet all of the following conditions:</div><div> 2-1. it doesn't have any virtual functions or virtual bases and</div><div> 2-2. it doesn't have any members that would cause the type to be non-trivial (e.g., __weak objc pointers) and</div><div> 2-3. it doesn't have any user-provided copy constructors, move constructors, or destructors (explicitly defaulted methods are OK) and<br></div><div> 2-4. trivial_abi=true for all of its base classes and members</div><div><br></div><div>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".</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-m_-8969288968174730138gmail-HOEnZb"><font color="#888888"><div></div><div>John.</div></font></span></div>
</div></div>
</blockquote></div><br></div></div>