[AST] Add C++11 attribute 'msvc::no_unique_address'

[RIscRIpt]

Add msvc::constexpr commit

[aaron.ballman]

Can you split this out into two separate reviews? That makes it easier for the reviewers so we don't mix up context as well as when we do code archeology for changes. (I also made some quick comments as I noticed things in the file, but I've not done a full review yet.)

clang/include/clang/AST/Decl.h

2961

clang/include/clang/Basic/Attr.td

3490

Type attribute? That seems surprising -- this is probably meant to be InheritableAttr?

3497

Hmmm, very odd that the msvc no unique address attribute is only supported on Itanium ABI targets. Should this be TargetMicrosoftCXXABI instead?

clang/include/clang/Basic/DiagnosticSemaKinds.td

3366–3368 ↗

(On Diff #462234)

You shouldn't need to add this diagnostic -- if the Subjects list is correct in Attr.td, we automatically diagnose this sort of thing.

[RIscRIpt]

Remove msvc::constexpr commit

[RIscRIpt]

In D134458#3809570, @aaron.ballman wrote:

Can you split this out into two separate reviews?

Sure, done: https://reviews.llvm.org/D134475

clang/include/clang/Basic/Attr.td

3497

My fault, I did blind copy-paste completely forgetting about ABI!

[RIscRIpt]

Add naive implementation ABI of [[msvc::no_unique_address]]

[RIscRIpt]

TODO: add proper ABI tests of [[msvc::no_unique_address]]

[aaron.ballman]

clang/include/clang/Basic/Attr.td

3492

Does the 201803 value match what MSVC returns from __has_cpp_attribute?

clang/test/AST/msvc-attrs.cpp

4

Thanks for the start to the tests! You should also add CodeGen tests as well as Sema tests. The CodeGen tests are to make sure we're codegenning the correct offsets to members, and the Sema tests are to ensure we're matching diagnostic behavior with MSVC rather than accepting code MSVC rejects.

[RIscRIpt]

Abandoning due to lack of time and expertise. I might come back and tackle it in several months.

clang/include/clang/AST/Decl.h

public:
   bool isMutable() const { return Mutable; }
 
   /// Determines whether this field is a bitfield.
   bool isBitField() const { return BitField; }
 
   /// Determines whether this is an unnamed bitfield.
   bool isUnnamedBitfield() const { return isBitField() && !getDeclName(); }
 
+  /// Determins whether this field has no unique address
+  bool hasNoUniqueAddress() const;
+
   /// Determines whether this field is a
   /// representative for an anonymous struct or union. Such fields are
   /// unnamed and are implicitly generated by the implementation to
   /// store the data for the anonymous union or struct.
   bool isAnonymousStructOrUnion() const;
 
   Expr *getBitWidth() const {
     if (!BitField)

clang/include/clang/Basic/Attr.td

def Pointer : InheritableAttr {
   let Subjects = SubjectList<[Struct]>;
   let Args = [TypeArgument<"DerefType", /*opt=*/1>];
   let Documentation = [LifetimePointerDocs];
 }
 def : MutualExclusions<[Owner, Pointer]>;
 
 // Microsoft-related attributes
 
 def MSConstexpr : DeclOrTypeAttr {
   let LangOpts = [MicrosoftExt];
   let Spellings = [CXX11<"msvc", "constexpr">];
 //  let Subjects = [Function];
   let Documentation = [MSConstexprDocs];
 }
 
+def MSNoUniqueAddress : InheritableAttr, TargetSpecificAttr<TargetItaniumCXXABI> {
+  let LangOpts = [MicrosoftExt];
+  let Spellings = [CXX11<"msvc", "no_unique_address", 201803>];
+  let Subjects = SubjectList<[NonBitField], ErrorDiag>;
+  let Documentation = [NoUniqueAddressDocs];
+  let SimpleHandler = 1;
+}
+
 def MSNoVTable : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> {
   let Spellings = [Declspec<"novtable">];
   let Subjects = SubjectList<[CXXRecord]>;
   let Documentation = [MSNoVTableDocs];
   let SimpleHandler = 1;
 }
 
 def : IgnoredAttr {

clang/lib/AST/Decl.cpp

 }
 
 FieldDecl *FieldDecl::CreateDeserialized(ASTContext &C, unsigned ID) {
   return new (C, ID) FieldDecl(Field, nullptr, SourceLocation(),
                                SourceLocation(), nullptr, QualType(), nullptr,
                                nullptr, false, ICIS_NoInit);
 }
 
+bool FieldDecl::hasNoUniqueAddress() const {
+  return hasAttr<NoUniqueAddressAttr>() || hasAttr<MSNoUniqueAddressAttr>();
+}
+
 bool FieldDecl::isAnonymousStructOrUnion() const {
   if (!isImplicit() || getDeclName())
     return false;
 
   if (const auto *Record = getType()->getAs<RecordType>())
     return Record->getDecl()->isAnonymousStructOrUnion();
 
   return false;

 
 bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
   if (isZeroLengthBitField(Ctx))
     return true;
 
   // C++2a [intro.object]p7:
   //   An object has nonzero size if it
   //     -- is not a potentially-overlapping subobject, or
-  if (!hasAttr<NoUniqueAddressAttr>())
+  if (!hasNoUniqueAddress())
     return false;
 
   //     -- is not of class type, or
   const auto *RT = getType()->getAs<RecordType>();
   if (!RT)
     return false;
   const RecordDecl *RD = RT->getDecl()->getDefinition();
   if (!RD) {

clang/lib/AST/RecordLayoutBuilder.cpp

 bool
 EmptySubobjectMap::CanPlaceFieldAtOffset(const FieldDecl *FD,
                                          CharUnits Offset) {
   if (!CanPlaceFieldSubobjectAtOffset(FD, Offset))
     return false;
 
   // We are able to place the member variable at this offset.
   // Make sure to update the empty field subobject map.
-  UpdateEmptyFieldSubobjects(FD, Offset, FD->hasAttr<NoUniqueAddressAttr>());
+  UpdateEmptyFieldSubobjects(FD, Offset, FD->hasNoUniqueAddress());
   return true;
 }
 
 void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
     const CXXRecordDecl *RD, const CXXRecordDecl *Class, CharUnits Offset,
     bool PlacingOverlappingField) {
   // We know that the only empty subobjects that can conflict with empty
   // field subobjects are subobjects of empty bases and potentially-overlapping
UnadjustedAlignment =
       std::max(UnadjustedAlignment, Context.toCharUnitsFromBits(FieldAlign));
   UpdateAlignment(Context.toCharUnitsFromBits(FieldAlign),
                   Context.toCharUnitsFromBits(UnpackedFieldAlign));
 }
 
 void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
                                              bool InsertExtraPadding) {
   auto *FieldClass = D->getType()->getAsCXXRecordDecl();
-  bool PotentiallyOverlapping = D->hasAttr<NoUniqueAddressAttr>() && FieldClass;
+  bool PotentiallyOverlapping = D->hasNoUniqueAddress() && FieldClass;
   bool IsOverlappingEmptyField =
       PotentiallyOverlapping && FieldClass->isEmpty();
 
   CharUnits FieldOffset =
       (IsUnion || IsOverlappingEmptyField) ? CharUnits::Zero() : getDataSize();
 
   const bool DefaultsToAIXPowerAlignment =
       Context.getTargetInfo().defaultsToAIXPowerAlignment();

clang/lib/CodeGen/CGExprAgg.cpp

EmitAggExpr(E, AggValueSlot::forLValue(
                      LV, *this, AggValueSlot::IsNotDestructed,
                      AggValueSlot::DoesNotNeedGCBarriers,
                      AggValueSlot::IsNotAliased, AggValueSlot::DoesNotOverlap));
   return LV;
 }
 
 AggValueSlot::Overlap_t
 CodeGenFunction::getOverlapForFieldInit(const FieldDecl *FD) {
-  if (!FD->hasAttr<NoUniqueAddressAttr>() || !FD->getType()->isRecordType())
+  if (!FD->hasNoUniqueAddress() || !FD->getType()->isRecordType())
     return AggValueSlot::DoesNotOverlap;
 
   // If the field lies entirely within the enclosing class's nvsize, its tail
   // padding cannot overlap any already-initialized object. (The only subobjects
   // with greater addresses that might already be initialized are vbases.)
   const RecordDecl *ClassRD = FD->getParent();
   const ASTRecordLayout &Layout = getContext().getASTRecordLayout(ClassRD);
   if (Layout.getFieldOffset(FD->getFieldIndex()) +

clang/lib/CodeGen/CGExprConstant.cpp

for (FieldDecl *Field : RD->fields()) {
 
     if (!Field->isBitField()) {
       // Handle non-bitfield members.
       if (!AppendField(Field, Layout.getFieldOffset(FieldNo), EltInit,
                        AllowOverwrite))
         return false;
       // After emitting a non-empty field with [[no_unique_address]], we may
       // need to overwrite its tail padding.
-      if (Field->hasAttr<NoUniqueAddressAttr>())
+      if (Field->hasNoUniqueAddress())
         AllowOverwrite = true;
     } else {
       // Otherwise we have a bitfield.
       if (auto *CI = dyn_cast<llvm::ConstantInt>(EltInit)) {
         if (!AppendBitField(Field, Layout.getFieldOffset(FieldNo), CI,
                             AllowOverwrite))
           return false;
       } else {
for (RecordDecl::field_iterator Field = RD->field_begin(),
 
     if (!Field->isBitField()) {
       // Handle non-bitfield members.
       if (!AppendField(*Field, Layout.getFieldOffset(FieldNo) + OffsetBits,
                        EltInit, AllowOverwrite))
         return false;
       // After emitting a non-empty field with [[no_unique_address]], we may
       // need to overwrite its tail padding.
-      if (Field->hasAttr<NoUniqueAddressAttr>())
+      if (Field->hasNoUniqueAddress())
         AllowOverwrite = true;
     } else {
       // Otherwise we have a bitfield.
       if (!AppendBitField(*Field, Layout.getFieldOffset(FieldNo) + OffsetBits,
                           cast<llvm::ConstantInt>(EltInit), AllowOverwrite))
         return false;
     }
   }

clang/lib/CodeGen/CGRecordLayoutBuilder.cpp

if (!Member->Data && Member->Kind != MemberInfo::Scissor)
       continue;
     if (Member->Offset < Tail) {
       assert(Prior->Kind == MemberInfo::Field &&
              "Only storage fields have tail padding!");
       if (!Prior->FD || Prior->FD->isBitField())
         Prior->Data = getByteArrayType(bitsToCharUnits(llvm::alignTo(
             cast<llvm::IntegerType>(Prior->Data)->getIntegerBitWidth(), 8)));
       else {
-        assert(Prior->FD->hasAttr<NoUniqueAddressAttr>() &&
+        assert(Prior->FD->hasNoUniqueAddress() &&
                "should not have reused this field's tail padding");
         Prior->Data = getByteArrayType(
             Context.getTypeInfoDataSizeInChars(Prior->FD->getType()).Width);
       }
     }
     if (Member->Data)
       Prior = Member;
     Tail = Prior->Offset + getSize(Prior->Data);

clang/lib/CodeGen/TargetInfo.cpp

static bool isEmptyField(ASTContext &Context, const FieldDecl *FD,
   // FIXME: We should use a predicate for whether this behavior is true in the
   // current ABI.
   //
   // The exception to the above rule are fields marked with the
   // [[no_unique_address]] attribute (since C++20).  Those do count as empty
   // according to the Itanium ABI.  The exception applies only to records,
   // not arrays of records, so we must also check whether we stripped off an
   // array type above.
-  if (isa<CXXRecordDecl>(RT->getDecl()) &&
-      (WasArray || !FD->hasAttr<NoUniqueAddressAttr>()))
+  if (isa<CXXRecordDecl>(RT->getDecl()) && (WasArray || !FD->hasNoUniqueAddress()))
     return false;
 
   return isEmptyRecord(Context, FT, AllowArrays);
 }
 
 /// isEmptyRecord - Return true iff a structure contains only empty
 /// fields. Note that a structure with a flexible array member is not
 /// considered empty.
if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
       }
 
     // Check the fields.
     for (const auto *FD : RD->fields()) {
       // Unlike isSingleElementStruct(), empty structure and array fields
       // do count.  So do anonymous bitfields that aren't zero-sized.
 
       // Like isSingleElementStruct(), ignore C++20 empty data members.
-      if (FD->hasAttr<NoUniqueAddressAttr>() &&
-          isEmptyRecord(getContext(), FD->getType(), true))
+      if (FD->hasNoUniqueAddress() && isEmptyRecord(getContext(), FD->getType(), true))
         continue;
 
       // Unlike isSingleElementStruct(), arrays do not count.
       // Nested structures still do though.
       if (!Found.isNull())
         return Ty;
       Found = GetSingleElementType(FD->getType());
     }

clang/test/AST/msvc-attrs.cpp

 // RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
-// RUN: not %clang_cc1 -Werror=ignored-attributes -ast-dump %s 2>&1 | grep "1 error generated"
+// RUN: not %clang_cc1 -Werror=ignored-attributes -ast-dump %s 2>&1 | grep "3 errors generated"
 
-// CHECK: msvc-attrs.cpp:[[@LINE+2]]:21, col:61> col:27 constexpr New1 'void *(void *)'
+struct Empty {};
+
+// CHECK: <line:[[@LINE+2]]:21, col:61> col:27 constexpr New1 'void *(void *)'
 // CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} <col:3, col:9>
 [[msvc::constexpr]] void *New1(void *where) { return where; }
+
+struct StructWithNUAField {
+  [[msvc::no_unique_address]] Empty e;
+  int f1;
+};
+static_assert(sizeof(StructWithNUAField) == sizeof(int)); // expected to fail without -fms-extensions