> On Jan 8, 2018, at 10:57 AM, David Blaikie <dblai...@gmail.com> wrote: > (just a side note: perhaps this conversation would've been more suited to > cfe-dev? I sort of missed it because I only check commits once a week, unless > I'm specifically cc'd on something. All good though :))
I suppose it's more design-and-development than just code review, so yes, perhaps cfe-dev would have been more appropriate. Regardless I should've made a point of CC'ing you, since you'd been part of the ongoing conversation. Apologies. John. > > On Wed, Jan 3, 2018 at 4:06 PM Richard Smith via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > On 3 January 2018 at 15:24, John McCall via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> On Jan 3, 2018, at 5:53 PM, Richard Smith <rich...@metafoo.co.uk >> <mailto:rich...@metafoo.co.uk>> wrote: >> On 3 January 2018 at 14:29, John McCall via cfe-commits >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> >>> On Jan 3, 2018, at 5:12 PM, Richard Smith <rich...@metafoo.co.uk >>> <mailto:rich...@metafoo.co.uk>> wrote: >>> >>> On 2 January 2018 at 20:55, John McCall via cfe-commits >>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>>> On Jan 2, 2018, at 10:43 PM, Richard Smith <rich...@metafoo.co.uk >>>> <mailto:rich...@metafoo.co.uk>> wrote: >>>> >>>> On 2 January 2018 at 19:02, John McCall via cfe-commits >>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>>> >>>>> On Jan 2, 2018, at 9:15 PM, Akira Hatanaka <ahatan...@apple.com >>>>> <mailto:ahatan...@apple.com>> wrote: >>>>> >>>>> >>>>> >>>>>> On Jan 2, 2018, at 4:56 PM, Richard Smith via cfe-commits >>>>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>>>>> >>>>>> On 2 January 2018 at 15:33, John McCall via cfe-commits >>>>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>>>>> Hey, Richard et al. Akira and I were talking about the right ABI rule >>>>>> for deciding can-pass-in-registers-ness for structs in the presence of >>>>>> trivial_abi, and I think I like Akira's approach but wanted to get your >>>>>> input. >>>>>> >>>>>> The current definition in Itanium is: >>>>>> >>>>>> non-trivial for the purposes of calls <> >>>>>> <> >>>>>> A type is considered non-trivial for the purposes of calls if: >>>>>> >>>>>> it has a non-trivial copy constructor, move constructor, or destructor, >>>>>> or >>>>>> <> >>>>>> I'm assuming we're implicitly excluding deleted functions here. (I'd >>>>>> prefer to make that explicit; this has been the source of a number of >>>>>> ABI mismatches.) >>>>>> all of its copy and move constructors are deleted. >>>>>> <> >>>>>> >>>>>> I'd suggest modifying this to: >>>>>> >>>>>> A type is considered non-trivial for the purposes of calls if: >>>>>> - if has a copy constructor, move constructor, or destructor >>>>>> which is non-trivial for the purposes of calls, or >>>>>> - all of its copy and move constructors are deleted and it does >>>>>> not have the trivial_abi attribute. >>>>>> >>>>>> A copy/move constructor is considered trivial for the purposes of calls >>>>>> if: >>>>>> - it is user-provided and >>>>>> - the class has the trivial_abi attribute and >>>>>> - a defaulted definition of the constructor would be >>>>>> trivial for the purposes of calls; or >>>>>> >>>>>> We'd need to say what happens if the function in question cannot validly >>>>>> be defaulted for any of the reasons in [dcl.fct.def.default]. Do we try >>>>>> to infer whether it's a copy or move constructor, and use the rules for >>>>>> a defaulted copy or move constructor? Or do we just say that's never >>>>>> trivial for the purposes of calls? Or something else? Eg: >>>>>> >>>>>> struct [[clang::trivial_abi]] A { >>>>>> A(A && = make()); >>>>>> }; >>>>>> >>>>>> Here, A::A(A&&) cannot validly be defaulted. Is A trivial for the >>>>>> purpose of calls? Likewise: >>>>>> >>>>>> struct [[clang::trivial_abi]] B { >>>>>> B(...); >>>>>> }; >>>>>> struct C { >>>>>> volatile B b; >>>>>> }; >>>>>> >>>>>> Here, C's copy constructor calls B::B(...). Is C trivial for the purpose >>>>>> of calls? (OK, Clang crashes on that example today. But still...) >>>>>> >>>>>> I'd be uncomfortable making the rules in [dcl.fct.def.default] part of >>>>>> the ABI; they seem to be changing relatively frequently. Perhaps we >>>>>> could say "if the function is a copy constructor ([class.copy.ctor]/1), >>>>>> then consider what an implicitly-declared defaulted copy constructor >>>>>> would do; if it's a move constructor ([class.copy.ctor]/2), then >>>>>> consider what an implicitly-declared defaulted move constructor would >>>>>> do; otherwise, it's not trivial for the purpose of calls". That'd mean A >>>>>> is trivial for the purpose of calls and C is not, which I think is >>>>>> probably the right answer. >>>>>> >>>>>> - it is not user-provided and >>>>>> - the class has no virtual functions and no virtual >>>>>> base classes, and >>>>>> - the constructor used to copy/move each direct base >>>>>> class subobject is trivial for the purposes of calls, and >>>>>> - for each non-static data member that is of class type >>>>>> (or array thereof), the constructor selected to copy/move that member is >>>>>> trivial for the purposes of calls. >>>>>> >>>>>> A destructor is considered trivial for the purposes of calls if: >>>>>> - it is not user-provided or the class has the trivial_abi >>>>>> attribute, and >>>>>> - the destructor is not virtual, and >>>>>> - all of the direct base classes of its class have destructors >>>>>> that are trivial for the purposes of calls, and >>>>>> - for all of the non-static data members of its class that are >>>>>> of class type (or array thereof), each such class is trivial for the >>>>>> purposes of calls. >>>>>> >>>>>> These definitions are intended to follow [class.copy.ctor]p11 and >>>>>> [class.dtor]p6 except for the special rules applicable to trivial_abi >>>>>> classes. >>>>>> >>>>>> If I could rephrase: a *tor is considered trivial for for the purposes >>>>>> of calls if it is either defaulted or the class has the trivial_abi >>>>>> attribute, and the defaulted definition would satisfy the language rule >>>>>> for being trivial but with the word "trivial" replaced by "trivial for >>>>>> the purposes of calls". So only effect of the trivial_abi attribute is >>>>>> to "undo" the non-triviality implied by a user-provided *tor when >>>>>> computing triviality for the purpose of calls. >>>>>> >>>>>> I think that's a reasonable rule, if we have a satisfactory notion of >>>>>> "defaulted definition". >>>>>> >>>>>> I'm not sure about the "defaulted definition" rule for copy/move >>>>>> constructors in trivial_abi classes. The intent is to allow class >>>>>> temploids with trivial_abi that are instantiated to contain non-trivial >>>>>> classes to just silently become non-trivial. I was thinking at first >>>>>> that it would be nice to have a general rule that trivial_abi classes >>>>>> only contain trivial_abi subobjects, but unfortunately that's not >>>>>> consistent with the standard triviality rule in some silly corner cases: >>>>>> a trivially-copyable class can have a non-trivially-copyable subobject >>>>>> if it happens to copy that subobject with a trivial copy constructor. I >>>>>> couldn't think of a better way of capturing this than the "defaulted >>>>>> definition" rule. I considered using the actual initializers used by >>>>>> the constructor, but that would introduce a lot of new complexity: >>>>>> suddenly we'd be asking about triviality for an arbitrary constructor, >>>>>> and copy/move elision make the question somewhat ambiguous anyway. >>>>>> >>>>>> Per the above examples, I don't think you can escape asking about >>>>>> triviality for an arbitrary constructor if you take this path. >>>>>> >>>>>> Another option, similar to your general rule, would be to say that a >>>>>> type is considered trivial for the purpose of calls if either: (1) it is >>>>>> trivial for the purpose of calls under the current Itanium ABI rule, or >>>>>> (2) it has the trivial_abi attribute and all members and base classes >>>>>> have types that are trivial for the purpose of calls. That would >>>>>> sidestep the "defaulted definition" complexity entirely, and while it >>>>>> differs from the way that the language computes triviality normally, it >>>>>> doesn't seem fundamentally unreasonable: when we're thinking about >>>>>> triviality for the purpose of calls, there's notionally a call to the >>>>>> trivial copy/move ctor being elided, not a call to an arbitrary ctor >>>>>> selected by overload resolution, and we'd just be pushing that effect >>>>>> from the class itself to its subobjects with this attribute. >>>>>> >>>>> >>>>> It sounds like a class containing a member that has a type annotated with >>>>> “trivial_abi” would not necessarily be considered trivial for the purpose >>>>> of calls according to rule (2)? For example, S1 would not be trivial for >>>>> the purpose of calls because it isn’t annotated with “trivial_abi” in the >>>>> code below: >>>>> >>>>> struct [[clang::trivial_abi]] S0 { >>>>> // user-provided special functions declared here. >>>>> }; >>>>> >>>>> struct S1 { >>>>> S0 f0; >>>>> }; >>>>> >>>>> I thought we wanted containing classes (S1 in this case) to be trivial >>>>> for the purpose of calls too? >>>> >>>> I would like that, yeah. >>>> >>>> OK, I think that's fair. Then we probably need the more complex rule. >>>> Which I think means we're at something equivalent to: >>>> >>>> A type is considered non-trivial for the purposes of calls if: >>>> - if has a copy constructor, move constructor, or destructor >>>> that is not deleted and is non-trivial for the purposes of calls, or >>>> - all of its copy and move constructors are deleted and it does >>>> not have the trivial_abi attribute. >>> >>> Hold on... this final "and it does not have the trivial_abi attribute" >>> looks wrong to me; it seems to break the "do what I mean"ness of the >>> attribute. Consider: >>> >>> template<typename T, typename U> struct [[clang::trivial_abi]] pair { ... }; >>> >>> std::pair<ContainsPointerToSelf, int> f(); // returned indirect >>> std::pair<ContainsPointerToSelf, NonCopyable> g(); // returned in registers >>> because all copy/move ctors deleted >>> >>> That seems like a bug. Can we just strike that addition, or does one of >>> your intended use cases need it? >> >> It was a last-minute addition that seemed like a good idea, but I was just >> thinking about all the copy/move ctors being explicitly deleted on the >> class, not any of the inheritance cases. I agree with striking it. >> >> The only use cases we really have in mind are >> - simple resource-owning classes like smart pointers, which would adopt >> the attribute, and >> - classes with defaulted copy/destruction semantics, which should >> propagate triviality if possible. >> >> I just think we need to be prepared to make the rule more general than that. >> >>>> A copy/move constructor is considered trivial for the purposes of calls >>>> if: >>>> - it is user-provided and >>>> - the class has the trivial_abi attribute and >>>> - a defaulted definition of a constructor with the >>>> signature of the implicit copy/move constructor for the class would be >>>> trivial for the purposes of calls; or >>> >>> One other concern here: what if the defaulted definition would be deleted? >>> I think in that case the constructor we're considering should also be >>> treated as if it were deleted. And that applies recursively: if the >>> implicit copy/move constructor would itself have been deleted, we want to >>> treat the original member of the type we're checking as being deleted. And >>> likewise, if a defaulted copy/move constructor invokes a copy/move >>> constructor of a trivial_abi class, and a defaulted copy/move constructor >>> for that class would have been deleted, we want to act as if the original >>> defaulted copy/move constructor was deleted. That seems awkward to specify >>> in the fashion we've been using until now, since the result of a triviality >>> calculation is now "deleted", "non-trivial", or "trivial", and deletedness >>> can change in either direction as a result of the attribute. >> >> Ugh. I feel like this problem is mostly a further indictment of the idea of >> basing this on what a defaulted definition would look like. >> >> We could just base it on the overall trivial-for-calls-ness of the subobject >> types. It's a very different rule from the standard triviality rule, but >> it's okay to differ here because this *only* affects special members of >> classes with the attribute. >> >> I like this idea a lot. Here's a concrete suggestion: >> >> """ >> A type has a triviality override if it has the trivial_abi attribute, and it >> has no virtual functions nor virtual base classes, and every subobject is >> trivial for the purposes of calls. The attribute is ill-formed if applied to >> a non-template class that does not meet these criteria; the attribute is >> ill-formed, no diagnostic required, if applied to a templated class and no >> instantiation of that class can meet these criteria. > > David B. and I were talking about whether this should be a required > diagnostic even in the template case, and I think we settled on "no" because > it could interfere with portability. Imagine that std::unique_ptr were made > trivial_abi in some STL; classes containing a std::unique_ptr could only be > trivial_abi on that target. On the other hand, I get that it's nice to have > a static guarantee that the attribute meant something. > > Maybe we could make it ill-formed, no diagnostic required, if the attribute > is present but the class can never have a triviality override (for any > instantiation, if a template). That would give us wide leeway to complain > about putting it on a class with a direct virtual base, or when there's a > non-trivial subobject whose type is defined in the "same library", or if > specifically requested to. > > Besides, we're not actually promising to pass it "directly". It's a totally > legal implementation (right now) to just ignore the attribute. That wouldn't > be ABI-compatible with compilers that implement it, of course, but not > everyone cares about that. > > I'd be OK with, say, downgrading this from an error to a warning, or making > it an error-with-a-warning-flag. But even in the case of a unique_ptr member, > I think a user would typically want to be told that the attribute didn't have > the effect they're looking for: that's exactly the case where I would expect > people to try to "override" the triviality of a subobject using the > attribute, so I think the diagnostic should be enabled by default. > > I suppose the "ill-formed, no diagnostic required" formulation at least gives > other implementers of the attribute a hint that they should also consider > producing a diagnostic. >> A type is trivial for the purposes of calls if: >> - it has a triviality override, or >> - it is trivial for the purposes of calls as specified in the Itanium C++ >> ABI, or would be so if all direct or indirect construction and destruction >> of types with a triviality override were ignored when computing the >> triviality (but not deletedness) of functions >> """ > > I like this wording, since we don't have to actually repeat anything from the > standard. > >> So we would still compute both a "trivial" and a "trivial for the purposes >> of calls" flag for defaulted copy constructors, move constructors, and >> destructors, but we'd only do the overload resolution and deletedness >> analysis once; trivial would always imply trivial for the purposes of calls, >> and the converse only fails when there is a subobject whose type has a >> triviality override. > > Right. > > John. > >> Put another way, we'd have four levels of triviality for special members: >> deleted, non-trivial, trivial for purposes of calls, and trivial. The >> triviality of a deleted member is "deleted". The triviality of any >> trivial_abi member is "trivial for purposes of calls". The triviality of any >> other user-provided member is "non-trivial". And the triviality of a >> non-user-provided non-deleted member is "deleted" if any subobject call is >> ill-formed, otherwise "non-trivial" for the special cases involving virtual >> bases and virtual functions, otherwise the mimimum of that value over all >> subobject calls. And a type is trivial for the purposes of calls unless any >> copy ctor, move ctor or dtor is "non-trivial" or all copy and move >> constructors are "deleted". >>> Here's a terse summary of the rule I'm considering: >>> >>> """ >>> For the determination of triviality for the purposes of calls, a modified >>> form of the program is considered. In this modified form, each copy or move >>> constructor or destructor of a class with the trivial_abi attribute is >>> replaced by a defaulted copy or move constructor or destructor (with the >>> signature of an implicit such declaration), and calls to the former are >>> transformed into calls to the latter within the implicit definitions of >>> defaulted special member functions. A function is deleted for the purposes >>> of calls in the original program if the corresponding function is deleted >>> in the modified program, and is otherwise trivial for the purposes of calls >>> in the original program if the corresponding function is trivial in the >>> modified program. >>> >>> A type is considered non-trivial for the purposes of calls if: >>> - if has a copy constructor, move constructor, or destructor that is >>> non-deleted and non-trivial for the purposes of calls, or >>> - all of its copy and move constructors are deleted for purposes of >>> calls. >>> """ >> >> Yikes. I feel like I would have no ability to explain this rule to a user. >> >>>> - it is not user-provided and >>>> - the class has no virtual functions and no virtual >>>> base classes, and >>>> - the constructor used to copy/move each direct base >>>> class subobject is trivial for the purposes of calls, and >>>> - for each non-static data member that is of class type >>>> (or array thereof), the constructor selected to copy/move that member is >>>> trivial for the purposes of calls. >>>> A constructor that is neither a copy constructor nor a move constructor >>>> is considered non-trivial for the purposes of calls. >>> >>> This clause is there to handle constructors that are copy/move constructors >>> only because of defaulted arguments? I wonder if this is necessary; I >>> think the allocator-like use cases would prefer that we just ignore the >>> non-initial arguments, wouldn't they? >>> >>> This doesn't affect the default argument case: if a constructor has a first >>> parameter of type T / cv T& / cv T&&, and all further parameters (if any) >>> have default arguments, it is still a copy or move constructor. Rather, we >>> reach this clause in any case where "the constructor used/selected to >>> copy/move [...]" has some other first parameter type or is X::X(...); such >>> a constructor is only selected when there is no viable copy/move >>> constructor. >> >> Oh, which can happen even for non-user-provided constructors because it's >> just the ordinary overload rules, of course. >> >>>> A destructor is considered trivial for the purposes of calls if: >>>> - it is not user-provided or the class has the trivial_abi >>>> attribute, and >>>> - the destructor is not virtual, and >>>> - all of the direct base classes of its class have destructors >>>> that are trivial for the purposes of calls, and >>>> - for all of the non-static data members of its class that are >>>> of class type (or array thereof), each such class is trivial for the >>>> purposes of calls. >>>> >>>> Bolded phrases are changed from John's initial email. >>> >>> Thank you for the revision; this is much improved. >>> >>> I'm concerned about the level of complexity we've discovered to be >>> necessary here, and in particular the necessity of having a side-notion of >>> "trivial for the purpose of calls" for all copy/move ctors and dtors, even >>> in classes that do not directly use the trivial_abi attribute. But I >>> suppose that's fundamental if we want to pass struct S1 (above) directly. >>> I'd like a simpler rule, but I'm not convinced there is one. >> >> Well, I think the adjustment I suggest above would cap the complexity a bit; >> at least we would need these speculative investigation into defaulted >> definitions that don't actually exist. But we'd still need to track the new >> kind of triviality for each ctor/dtor. >> >> John. >> >>> >>> John. >>> >>>> >>>> John. >>>> >>>>> >>>>>> I'm also not sure about the right rules about virtual methods. Should >>>>>> we allow polymorphic classes to be made trivial by application of the >>>>>> attribute? >>>>>> >>>>>> I think that it probably doesn't make much sense to pass dynamic classes >>>>>> indirectly unless we can avoid passing the vptr; otherwise I'd expect >>>>>> we'd use too many registers for it to be worthwhile. Perhaps as a >>>>>> compromise, we could make the attribute ill-formed if used on a class >>>>>> definition that introduces any virtual bases or explicitly declares any >>>>>> member functions as 'virtual'. That gives us the room to make this >>>>>> decision later if we find we want to. >>>>>> _______________________________________________ >>>>>> 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> >>>> >>>> >>>> _______________________________________________ >>>> 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> >>>> >>>> >>> >>> >>> _______________________________________________ >>> 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> >>> >>> >> >> >> _______________________________________________ >> 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> >> >> > > > _______________________________________________ > 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> > > _______________________________________________ > 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>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
