akhuang updated this revision to Diff 551242.
akhuang marked an inline comment as done.
akhuang added a comment.
Herald added a reviewer: shafik.

Add function to check for no_unique_address attributes
Some changes to layout code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157762

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/AST/Decl.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/CodeGen/ABIInfoImpl.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/CodeGen/Targets/SystemZ.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Layout/ms-no-unique-address.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8754,7 +8754,7 @@
   CXXRecordDecl *ToD = Import(FromD, Lang_CXX20);
   EXPECT_TRUE(ToD->isEmpty());
   for (auto *FD : ToD->fields())
-    EXPECT_EQ(true, FD->hasAttr<NoUniqueAddressAttr>());
+    EXPECT_EQ(true, FD->hasNoUniqueAddress());
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase, ImportExistingTypedefToRecord) {
Index: clang/test/Layout/ms-no-unique-address.cpp
===================================================================
--- /dev/null
+++ clang/test/Layout/ms-no-unique-address.cpp
@@ -0,0 +1,310 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-pc-win32 -fdump-record-layouts %s | FileCheck %s
+
+namespace Empty {
+  struct A {};
+  struct A2 {};
+  struct A3 { [[msvc::no_unique_address]] A a; };
+  struct alignas(8) A4 {};
+
+  struct B {
+    [[msvc::no_unique_address]] A a;
+    char b;
+  };
+  static_assert(sizeof(B) == 1);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::B
+  // CHECK-NEXT:     0 |   struct Empty::A a (empty)
+  // CHECK-NEXT:     0 |   char b
+  // CHECK-NEXT:       | [sizeof=1, align=1,
+  // CHECK-NEXT:       |  nvsize=1, nvalign=1]
+
+  struct C {
+    [[msvc::no_unique_address]] A a;
+    [[msvc::no_unique_address]] A2 a2;
+    char c;
+  };
+  static_assert(sizeof(C) == 1);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::C
+  // CHECK-NEXT:     0 |   struct Empty::A a (empty)
+  // CHECK-NEXT:     0 |   struct Empty::A2 a2 (empty)
+  // CHECK-NEXT:     0 |   char c
+  // CHECK-NEXT:       | [sizeof=1, align=1,
+  // CHECK-NEXT:       |  nvsize=1, nvalign=1]
+
+  struct D {
+    [[msvc::no_unique_address]] A3 a;
+    char c;
+  };
+  static_assert(sizeof(D) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::D
+  // CHECK-NEXT:     0 |   struct Empty::A3 a (empty)
+  // CHECK-NEXT:     0 |     struct Empty::A a (empty)
+  // CHECK-NEXT:     1 |   char c 
+  // CHECK-NEXT:       | [sizeof=2, align=1,
+  // CHECK-NEXT:       |  nvsize=2, nvalign=1]
+
+  struct E {
+    [[msvc::no_unique_address]] A a1;
+    [[msvc::no_unique_address]] A a2;
+    char e;
+  };
+  static_assert(sizeof(E) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::E
+  // CHECK-NEXT:     0 |   struct Empty::A a1 (empty)
+  // CHECK-NEXT:     1 |   struct Empty::A a2 (empty)
+  // CHECK-NEXT:     0 |   char e
+  // CHECK-NEXT:       | [sizeof=2, align=1,
+  // CHECK-NEXT:       |  nvsize=2, nvalign=1]
+
+  struct F {
+    ~F();
+    [[msvc::no_unique_address]] A a1;
+    [[msvc::no_unique_address]] A a2;
+    char f;
+  };
+  static_assert(sizeof(F) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::F
+  // CHECK-NEXT:     0 |   struct Empty::A a1 (empty)
+  // CHECK-NEXT:     1 |   struct Empty::A a2 (empty)
+  // CHECK-NEXT:     0 |   char f
+  // CHECK-NEXT:       | [sizeof=2, align=1,
+  // CHECK-NEXT:       |  nvsize=2, nvalign=1]
+
+  struct G { [[msvc::no_unique_address]] A a; ~G(); };
+  static_assert(sizeof(G) == 1);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::G
+  // CHECK-NEXT:     0 |   struct Empty::A a (empty)
+  // CHECK-NEXT:       | [sizeof=1, align=1,
+  // CHECK-NEXT:       |  nvsize=1, nvalign=1]
+
+  struct H {
+    [[msvc::no_unique_address]] A a;
+    [[msvc::no_unique_address]] A b;
+    ~H();
+  };
+  static_assert(sizeof(H) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::H
+  // CHECK-NEXT:     0 |   struct Empty::A a (empty)
+  // CHECK-NEXT:     1 |   struct Empty::A b (empty)
+  // CHECK-NEXT:       | [sizeof=2, align=1,
+  // CHECK-NEXT:       |  nvsize=2, nvalign=1]
+
+  struct I {
+    [[msvc::no_unique_address]] A4 a;
+    [[msvc::no_unique_address]] A4 b;
+  };
+  static_assert(sizeof(I) == 16);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::I
+  // CHECK-NEXT:     0 |   struct Empty::A4 a (empty)
+  // CHECK-NEXT:     8 |   struct Empty::A4 b (empty)
+  // CHECK-NEXT:       | [sizeof=16, align=8,
+  // CHECK-NEXT:       |  nvsize=16, nvalign=8]
+
+  // FIXME: MSVC puts both fields at offset 0.
+  struct J {
+    [[msvc::no_unique_address]] A4 a;
+    A4 b;
+  };
+  static_assert(sizeof(J) == 16);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::J
+  // CHECK-NEXT:     0 |   struct Empty::A4 a (empty)
+  // CHECK-NEXT:     8 |   struct Empty::A4 b (empty)
+  // CHECK-NEXT:       | [sizeof=16, align=8,
+  // CHECK-NEXT:       |  nvsize=16, nvalign=8]
+
+  // FIXME: MSVC puts b at offset 1 instead of 8, and the struct size is 8.
+  struct K {
+    [[msvc::no_unique_address]] A4 a;
+    [[msvc::no_unique_address]] char c;
+    [[msvc::no_unique_address]] A4 b;
+  };
+  static_assert(sizeof(K) == 16);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::K
+  // CHECK-NEXT:     0 |   struct Empty::A4 a (empty)
+  // CHECK-NEXT:     0 |   char c
+  // CHECK-NEXT:     8 |   struct Empty::A4 b (empty)
+  // CHECK-NEXT:       | [sizeof=16, align=8,
+  // CHECK-NEXT:       |  nvsize=16, nvalign=8]
+
+  struct OversizedEmpty : A {
+    ~OversizedEmpty();
+    [[msvc::no_unique_address]] A a;
+  };
+  static_assert(sizeof(OversizedEmpty) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::OversizedEmpty
+  // CHECK-NEXT:     0 |   struct Empty::A (base) (empty)
+  // CHECK-NEXT:     1 |   struct Empty::A a (empty)
+  // CHECK-NEXT:       | [sizeof=2, align=1,
+  // CHECK-NEXT:       |  nvsize=2, nvalign=1]
+
+  struct HasOversizedEmpty {
+    [[msvc::no_unique_address]] OversizedEmpty m;
+  };
+  static_assert(sizeof(HasOversizedEmpty) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::HasOversizedEmpty
+  // CHECK-NEXT:     0 |   struct Empty::OversizedEmpty m (empty)
+  // CHECK-NEXT:     0 |     struct Empty::A (base) (empty)
+  // CHECK-NEXT:     1 |     struct Empty::A a (empty)
+  // CHECK-NEXT:       | [sizeof=2, align=1,
+  // CHECK-NEXT:       |  nvsize=2, nvalign=1]
+
+  struct EmptyWithNonzeroDSize {
+    [[msvc::no_unique_address]] A a;
+    int x;
+    [[msvc::no_unique_address]] A b;
+    int y;
+    [[msvc::no_unique_address]] A c;
+  };
+  static_assert(sizeof(EmptyWithNonzeroDSize) == 8);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::EmptyWithNonzeroDSize
+  // CHECK-NEXT:     0 |   struct Empty::A a (empty)
+  // CHECK-NEXT:     0 |   int x
+  // CHECK-NEXT:     1 |   struct Empty::A b (empty)
+  // CHECK-NEXT:     4 |   int y
+  // CHECK-NEXT:     2 |   struct Empty::A c (empty)
+  // CHECK-NEXT:       | [sizeof=8,  align=4,
+  // CHECK-NEXT:       |  nvsize=8, nvalign=4]
+
+  struct EmptyWithNonzeroDSizeNonPOD {
+    ~EmptyWithNonzeroDSizeNonPOD();
+    [[msvc::no_unique_address]] A a;
+    int x;
+    [[msvc::no_unique_address]] A b;
+    int y;
+    [[msvc::no_unique_address]] A c;
+  };
+  static_assert(sizeof(EmptyWithNonzeroDSizeNonPOD) == 8);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct Empty::EmptyWithNonzeroDSizeNonPOD
+  // CHECK-NEXT:     0 |   struct Empty::A a (empty)
+  // CHECK-NEXT:     0 |   int x
+  // CHECK-NEXT:     1 |   struct Empty::A b (empty)
+  // CHECK-NEXT:     4 |   int y
+  // CHECK-NEXT:     2 |   struct Empty::A c (empty)
+  // CHECK-NEXT:       | [sizeof=8, align=4,
+  // CHECK-NEXT:       |  nvsize=8, nvalign=4]
+}
+
+namespace POD {
+  struct A { int n; char c[3]; };
+  struct B { [[msvc::no_unique_address]] A a; char d; };
+  static_assert(sizeof(B) == 12);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct POD::B
+  // CHECK-NEXT:     0 |   struct POD::A a
+  // CHECK-NEXT:     0 |     int n
+  // CHECK-NEXT:     4 |     char[3] c
+  // CHECK-NEXT:     8 |   char d
+  // CHECK-NEXT:       | [sizeof=12,  align=4,
+  // CHECK-NEXT:       |  nvsize=12, nvalign=4]
+}
+
+namespace NonPOD {
+  struct A { int n; char c[3]; ~A(); };
+  struct B { [[msvc::no_unique_address]] A a; char d; };
+  static_assert(sizeof(B) == 12);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct NonPOD::B
+  // CHECK-NEXT:     0 |   struct NonPOD::A a
+  // CHECK-NEXT:     0 |     int n
+  // CHECK-NEXT:     4 |     char[3] c
+  // CHECK-NEXT:     8 |   char d
+  // CHECK-NEXT:       | [sizeof=12, align=4,
+  // CHECK-NEXT:       |  nvsize=12, nvalign=4]
+}
+
+namespace VBases {
+  // The nvsize of an object includes the complete size of its empty subobjects
+  // (although it's unclear why). Ensure this corner case is handled properly.
+  struct alignas(8) A { ~A(); }; // dsize 0, nvsize 0, size 8
+  struct B : A { char c; }; // dsize 1, nvsize 8, size 8
+  static_assert(sizeof(B) == 8);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct VBases::B
+  // CHECK-NEXT:     0 |   struct VBases::A (base) (empty)
+  // CHECK-NEXT:     0 |   char c
+  // CHECK-NEXT:       | [sizeof=8, align=8,
+  // CHECK-NEXT:       |  nvsize=8, nvalign=8]
+
+  struct V { int n; };
+
+  struct C : B, virtual V {};
+  static_assert(sizeof(C) == 24);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct VBases::C
+  // CHECK-NEXT:     0 |   struct VBases::B (base)
+  // CHECK-NEXT:     0 |     struct VBases::A (base) (empty)
+  // CHECK-NEXT:     0 |     char c
+  // CHECK-NEXT:     8 |   (C vbtable pointer)
+  // CHECK-NEXT:    16 |   struct VBases::V (virtual base)
+  // CHECK-NEXT:    16 |     int n
+  // CHECK-NEXT:       | [sizeof=24, align=8,
+  // CHECK-NEXT:       |  nvsize=16, nvalign=8]
+
+  struct D : virtual V {
+    [[msvc::no_unique_address]] B b;
+  };
+  static_assert(sizeof(D) == 24);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct VBases::D
+  // CHECK-NEXT:     0 |   (D vbtable pointer)
+  // CHECK-NEXT:     8 |   struct VBases::B b
+  // CHECK-NEXT:     8 |     struct VBases::A (base) (empty)
+  // CHECK-NEXT:     8 |     char c
+  // CHECK-NEXT:    16 |   struct VBases::V (virtual base)
+  // CHECK-NEXT:    16 |     int n
+  // CHECK-NEXT:       | [sizeof=24, align=8,
+  // CHECK-NEXT:       |  nvsize=16, nvalign=8]
+
+  struct X : virtual A { [[msvc::no_unique_address]] A a; };
+  // This behaves differently from MSVC. It seems like after field a,
+  // MSVC stops overlapping fields, so x is at ofset 24.
+  struct E : virtual A {
+    [[msvc::no_unique_address]] A a;
+    [[msvc::no_unique_address]] X x;
+  };
+  static_assert(sizeof(E) == 32);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:          0 | struct VBases::E
+  // CHECK-NEXT:     0 |   (E vbtable pointer)
+  // CHECK-NEXT:    16 |   struct VBases::A a (empty)
+  // CHECK-NEXT:     8 |   struct VBases::X x
+  // CHECK-NEXT:     8 |     (X vbtable pointer)
+  // CHECK-NEXT:    24 |     struct VBases::A a (empty)
+  // CHECK-NEXT:    32 |     struct VBases::A (virtual base) (empty)
+  // CHECK-NEXT:    32 |   struct VBases::A (virtual base) (empty)
+  // CHECK-NEXT:       | [sizeof=32, align=8,
+  // CHECK-NEXT:       |  nvsize=32, nvalign=8]
+}
\ No newline at end of file
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4365,6 +4365,7 @@
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
   case ParsedAttr::AT_NoUniqueAddress:
+  case ParsedAttr::AT_NoUniqueAddressMSVC:
   case ParsedAttr::AT_Likely:
   case ParsedAttr::AT_Unlikely:
     return true;
Index: clang/lib/CodeGen/Targets/SystemZ.cpp
===================================================================
--- clang/lib/CodeGen/Targets/SystemZ.cpp
+++ clang/lib/CodeGen/Targets/SystemZ.cpp
@@ -223,7 +223,7 @@
       // do count.  So do anonymous bitfields that aren't zero-sized.
 
       // Like isSingleElementStruct(), ignore C++20 empty data members.
-      if (FD->hasAttr<NoUniqueAddressAttr>() &&
+      if (FD->hasNoUniqueAddress() &&
           isEmptyRecord(getContext(), FD->getType(), true))
         continue;
 
Index: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
===================================================================
--- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -756,7 +756,7 @@
         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);
@@ -812,6 +812,7 @@
       continue;
     CharUnits Offset = Member->Offset;
     assert(Offset >= Size);
+
     // Insert padding if we need to.
     if (Offset !=
         Size.alignTo(Packed ? CharUnits::One() : getAlignment(Member->Data)))
Index: clang/lib/CodeGen/CGExprConstant.cpp
===================================================================
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -756,7 +756,7 @@
         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.
@@ -857,7 +857,7 @@
         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.
Index: clang/lib/CodeGen/CGExprAgg.cpp
===================================================================
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -2061,7 +2061,7 @@
 
 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
Index: clang/lib/CodeGen/ABIInfoImpl.cpp
===================================================================
--- clang/lib/CodeGen/ABIInfoImpl.cpp
+++ clang/lib/CodeGen/ABIInfoImpl.cpp
@@ -280,7 +280,7 @@
   // not arrays of records, so we must also check whether we stripped off an
   // array type above.
   if (isa<CXXRecordDecl>(RT->getDecl()) &&
-      (WasArray || (!AsIfNoUniqueAddr && !FD->hasAttr<NoUniqueAddressAttr>())))
+      (WasArray || (!AsIfNoUniqueAddr && !FD->hasNoUniqueAddress())))
     return false;
 
   return isEmptyRecord(Context, FT, AllowArrays, AsIfNoUniqueAddr);
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -474,7 +474,8 @@
 
   // 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>());
+  bool PlacingOverlappingField = FD->hasNoUniqueAddress();
+  UpdateEmptyFieldSubobjects(FD, Offset, PlacingOverlappingField);
   return true;
 }
 
@@ -2545,7 +2546,10 @@
     CharUnits Alignment;
   };
   typedef llvm::DenseMap<const CXXRecordDecl *, CharUnits> BaseOffsetsMapTy;
-  MicrosoftRecordLayoutBuilder(const ASTContext &Context) : Context(Context) {}
+  MicrosoftRecordLayoutBuilder(const ASTContext &Context,
+                               EmptySubobjectMap *EmptySubobjects)
+      : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
 private:
   MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
   void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2562,7 +2566,11 @@
   void layoutNonVirtualBase(const CXXRecordDecl *RD,
                             const CXXRecordDecl *BaseDecl,
                             const ASTRecordLayout &BaseLayout,
-                            const ASTRecordLayout *&PreviousBaseLayout);
+                            const ASTRecordLayout *&PreviousBaseLayout,
+                            BaseSubobjectInfo *Base);
+  BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
+                                              bool IsVirtual,
+                                              BaseSubobjectInfo *Derived);
   void injectVFPtr(const CXXRecordDecl *RD);
   void injectVBPtr(const CXXRecordDecl *RD);
   /// Lays out the fields of the record.  Also rounds size up to
@@ -2595,6 +2603,12 @@
       llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtorDispSet,
       const CXXRecordDecl *RD) const;
   const ASTContext &Context;
+  EmptySubobjectMap *EmptySubobjects;
+  llvm::SpecificBumpPtrAllocator<BaseSubobjectInfo> BaseSubobjectInfoAllocator;
+  typedef llvm::DenseMap<const CXXRecordDecl *, BaseSubobjectInfo *>
+      BaseSubobjectInfoMapTy;
+  BaseSubobjectInfoMapTy VirtualBaseInfo;
+
   /// The size of the record being laid out.
   CharUnits Size;
   /// The non-virtual size of the record layout.
@@ -2818,6 +2832,8 @@
   // Iterate through the bases and lay out the non-virtual ones.
   for (const CXXBaseSpecifier &Base : RD->bases()) {
     const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
+    BaseSubobjectInfo *BaseInfo =
+        computeBaseSubobjectInfo(BaseDecl, Base.isVirtual(), nullptr);
     HasPolymorphicBaseClass |= BaseDecl->isPolymorphic();
     const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl);
     // Mark and skip virtual bases.
@@ -2839,7 +2855,8 @@
       LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase();
     }
     // Lay out the base.
