> On Jan 3, 2018, at 10:25 AM, John McCall <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. > John. > > >> I'm thinking about using the following rules instead. What do you think? >> >> trivial_abi=true for a class if it meets the following conditions: >> >> 1. it has attribute "trivial_abi" or >> 2. if it doesn't have attribute "trivial_abi", it must meet all of the >> following conditions: >> 2-1. it doesn't have any virtual functions or virtual bases and >> 2-2. it doesn't have any members that would cause the type to be >> non-trivial (e.g., __weak objc pointers) and >> 2-3. it doesn't have any user-provided copy constructors, move >> constructors, or destructors (explicitly defaulted methods are OK) and >> 2-4. trivial_abi=true for all of its base classes and members >> >> 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". >> >> John. >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> > > > > -- > I suppose you'd like my real thoughts as well.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
