[PATCH] D139741: [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/include/clang/AST/Decl.h:3063
+  /// Determine if this field is a potentially-overlapping, that is,
+  /// subobject with the [[no_unique_address]] attribute
+  bool isPotentiallyOverlapping() const;
----------------
Maybe clarify that this also checks the object is of class type?  That's technically not part of the C++ standard definition of a "potentially-overlapping subobject".  (I'm not sure what rule, if any, prevents non-class objects from overlapping.)


================
Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:386
           bitsToCharUnits(getFieldBitOffset(*Field)), MemberInfo::Field,
-          getStorageType(*Field), *Field));
+          (Field->isPotentiallyOverlapping()
+               ? getStorageType(Field->getType()->getAsCXXRecordDecl())
----------------
Extra parentheses.


================
Comment at: clang/test/CodeGenCXX/no-unique-address-3.cpp:60
+};
+Foo I;
+
----------------
It would be nice to also explicitly check the LLVM IR types, instead of just checking that emitting LLVM IR doesn't crash.

(We also try to avoid using -emit-obj from clang tests, since that only works if the relevant backend is linked into clang.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139741