-    layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout);
+    layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout,
+                         BaseInfo);
   }
   // Figure out if we need a fresh VFPtr for this class.
   if (RD->isPolymorphic()) {
@@ -2864,10 +2881,21 @@
   bool CheckLeadingLayout = !PrimaryBase;
   // Iterate through the bases and lay out the non-virtual ones.
   for (const CXXBaseSpecifier &Base : RD->bases()) {
-    if (Base.isVirtual())
-      continue;
     const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
     const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl);
+    BaseSubobjectInfo *BaseInfo =
+        computeBaseSubobjectInfo(BaseDecl, Base.isVirtual(), nullptr);
+
+    if (Base.isVirtual()) {
+      // Mark offset for virtual base.
+      CharUnits Offset = CharUnits::Zero();
+      while (!EmptySubobjects->CanPlaceBaseAtOffset(BaseInfo, Offset)) {
+        ElementInfo Info = getAdjustedElementInfo(BaseLayout);
+        Offset += Info.Alignment;
+      }
+      continue;
+    }
+
     // Only lay out bases without extendable VFPtrs on the second pass.
     if (BaseLayout.hasExtendableVFPtr()) {
       VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize();
@@ -2880,7 +2908,8 @@
       LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase();
     }
     // Lay out the base.
-    layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout);
+    layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout,
+                         BaseInfo);
     VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize();
   }
   // Set our VBPtroffset if we know it at this point.
