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