> On Jan 3, 2018, at 6:39 PM, John McCall <rjmcc...@gmail.com> wrote: > > On Wed, Jan 3, 2018 at 2:07 PM, Akira Hatanaka <ahatan...@apple.com > <mailto:ahatan...@apple.com>> wrote: >> On Jan 3, 2018, at 10:25 AM, John McCall <rjmcc...@gmail.com >> <mailto:rjmcc...@gmail.com>> wrote: >> >> On Wed, Jan 3, 2018 at 12:24 PM, Akira Hatanaka <ahatan...@apple.com >> <mailto:ahatan...@apple.com>> wrote: >>> On Jan 2, 2018, at 9:42 AM, David Blaikie via cfe-commits >>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>> >>> >>> >>> On Tue, Dec 19, 2017 at 9:43 PM Akira Hatanaka <ahata...@gmail.com >>> <mailto:ahata...@gmail.com>> wrote: >>> On Tue, Dec 12, 2017 at 12:12 PM, John McCall <rjmcc...@gmail.com >>> <mailto:rjmcc...@gmail.com>> wrote: >>> On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie <dblai...@gmail.com >>> <mailto:dblai...@gmail.com>> wrote: >>> On Mon, Dec 11, 2017 at 5:38 PM John McCall <rjmcc...@gmail.com >>> <mailto:rjmcc...@gmail.com>> wrote: >>> On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie <dblai...@gmail.com >>> <mailto:dblai...@gmail.com>> wrote: >>> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator >>> <revi...@reviews.llvm.org <mailto:revi...@reviews.llvm.org>> wrote: >>> rjmccall added a comment. >>> >>> In https://reviews.llvm.org/D41039#951648 >>> <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. >>> >>> Agreed on both counts (would love a better name, don't have any stand-out >>> candidates off the top of my head). >>> >>> 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. >>> >>> Incidentally, your comments are not showing up on Phabricator for some >>> reason. >>> >>> 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... >>> >>> Well, fortunately the mailing list is also archived. :) >>> >>> >>> The term "trivially movable" suggests itself, with two caveats: >>> - 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?). >>> - 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. >>> >>> Fair point. >>> >>> "trivially_movable" is a long attribute anyway, and >>> "trivially_destructively_movable" is even worse. >>> >>> 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. >>> >>> *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. >>> >>> 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. >>> >>> >>> 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. >>> >>> Fair point, much like the quirky but useful behavior of "= default". Good >>> point about non-dependent contexts still being relevant to this subjective >>> behavior... >>> >>> 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). >>> >>> That's an interesting idea. >>> >>> >>> 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. >>> >>> 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. >>> >>> What makes it too complicated? That would seem unfortunate to diverge here, >>> I would think.. >>> >> >> Based on my understanding of the discussion yesterday, I think we’ve decided >> not to diverge from the standard: >> >> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180101/thread.html#214043 >> >> <http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180101/thread.html#214043> >> >> My patch keeps track of the triviality of special functions using two >> bitfields in CXXRecordDecl (HasTrivialSpecialMembers and >> HasTrivialSpecialMembersForCall) 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. >> >> 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. >> >> Under the definition we developed in that thread, I think your new tracking >> bits are unavoidable. >> > > Yes, I think they are unavoidable. > > 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? >
I think I understand the main idea of the new language rule in the other thread. I’m looking into implementing it in my new patch. > John.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