@@ -2908,10 +2937,9 @@
 }
 
 void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
-    const CXXRecordDecl *RD,
-    const CXXRecordDecl *BaseDecl,
+    const CXXRecordDecl *RD, const CXXRecordDecl *BaseDecl,
     const ASTRecordLayout &BaseLayout,
-    const ASTRecordLayout *&PreviousBaseLayout) {
+    const ASTRecordLayout *&PreviousBaseLayout, BaseSubobjectInfo *Base) {
   // Insert padding between two bases if the left first one is zero sized or
   // contains a zero sized subobject and the right is zero sized or one leads
   // with a zero sized base.
@@ -2938,13 +2966,94 @@
     } else {
       // Otherwise, lay the base out at the end of the MDC.
       BaseOffset = Size = Size.alignTo(Info.Alignment);
+      if (Base)
+        while (!EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset))
+          BaseOffset += Info.Alignment;
     }
   }
+
   Bases.insert(std::make_pair(BaseDecl, BaseOffset));
   Size += BaseLayout.getNonVirtualSize();
+  DataSize = Size;
   PreviousBaseLayout = &BaseLayout;
 }
 
+BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo(
+    const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) {
+  BaseSubobjectInfo *Info;
+
+  if (IsVirtual) {
+    // Check if we already have info about this virtual base.
+    BaseSubobjectInfo *&InfoSlot = VirtualBaseInfo[RD];
+    if (InfoSlot) {
+      assert(InfoSlot->Class == RD && "Wrong class for virtual base info!");
+      return InfoSlot;
+    }
+
+    // We don't, create it.
+    InfoSlot = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo;
+    Info = InfoSlot;
+  } else {
+    Info = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo;
+  }
+
+  Info->Class = RD;
+  Info->IsVirtual = IsVirtual;
+  Info->Derived = nullptr;
+  Info->PrimaryVirtualBaseInfo = nullptr;
+
+  const CXXRecordDecl *PrimaryVirtualBase = nullptr;
+  BaseSubobjectInfo *PrimaryVirtualBaseInfo = nullptr;
+
+  // Check if this base has a primary virtual base.
+  if (RD->getNumVBases()) {
+    const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
+    if (Layout.isPrimaryBaseVirtual()) {
+      // This base does have a primary virtual base.
+      PrimaryVirtualBase = Layout.getPrimaryBase();
+      assert(PrimaryVirtualBase && "Didn't have a primary virtual base!");
+
+      // Now check if we have base subobject info about this primary base.
+      PrimaryVirtualBaseInfo = VirtualBaseInfo.lookup(PrimaryVirtualBase);
+
+      if (PrimaryVirtualBaseInfo) {
+        if (PrimaryVirtualBaseInfo->Derived) {
+          // We did have info about this primary base, and it turns out that it
+          // has already been claimed as a primary virtual base for another
+          // base.
+          PrimaryVirtualBase = nullptr;
+        } else {
+          // We can claim this base as our primary base.
+          Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo;
+          PrimaryVirtualBaseInfo->Derived = Info;
+        }
+      }
+    }
+  }
+
+  // Now go through all direct bases.
+  for (const auto &I : RD->bases()) {
+    bool IsVirtual = I.isVirtual();
+
+    const CXXRecordDecl *BaseDecl = I.getType()->getAsCXXRecordDecl();
+
+    Info->Bases.push_back(computeBaseSubobjectInfo(BaseDecl, IsVirtual, Info));
+  }
+
+  if (PrimaryVirtualBase && !PrimaryVirtualBaseInfo) {
+    // Traversing the bases must have created the base info for our primary
+    // virtual base.
+    PrimaryVirtualBaseInfo = VirtualBaseInfo.lookup(PrimaryVirtualBase);
+    assert(PrimaryVirtualBaseInfo && "Did not create a primary virtual base!");
+
+    // Claim the primary virtual base as our primary virtual base.
+    Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo;
+    PrimaryVirtualBaseInfo->Derived = Info;
+  }
+
+  return Info;
+}
+
 void MicrosoftRecordLayoutBuilder::layoutFields(const RecordDecl *RD) {
   LastFieldIsNonZeroWidthBitfield = false;
   for (const FieldDecl *Field : RD->fields())
@@ -2956,18 +3065,37 @@
     layoutBitField(FD);
     return;
   }
