[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/test/SemaCXX/attr-trivial-abi.cpp:9-11
+// On Windows, trivial relocatability depends only on a trivial copy constructor existing.
+// In this case, it is implicitly deleted. Similar concerns apply to later tests.
+static_assert(!__is_trivially_relocatable(a<int>), "");
----------------
devin.jeanpierre wrote:
> Quuxplusone wrote:
> > devin.jeanpierre wrote:
> > > Quuxplusone wrote:
> > > > (1) Why should Windows be different from everyone else here?
> > > > (2) AFAIK, a user-defined move ctor means the copy ctor is //not implicitly declared//, but it's not //deleted//; so I think this comment is slightly wrong.
> > > > (2) AFAIK, a user-defined move ctor means the copy ctor is not implicitly declared, but it's not deleted; so I think this comment is slightly wrong.
> > > Sorry, I get that confused a lot. :)
> > > 
> > > > (1) Why should Windows be different from everyone else here?
> > > In this change so far, an object is only considered trivially relocatable if it's trivial for calls, and Windows is non-conforming wrt what it considers trivial for calls: it won't consider something trivial for calls if it isn't trivially copyable.
> > > 
> > > Code: https://clang.llvm.org/doxygen/SemaDeclCXX_8cpp_source.html#l06558
> > > 
> > > It surprised me too. This is a good motivator IMO to support something like your proposal, and have types be trivially relocatable that aren't trivial for calls. This allows, as you mention elsewhere, for optimizations that aren't ABI-breaking, and for e.g. non-conforming platforms like this to still take advantage of trivial relocatability.
> > > In this change so far, an object is only considered trivially relocatable if it's trivial for calls, and Windows [has a calling convention that's different from the Itanium C++ ABI's calling convention]: it won't consider something trivial for calls if it isn't trivially copyable.
> > 
> > Ah, I see, and I agree with your logic here. It's unintuitive but only for the same reasons we've already hashed over: we're introducing an `__is_trivially_relocatable(T)` that gives zero false-positives, but (for now) frequent false-negatives, and this just happens to be one //additional// source of false negatives on Win32 specifically. (And I keep tripping over it because this frequent-false-negative `__is_trivially_relocatable(T)` is named the same as D50119's "perfect" discriminator.)
> > 
> > Everyone's on board with the idea that we're promising to preserve the true positives forever, but at the same time we're expecting that we might //someday// fix the false negatives, right? I.e., nobody's expecting `!__is_trivially_relocatable(a<int>)` to remain true on Win32 forever? (I'm pretty sure we're all on board with this.)
> > Everyone's on board with the idea that we're promising to preserve the true positives forever, but at the same time we're expecting that we might someday fix the false negatives, right? I.e., nobody's expecting !__is_trivially_relocatable(a<int>) to remain true on Win32 forever? (I'm pretty sure we're all on board with this.)
> 
> +1
> 
> My hope is that future changes can fix the false negatives -- either with explicit annotations (like `[[trivially_relocatable]`), or with improved automatic inference, etc.
> 
> (For example, we could imagine marking types as trivially relocatable if they have *a* trivial (for calls) copy or move constructor and a trivial (for calls) destructor, even if they also have a nontrivial move or copy destructor. This would be a superset of trivial-for-calls types on all platforms, and especially Windows.)
> Everyone's on board with the idea that we're promising to preserve the true positives forever, but at the same time we're expecting that we might //someday// fix the false negatives, right? I.e., nobody's expecting `!__is_trivially_relocatable(a<int>)` to remain true on Win32 forever? (I'm pretty sure we're all on board with this.)

+1 from me too. Users of `__is_trivially_relocatable` should expect it to start returning `true` for more trivially-relocatable types over time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732