[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Can this attribute not be applied to a base class, or to a type?  I think every time I've seen someone get bitten by the unique-address rule, it was because they had a base class that deleted some constructors (or something like that) and which was a base of a million different types; I'm sure the language model they'd prefer would be to just add an attribute to that one type instead of chasing down every place where they declared a field of the type.



================
Comment at: include/clang/Basic/Attr.td:345
+  let CXXABIs = ["GenericItanium", "GenericARM", "iOS", "iOS64", "WatchOS",
+                 "GenericAArch64", "GenericMIPS", "WebAssembly"];
+}
----------------
Can you just duplicate the thing that Slava Pestov recently did for LangOpts?  The tblgen enhancement really isn't very complicated, and I think we can agree that this is terrible. :)

We can probably kill the need for `CXXABIs` in tblgen entirely.


================
Comment at: include/clang/Basic/AttrDocs.td:1020
+point within the class (except that it does not share a vptr with the enclosing
+object).
+  }];
----------------
Can you include an example here?  And should this documentation mention that this is a standard C++ attribute (or at least one proposed for adoption)?


================
Comment at: lib/AST/Decl.cpp:3937
+  //     -- [has] virtual member functions or virtual base classes, or
+  //     -- has subobjects of nonzero size or bit-fields of nonzero length
+  if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
----------------
Surely a bit-field of nonzero length is a subobject of nonzero size.


================
Comment at: lib/AST/Decl.cpp:3945
+        return false;
+  }
+
----------------
Is this a C/C++ modules interaction?


================
Comment at: lib/CodeGen/CGExprAgg.cpp:1850
+AggValueSlot::Overlap_t
+CodeGenFunction::overlapForFieldInit(const FieldDecl *FD) {
+  if (!FD->hasAttr<NoUniqueAddressAttr>() || !FD->getType()->isRecordType())
----------------
`getOverlapForFieldInit`?  `overlap` is a verb.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63451