<div dir="ltr">On Wed, Jan 3, 2018 at 2:07 PM, Akira Hatanaka <span dir="ltr"><<a href="mailto:ahatanaka@apple.com" target="_blank">ahatanaka@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><div><div class="h5"><blockquote type="cite"><div>On Jan 3, 2018, at 10:25 AM, John McCall <<a href="mailto:rjmccall@gmail.com" target="_blank">rjmccall@gmail.com</a>> wrote:</div><br class="m_6456577961230111160Apple-interchange-newline"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Wed, Jan 3, 2018 at 12:24 PM, Akira Hatanaka<span class="m_6456577961230111160Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:ahatanaka@apple.com" target="_blank">ahatanaka@apple.com</a>></span><span class="m_6456577961230111160Apple-converted-space"><wbr> </span>wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div dir="auto" style="word-wrap:break-word;line-break:after-white-space"><div dir="auto" style="word-wrap:break-word;line-break:after-white-space"><div dir="auto" style="word-wrap:break-word;line-break:after-white-space"><div dir="auto" style="word-wrap:break-word;line-break:after-white-space"><div dir="auto" style="word-wrap:break-word;line-break:after-white-space"><div><div><div class="m_6456577961230111160h5"><blockquote type="cite"><div>On Jan 2, 2018, at 9:42 AM, David Blaikie via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:</div><br class="m_6456577961230111160m_-7956724118214722485Apple-interchange-newline"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Dec 19, 2017 at 9:43 PM Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Dec 12, 2017 at 12:12 PM, John McCall<span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:rjmccall@gmail.com" target="_blank">rjmccall@gmail.com</a>></span><span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>wr<wbr>ote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div class="m_6456577961230111160m_-7956724118214722485m_-1976606938441742476gmail-m_-8969288968174730138gmail-h5">On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie<span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span><span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>w<wbr>rote:<br></div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_6456577961230111160m_-7956724118214722485m_-1976606938441742476gmail-m_-8969288968174730138gmail-h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color: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="m_6456577961230111160m_-7956724118214722485m_-1976606938441742476gmail-m_-8969288968174730138gmail-m_-2329063236051570055h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie<span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span><span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>w<wbr>rote:<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-width:1px;border-left-style:solid;border-left-color: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="m_6456577961230111160m_-7956724118214722485m_-1976606938441742476gmail-m_-8969288968174730138gmail-m_-2329063236051570055m_6617752451251688076m_-4185080464891481584gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">rjmccall added a comment.<br><br>In<span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span><a href="https://reviews.llvm.org/D41039#951648" rel="noreferrer" target="_blank">https://reviews.llvm.org/D4<wbr>1039#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> <span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>template <class T> struct __attribute__((trivial_abi)) holder {<br>     T value;<br>     ~holder() {}<br> <span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>};<br> <span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>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> <span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>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> <span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>};<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="m_6456577961230111160m_-7956724118214722485m_-1976606938441742476gmail-m_-8969288968174730138gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color: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-width:1px;border-left-style:solid;border-left-color: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> <span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>- 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> <span class="m_6456577961230111160m_-7956724118214722485Apple-converted-space"> </span>- 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-width:1px;border-left-style:solid;border-left-color: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="m_6456577961230111160m_-7956724118214722485m_-1976606938441742476gmail-m_-8969288968174730138gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color: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-width:1px;border-left-style:solid;border-left-color: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="m_6456577961230111160m_-7956724118214722485m_-1976606938441742476gmail-m_-8969288968174730138gmail-HOEnZb"><font color="#888888"><div><br></div></font></span></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div></div></blockquote><div><br>What makes it too complicated? That would seem unfortunate to diverge here, I would think..<br> </div></div></div></div></blockquote><div><br></div></div></div><div><div>Based on my understanding of the discussion yesterday, I think we’ve decided not to diverge from the standard:</div><div><br></div><div><a href="http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180101/thread.html#214043" target="_blank">http://lists.llvm.org/pipermai<wbr>l/cfe-commits/Week-of-Mon-<wbr>20180101/thread.html#214043</a></div><div><br></div></div><div>My patch keeps track of the triviality of special functions using two bitfields in CXXRecordDecl (HasTrivialSpecialMembers and HasTrivialSpecialMembersForCal<wbr>l) and two flags in FunctionDecl (IsTrivial and IsTrivialForCall) because it needs to distinguish between triviality according to the current standard’s definition and triviality for the purpose of calls. Those flags have to be updated in various places, which I thought might be too complicated.</div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Well, we're not changing the definition of triviality, and the intent is that we're not changing the definition of triviality-for-calls (as standardized by Itanium) in the absence of the trivial_abi attribute.</div><div><br></div><div>Under the definition we developed in that thread, I think your new tracking bits are unavoidable.</div><div><br></div></div></div></div></div></blockquote><div><br></div></div></div>Yes, I think they are unavoidable.</div></div></blockquote><div><br></div><div> Okay, I think we've come up with a reasonable language rule in the other thread.  Do you need anything else before you iterate on this patch?</div><div><br></div><div>John.</div></div>
</div></div>