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