+
   LastFieldIsNonZeroWidthBitfield = false;
   ElementInfo Info = getAdjustedElementInfo(FD);
   Alignment = std::max(Alignment, Info.Alignment);
-  CharUnits FieldOffset;
-  if (UseExternalLayout)
+
+  const CXXRecordDecl *FieldClass = FD->getType()->getAsCXXRecordDecl();
+  bool IsOverlappingEmptyField = FD->isPotentiallyOverlapping() &&
+                                 FieldClass->isEmpty() &&
+                                 FieldClass->fields().empty();
+
+  CharUnits FieldOffset = CharUnits::Zero();
+
+  if (UseExternalLayout) {
     FieldOffset =
         Context.toCharUnitsFromBits(External.getExternalFieldOffset(FD));
-  else if (IsUnion)
+  } else if (IsUnion) {
     FieldOffset = CharUnits::Zero();
-  else
+  } else if (EmptySubobjects) {
+    if (!IsOverlappingEmptyField)
+      FieldOffset = DataSize.alignTo(Info.Alignment);
+
+    while (!EmptySubobjects->CanPlaceFieldAtOffset(FD, FieldOffset))
+      FieldOffset += Info.Alignment;
+  } else {
     FieldOffset = Size.alignTo(Info.Alignment);
+  }
   placeFieldAtOffset(FieldOffset);
