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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

Can you also add some SemaCXX tests that ensure the attribute is properly diagnosed when written on a bit-field, a static data member, a function, is given an argument, etc?



================
Comment at: include/clang/AST/DeclCXX.h:337-341
     /// true when this class is empty for traits purposes,
-    /// i.e. has no data members other than 0-width bit-fields, has no
+    /// i.e. has no data members other than 0-width bit-fields and empty
+    /// fields marked [[no_unique_address]], has no
     /// virtual function/base, and doesn't inherit from a non-empty
     /// class. Doesn't take union-ness into account.
----------------
Do you mind re-flowing this entire comment to 80 cols?


================
Comment at: lib/AST/Decl.cpp:3927
+  //     -- is not of class type, or
+  auto *RT = getType()->getAs<RecordType>();
+  if (!RT)
----------------
`const auto *` (elsewhere in the function as well)


================
Comment at: lib/AST/Decl.cpp:3930
+    return false;
+  auto *RD = RT->getDecl()->getDefinition();
+  if (!RD) {
----------------
I'd prefer to see a concrete type here rather than `auto`.


================
Comment at: lib/AST/RecordLayoutBuilder.cpp:1785
+    if (PotentiallyOverlapping) {
+      auto &Layout = Context.getASTRecordLayout(FieldClass);
+      EffectiveFieldSize =
----------------
Don't use `auto` here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63451