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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone 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:
> > (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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114732