[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/lib/AST/Type.cpp:2500
+    return BaseElementType.isTriviallyCopyableType(Context) &&
+           !BaseElementType.isDestructedType();
+  }
----------------
devin.jeanpierre wrote:
> rsmith wrote:
> > You can simplify this a little by dropping this `isDestructedType()` check; trivially copyable implies trivially destructible.
> > 
> > It would be a little more precise to check for `!isNonTrivialToPrimitiveDestructiveMove() && !isDestructedType()` here; that'd treat `__autoreleasing` pointers as trivially-relocatable, but not `__strong` pointers (because "destructive move" there actually means "non-destructive move" and needs to leave the object in a valid but unspecified state, which means nulling out a moved-from `__strong` pointer). I think getting this "fully" right would require a bit more plumbing to properly handle the case of a `struct` containing a `__strong` pointer (which is trivially relocatable but not trivially anything else).
> Oooh. `isNonTrivialToPrimitiveDestructiveMove` is perfect.
> 
> Actually, a struct containing a `__strong` pointer is already handled -- Clang [already](https://github.com/llvm/llvm-project/blob/603c1a62f8595f70c3e96ecbec8976d411c0cc08/clang/lib/AST/DeclCXX.cpp#L1037-L1059) ensures they're passed in registers, and does all the appropriate logic. It's just `__strong` pointers themselves that I'd need to handle the same way here. Unfortunately, I really, *really* don't understand the source code dealing with `__strong` pointers in structs. (In particular, I don't understand the difference between GC::Strong and ObjCLifetime::OCL_Strong, plus I'm uncomfy with the global language opts.)
> 
> It's tempting to use the return value of `isNonTrivialToPrimitiveDestructiveMove` and check for `Strong` -- but now that's some *third* thing, and not mirroring the logic of when it's in a struct as a data member.
The existing handling only covers Objective-C++ and not (non-C++) Objective-C (the C and C++ codepaths for computing struct properties are entirely different, because the C++ side of things is primarily working out properties of special member functions). It looks like your test coverage doesn't cover the non-C++ side of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114732