+
+  if (!IsOverlappingEmptyField) {
+    DataSize = std::max(DataSize, FieldOffset + Info.Size);
+  }
   Size = std::max(Size, FieldOffset + Info.Size);
 }
 
@@ -3003,13 +3131,13 @@
     Alignment = std::max(Alignment, Info.Alignment);
   } else if (IsUnion) {
     placeFieldAtOffset(CharUnits::Zero());
-    Size = std::max(Size, Info.Size);
+    DataSize = Size = std::max(Size, Info.Size);
     // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
   } else {
     // Allocate a new block of memory and place the bitfield in it.
     CharUnits FieldOffset = Size.alignTo(Info.Alignment);
     placeFieldAtOffset(FieldOffset);
-    Size = FieldOffset + Info.Size;
+    DataSize = Size = FieldOffset + Info.Size;
     Alignment = std::max(Alignment, Info.Alignment);
     RemainingBitsInField = Context.toBits(Info.Size) - Width;
   }
@@ -3029,13 +3157,13 @@
   ElementInfo Info = getAdjustedElementInfo(FD);
   if (IsUnion) {
     placeFieldAtOffset(CharUnits::Zero());
-    Size = std::max(Size, Info.Size);
+    DataSize = Size = std::max(Size, Info.Size);
     // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
   } else {
     // Round up the current record size to the field's alignment boundary.
     CharUnits FieldOffset = Size.alignTo(Info.Alignment);
     placeFieldAtOffset(FieldOffset);
-    Size = FieldOffset;
+    DataSize = Size = FieldOffset;
     Alignment = std::max(Alignment, Info.Alignment);
   }
 }
