[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 18 19:44:37 PDT 2019
rsmith marked 2 inline comments as done.
rsmith added inline comments.
================
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)) {
----------------
rjmccall wrote:
> Surely a bit-field of nonzero length is a subobject of nonzero size.
Usually, but not if it's unnamed (unnamed bit-fields aren't subobjects). In any case, this is a quote from the C++ standard.
================
Comment at: lib/AST/Decl.cpp:3945
+ return false;
+ }
+
----------------
rjmccall wrote:
> Is this a C/C++ modules interaction?
We don't allow C modules to be imported into C++ compilations or vice versa, so this should be unreachable unless we start allowing the attribute in C. Nice catch.
I guess the question is, then: should we allow this attribute in C (either with a GNU `__attribute__` spelling or as a C20 `[[clang::attribute]]`)? I don't think it's really useful in C (empty structs are ill-formed, and you can't reuse tail padding because structs are always trivial, at least in standard C), so I'm inclined to say no.
================
Comment at: lib/CodeGen/CGExprAgg.cpp:1850
+AggValueSlot::Overlap_t
+CodeGenFunction::overlapForFieldInit(const FieldDecl *FD) {
+ if (!FD->hasAttr<NoUniqueAddressAttr>() || !FD->getType()->isRecordType())
----------------
rjmccall wrote:
> `getOverlapForFieldInit`? `overlap` is a verb.
Good idea. (Both this and `overlapForBaseInit` are pre-existing; I'll rename both.)
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63451/new/
https://reviews.llvm.org/D63451
More information about the cfe-commits
mailing list