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. 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. "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. 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. https://reviews.llvm.org/D41039
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits