On Tue, Dec 19, 2017 at 9:43 PM Akira Hatanaka <ahata...@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 12:12 PM, John McCall <rjmcc...@gmail.com> wrote: > >> On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie <dblai...@gmail.com> >> wrote: >> >>> On Mon, Dec 11, 2017 at 5:38 PM John McCall <rjmcc...@gmail.com> wrote: >>> >>>> On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie <dblai...@gmail.com> >>>> wrote: >>>> >>>>> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator < >>>>> revi...@reviews.llvm.org> wrote: >>>>> >>>>>> rjmccall added a comment. >>>>>> >>>>>> In 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.. > 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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