@@ -3103,6 +3231,7 @@
 void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
   if (!HasVBPtr)
     return;
+
   // Vtordisps are always 4 bytes (even in 64-bit mode)
   CharUnits VtorDispSize = CharUnits::fromQuantity(4);
   CharUnits VtorDispAlignment = VtorDispSize;
@@ -3136,7 +3265,7 @@
     if ((PreviousBaseLayout && PreviousBaseLayout->endsWithZeroSizedObject() &&
          BaseLayout.leadsWithZeroSizedBase() && !recordUsesEBO(RD)) ||
         HasVtordisp) {
-      Size = Size.alignTo(VtorDispAlignment) + VtorDispSize;
+      DataSize = Size = Size.alignTo(VtorDispAlignment) + VtorDispSize;
       Alignment = std::max(VtorDispAlignment, Alignment);
     }
     // Insert the virtual base.
@@ -3304,8 +3433,9 @@
   const ASTRecordLayout *NewEntry = nullptr;
 
   if (isMsLayout(*this)) {
-    MicrosoftRecordLayoutBuilder Builder(*this);
     if (const auto *RD = dyn_cast<CXXRecordDecl>(D)) {
+      EmptySubobjectMap EmptySubobjects(*this, RD);
+      MicrosoftRecordLayoutBuilder Builder(*this, &EmptySubobjects);
       Builder.cxxLayout(RD);
       NewEntry = new (*this) ASTRecordLayout(
           *this, Builder.Size, Builder.Alignment, Builder.Alignment,
@@ -3317,6 +3447,7 @@
           Builder.EndsWithZeroSizedObject, Builder.LeadsWithZeroSizedBase,
           Builder.Bases, Builder.VBases);
     } else {
+      MicrosoftRecordLayoutBuilder Builder(*this, /*EmptySubobjects*/ nullptr);
       Builder.layout(D);
       NewEntry = new (*this) ASTRecordLayout(
           *this, Builder.Size, Builder.Alignment, Builder.Alignment,
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4474,6 +4474,10 @@
   return getBitWidth()->EvaluateKnownConstInt(Ctx).getZExtValue();
 }
 
+bool FieldDecl::hasNoUniqueAddress() const {
+  return hasAttr<NoUniqueAddressAttr>() || hasAttr<NoUniqueAddressMSVCAttr>();
+}
+
 bool FieldDecl::isZeroLengthBitField(const ASTContext &Ctx) const {
   return isUnnamedBitfield() && !getBitWidth()->isValueDependent() &&
          getBitWidthValue(Ctx) == 0;
@@ -4486,7 +4490,7 @@
   // 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
@@ -4505,6 +4509,14 @@
   if (!CXXRD->isEmpty())
     return false;
 
+  // MS ABI: nonzero if class type with class type fields
+  if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+      llvm::any_of(CXXRD->fields(), [&](const FieldDecl *Field) {
+        return Field->getType()->getAs<RecordType>();
+      })) {
+    return false;
+  }
+
   // Otherwise, [...] the circumstances under which the object has zero size
   // are implementation-defined.
   // FIXME: This might be Itanium ABI specific; we don't yet know what the MS
@@ -4513,7 +4525,7 @@
 }
 
 bool FieldDecl::isPotentiallyOverlapping() const {
-  return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
+  return hasNoUniqueAddress() && getType()->getAsCXXRecordDecl();
 }
 
 unsigned FieldDecl::getFieldIndex() const {
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1405,6 +1405,9 @@
 
 ``[[no_unique_address]]`` is a standard C++20 attribute. Clang supports its use
 in C++11 onwards.
+
+  For Windows targets, ``[[no_unique_address]]`` is ignored, but there is a
+  similar attribute spelled ``[[msvc::no_unique_address]]``.
   }];
 }
 
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1794,6 +1794,13 @@
   let SimpleHandler = 1;
 }
 
+def NoUniqueAddressMSVC : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> {
+  let Spellings = [CXX11<"msvc", "no_unique_address", 201803>];
+  let Subjects = SubjectList<[NonBitField], ErrorDiag>;
+  let Documentation = [NoUniqueAddressDocs];
+  let SimpleHandler = 1;
+}
+
 def ReturnsTwice : InheritableAttr {
   let Spellings = [GCC<"returns_twice">];
   let Subjects = SubjectList<[Function]>;
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3131,6 +3131,8 @@
   /// Set the C++11 in-class initializer for this member.
   void setInClassInitializer(Expr *NewInit);
 
+  bool hasNoUniqueAddress() const;
+
 private:
   void setLazyInClassInitializer(LazyDeclStmtPtr NewInit);
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to