Implement [[msvc::no_unique_address]]

[akhuang]

Summary

This implements the [[msvc::no_unique_address]] attribute.

There is not ABI compatibility in this patch because the attribute is relatively new and there's still some uncertainty in the MSVC version.

Bug: https://github.com/llvm/llvm-project/issues/49358

[dblaikie]

clang/lib/AST/Decl.cpp

4510–4512

maybe llvm::any_of? Not especially shorter, but maybe makes it a smidge easier to read?

4524–4525

Having to check both of these in several places seems problematic - can we wrap that up somewhere? (or, maybe ideally, is there a way for msvc::no_unique_address to map to the actual NoUniqueAddressAttr as a different spelling of the same thing?)

[aaron.ballman]

Thank you for working on this!

I had some comments, but I ran out of time before I could complete my review.

clang/include/clang/Basic/Attr.td

1799

Hmmm, it would sure be nice if we could combine this attribute with NoUniqueAddress above and just have an accessor for whether it's the microsoft version or not... but I think the TargetSpecificAttr bit prevents us from doing that. CC @erichkeane in case he's got ideas.

(No changes needed currently, mostly just a cleanup question.)

1800

I was a bit shocked to learn that this really is the correct return value for __has_cpp_attribute: https://godbolt.org/z/MW9q1hPEh

clang/include/clang/Basic/AttrDocs.td

1410

"similar" makes me wonder "so how is it different?" which the docs don't answer. I think this could be stated as:

For Windows targets, ``[[no_unique_address]]`` is ignored; use ``[[msvc::no_unique_address]]`` instead.

WDYT?

clang/lib/AST/Decl.cpp

4509

Hmmm, this seems to be more like MSVC doesn't believe an empty structure or union can have zero size: https://godbolt.org/z/fj4eYvnM7

4524–4525

This was why I was hoping we could merge the two in Attr.td, but I'm not certain that will be easy.

[akhuang]

clang/lib/AST/Decl.cpp

4524–4525

What does merging the two in Attr.td mean? Could we just put the two spellings in one attribute, or would that make it impossible for clang-cl to ignore the [[no_unique_address]] spelling

[akhuang]

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

[erichkeane]

still on vaca, but doing a drive-by.

clang/include/clang/Basic/Attr.td

1799

I think I would combine this and the one above as a single attribute. Then, only 'add' it in SemaDeclAttr.cpp when the spelling is the appropriate one for the current target. IMO, msvc::no_unique_address should ONLY be valid in MSVC mode, else we encourage its propagation.

We don't need it to be a 'TargetSpecificAttr', just do the 'ignored' warning in the handler in SemaDeclAttr.td.

The accessor that we need is to just query which spelling.

1800

Thats... interesting....horrifying... else?

clang/lib/AST/Decl.cpp

4477

This ends up going away if we combine them.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1799

That's the approach I was thinking of, but then we lose some of the declarative expressiveness from Attr.td, which is unfortunate. But this is a pretty unique situation and so perhaps it's reasonable and we can try to think of a tablegen approach once we get a second attribute in the same situation.

clang/lib/AST/Decl.cpp

4524–4525

We can have multiple syntactic spellings for the same semantic attribute (e.g., __attribute__((foo)) and __attribute__((bar)) can both map to a single FooBarAttr AST node), and we have "accessors" on the AST node that let you tell which spelling was used: https://github.com/llvm/llvm-project/blob/90ecadde62f30275c35fdf7928e3477a41691d21/clang/include/clang/Basic/Attr.td#L4095

The suggestion Erich and I are thinking of is:

1. Add the additional spelling to NoUniqueAddress.
2. Add accessors to differentiate the spellings.
3. Remove the TargetSpecificAttr from NoUniqueAddress, manually implement those checks in an attribute handler in SemaDeclAttr.cpp.

Then, anywhere you care about the attribute in general, you can look for isa<NoUniqueAddress>, and anywhere you care about which spelling, you can use cast<NoUniqueAddress>(A)->isMSVC() (or whatever you name the accessors).

[akhuang]

clang/include/clang/Basic/AttrDocs.td

1410

yep, that sounds good

[akhuang]

combine attributes into the same attribute and add accessors

[akhuang]

clang/lib/AST/Decl.cpp

4524–4525

Thanks! This patch should implement this, and so far, I don't think there are any places outside of SemaDeclAttr where we have to differentiate the two.

[mstorsjo]

clang/include/clang/Basic/AttrDocs.td

1409

On MSVC targets, [[no_unique_address]] is ignored - it's not ignored for mingw targets.

[dblaikie]

clang/lib/AST/Decl.cpp

4510

4524–4525

(awesome - glad to see this was so tidy to do/that the generic attribute handling stuff mostly covers it)

clang/lib/AST/RecordLayoutBuilder.cpp

3096–3098

[akhuang]

I'm still trying to figure out the MSVC layouts. I used the EmptySubobjects class for a lot of the logic, which is not always the same as what MSVC does. (some examples of differences in https://godbolt.org/z/6cP554ddb)

[akhuang]

address small fixes

[akhuang]

more code updates and added some test cases

[akhuang]

add note to the docs about no ABI compatibility

[akhuang]

turned this into a github PR https://github.com/llvm/llvm-project/pull/65675/commits/923a43cd6386f6e57023fd8928eed0dc0ab04d57

clang/include/clang/Basic/Attr.td

def ArmSveVectorBits : TypeAttr {
   let ASTNode = 0;
 }
 
 def ArmMveStrictPolymorphism : TypeAttr, TargetSpecificAttr<TargetARM> {
   let Spellings = [Clang<"__clang_arm_mve_strict_polymorphism">];
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr<TargetItaniumCXXABI> {
-  let Spellings = [CXX11<"", "no_unique_address", 201803>];
+def NoUniqueAddress : InheritableAttr {
+  let Spellings = [CXX11<"", "no_unique_address", 201803>,
+                   CXX11<"msvc", "no_unique_address", 201803>];
+  let Accessors = [Accessor<"isDefault", [CXX11<"", "no_unique_address", 201803>]>,
+                   Accessor<"isMSVC", [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]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }
 
 def DisableTailCalls : InheritableAttr {
   let Spellings = [Clang<"disable_tail_calls">];
   let Subjects = SubjectList<[Function, ObjCMethod]>;

clang/include/clang/Basic/AttrDocs.td

template<typename T, typename Alloc> struct my_vector {
     T *p;
     [[no_unique_address]] Alloc alloc;
     // ...
   };
   static_assert(sizeof(my_vector<int, std::allocator<int>>) == sizeof(int*));
 
 ``[[no_unique_address]]`` is a standard C++20 attribute. Clang supports its use
 in C++11 onwards.
+
+  On MSVC targets, ``[[no_unique_address]]`` is ignored; use
+  ``[[msvc::no_unique_address]]`` instead.
   }];
 }
 
 def ObjCRequiresSuperDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 Some Objective-C classes allow a subclass to override a particular method in a
 parent class but expect that the overriding method also calls the overridden

clang/lib/AST/Decl.cpp

else
     Init = NewInit;
 }
 
 unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
   assert(isBitField() && "not a bitfield");
   return getBitWidth()->EvaluateKnownConstInt(Ctx).getZExtValue();
 }
 
 bool FieldDecl::isZeroLengthBitField(const ASTContext &Ctx) const {
   return isUnnamedBitfield() && !getBitWidth()->isValueDependent() &&
          getBitWidthValue(Ctx) == 0;
 }
 
 bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
   if (isZeroLengthBitField(Ctx))
     return true;
 
bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
   }
 
   //     -- [has] virtual member functions or virtual base classes, or
   //     -- has subobjects of nonzero size or bit-fields of nonzero length
   const auto *CXXRD = cast<CXXRecordDecl>(RD);
   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
   // ABI will do.
   return true;
 }
 
 bool FieldDecl::isPotentiallyOverlapping() const {
   return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
 }
 
 unsigned FieldDecl::getFieldIndex() const {
   const FieldDecl *Canonical = getCanonicalDecl();
   if (Canonical != this)
     return Canonical->getFieldIndex();
 
   if (CachedFieldIndex) return CachedFieldIndex - 1;
 

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>());
+  bool PlacingOverlappingField = FD->hasAttr<NoUniqueAddressAttr>();
+  UpdateEmptyFieldSubobjects(FD, Offset, PlacingOverlappingField);
   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

 
 namespace {
 struct MicrosoftRecordLayoutBuilder {
   struct ElementInfo {
     CharUnits Size;
     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;
 public:
   void layout(const RecordDecl *RD);
   void cxxLayout(const CXXRecordDecl *RD);
   /// Initializes size and alignment and honors some flags.
   void initializeLayout(const RecordDecl *RD);
   /// Initialized C++ layout, compute alignment and virtual alignment and
   /// existence of vfptrs and vbptrs.  Alignment is needed before the vfptr is
   /// laid out.
   void initializeCXXLayout(const CXXRecordDecl *RD);
   void layoutNonVirtualBases(const CXXRecordDecl *RD);
   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
   /// alignment.
   void layoutFields(const RecordDecl *RD);
   void layoutField(const FieldDecl *FD);
   void layoutBitField(const FieldDecl *FD);
   /// Lays out a single zero-width bit-field in the record and handles
public:
   void placeFieldAtBitOffset(uint64_t FieldOffset) {
     FieldOffsets.push_back(FieldOffset);
   }
   /// Compute the set of virtual bases for which vtordisps are required.
   void computeVtorDispSet(
       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.
   CharUnits NonVirtualSize;
   /// The data size of the record layout.
   CharUnits DataSize;
   /// The current alignment of the record layout.
   CharUnits Alignment;
MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
   // first.  We use these passes to calculate some additional aggregated
   // information about the bases, such as required alignment and the presence of
   // zero sized members.
   const ASTRecordLayout *PreviousBaseLayout = nullptr;
   bool HasPolymorphicBaseClass = false;
   // 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.
     if (Base.isVirtual()) {
       HasVBPtr = true;
       continue;
     }
     // Check for a base to share a VBPtr with.
     if (!SharedVBPtrBase && BaseLayout.hasVBPtr()) {
       SharedVBPtrBase = BaseDecl;
       HasVBPtr = true;
     }
     // Only lay out bases with extendable VFPtrs on the first pass.
     if (!BaseLayout.hasExtendableVFPtr())
       continue;
     // If we don't have a primary base, this one qualifies.
     if (!PrimaryBase) {
       PrimaryBase = BaseDecl;
       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()) {
     if (!HasPolymorphicBaseClass)
       // This class introduces polymorphism, so we need a vftable to store the
       // RTTI information.
       HasOwnVFPtr = true;
     else if (!PrimaryBase) {
       // We have a polymorphic base class but can't extend its vftable. Add a
       // new vfptr if we would use any vftable slots.
       for (CXXMethodDecl *M : RD->methods()) {
         if (MicrosoftVTableContext::hasVtableSlot(M) &&
             M->size_overridden_methods() == 0) {
           HasOwnVFPtr = true;
           break;
         }
       }
     }
   }
   // If we don't have a primary base then we have a leading object that could
   // itself lead with a zero-sized object, something we track.
   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();
       continue;
     }
     // If this is the first layout, check to see if it leads with a zero sized
     // object.  If it does, so do we.
     if (CheckLeadingLayout) {
       CheckLeadingLayout = false;
       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.
   if (!HasVBPtr)
     VBPtrOffset = CharUnits::fromQuantity(-1);
   else if (SharedVBPtrBase) {
     const ASTRecordLayout &Layout = Context.getASTRecordLayout(SharedVBPtrBase);
     VBPtrOffset = Bases[SharedVBPtrBase] + Layout.getVBPtrOffset();
if (LVA->getVersion() <= LangOptions::MSVC2015)
       return false;
   // TODO: Some later version of MSVC will change the default behavior of the
   // compiler to enable EBO by default.  When this happens, we will need an
   // additional isCompatibleWithMSVC check.
   return false;
 }
 
 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.
   bool MDCUsesEBO = recordUsesEBO(RD);
   if (PreviousBaseLayout && PreviousBaseLayout->endsWithZeroSizedObject() &&
       BaseLayout.leadsWithZeroSizedBase() && !MDCUsesEBO)
     Size++;
   ElementInfo Info = getAdjustedElementInfo(BaseLayout);
void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
 
   if (!FoundBase) {
     if (MDCUsesEBO && BaseDecl->isEmpty()) {
       assert(BaseLayout.getNonVirtualSize() == CharUnits::Zero());
       BaseOffset = CharUnits::Zero();
     } 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())
     layoutField(Field);
 }
 
 void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) {
   if (FD->isBitField()) {
     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);
 }
 
 void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
   unsigned Width = FD->getBitWidthValue(Context);
   if (Width == 0) {
     layoutZeroWidthBitField(FD);
     return;
if (UseExternalLayout) {
     placeFieldAtBitOffset(FieldBitOffset);
     auto NewSize = Context.toCharUnitsFromBits(
         llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) +
         Context.toBits(Info.Size));
     Size = std::max(Size, NewSize);
     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;
   }
 }
 
 void
 MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) {
   // Zero-width bitfields are ignored unless they follow a non-zero-width
   // bitfield.
   if (!LastFieldIsNonZeroWidthBitfield) {
     placeFieldAtOffset(IsUnion ? CharUnits::Zero() : Size);
     // TODO: Add a Sema warning that MS ignores alignment for zero
     // sized bitfields that occur after zero-size bitfields or non-bitfields.
     return;
   }
   LastFieldIsNonZeroWidthBitfield = false;
   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);
   }
 }
 
 void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) {
   if (!HasVBPtr || SharedVBPtrBase)
     return;
   // Inject the VBPointer at the injection site.
for (uint64_t &FieldOffset : FieldOffsets)
     FieldOffset += Context.toBits(Offset);
   for (BaseOffsetsMapTy::value_type &Base : Bases)
     Base.second += Offset;
 }
 
 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;
   // vtordisps respect pragma pack.
   if (!MaxFieldAlignment.isZero())
     VtorDispAlignment = std::min(VtorDispAlignment, MaxFieldAlignment);
   // The alignment of the vtordisp is at least the required alignment of the
   // entire record.  This requirement may be present to support vtordisp
for (const CXXBaseSpecifier &VBase : RD->vbases()) {
     // 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.  The padding between virtual bases is 4
     // bytes (in both 32 and 64 bits modes) and always involves rounding up to
     // the required alignment, we don't know why.
     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.
     ElementInfo Info = getAdjustedElementInfo(BaseLayout);
     CharUnits BaseOffset;
 
     // Respect the external AST source base offset, if present.
     if (UseExternalLayout) {
ASTContext::getASTRecordLayout(const RecordDecl *D) const {
   // Note that we can't save a reference to the entry because this function
   // is recursive.
   const ASTRecordLayout *Entry = ASTRecordLayouts[D];
   if (Entry) return *Entry;
 
   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,
           Builder.Alignment, Builder.RequiredAlignment, Builder.HasOwnVFPtr,
           Builder.HasOwnVFPtr || Builder.PrimaryBase, Builder.VBPtrOffset,
           Builder.DataSize, Builder.FieldOffsets, Builder.NonVirtualSize,
           Builder.Alignment, Builder.Alignment, CharUnits::Zero(),
           Builder.PrimaryBase, false, Builder.SharedVBPtrBase,
           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,
           Builder.Alignment, Builder.RequiredAlignment, Builder.Size,
           Builder.FieldOffsets);
     }
   } else {
     if (const auto *RD = dyn_cast<CXXRecordDecl>(D)) {

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->hasAttr<NoUniqueAddressAttr>()) &&
                "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);
void CGRecordLowering::insertPadding() {
   CharUnits Size = CharUnits::Zero();
   for (std::vector<MemberInfo>::const_iterator Member = Members.begin(),
                                                MemberEnd = Members.end();
        Member != MemberEnd; ++Member) {
     if (!Member->Data)
       continue;
     CharUnits Offset = Member->Offset;
     assert(Offset >= Size);
+
     // Insert padding if we need to.
     if (Offset !=
         Size.alignTo(Packed ? CharUnits::One() : getAlignment(Member->Data)))
       Padding.push_back(std::make_pair(Size, Offset - Size));
     Size = Offset + getSize(Member->Data);
   }
   if (Padding.empty())
     return;

clang/lib/Sema/SemaDeclAttr.cpp

static void handleNoMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (VDecl && !VDecl->isFunctionPointerType()) {
     S.Diag(AL.getLoc(), diag::warn_attribute_ignored_non_function_pointer)
         << AL << VDecl;
     return;
   }
   D->addAttr(NoMergeAttr::Create(S.Context, AL));
 }
 
+static void handleNoUniqueAddressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  NoUniqueAddressAttr TmpAttr(S.Context, AL);
+  if (S.getLangOpts().MSVCCompat) {
+    if (TmpAttr.isDefault()) {
+      S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+      return;
+    }
+  } else if (TmpAttr.isMSVC()) {
+    S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+    return;
+  }
+  D->addAttr(NoUniqueAddressAttr::Create(S.Context, AL));
+}
+
 static void handleSYCLKernelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   // The 'sycl_kernel' attribute applies only to function templates.
   const auto *FD = cast<FunctionDecl>(D);
   const FunctionTemplateDecl *FT = FD->getDescribedFunctionTemplate();
   assert(FT && "Function template is expected");
 
   // Function template must have at least two template parameters.
   const TemplateParameterList *TL = FT->getTemplateParameters();
case ParsedAttr::AT_ZeroCallUsedRegs:
     handleZeroCallUsedRegsAttr(S, D, AL);
     break;
   case ParsedAttr::AT_FunctionReturnThunks:
     handleFunctionReturnThunksAttr(S, D, AL);
     break;
   case ParsedAttr::AT_NoMerge:
     handleNoMergeAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_NoUniqueAddress:
+    handleNoUniqueAddressAttr(S, D, AL);
+    break;
 
   case ParsedAttr::AT_AvailableOnlyInDefaultEvalMethod:
     handleAvailableOnlyInDefaultEvalMethod(S, D, AL);
     break;
 
   // Microsoft attributes:
   case ParsedAttr::AT_LayoutVersion:
     handleLayoutVersion(S, D, AL);

clang/test/Layout/ms-no-unique-address.cpp

+// 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;
+    int i;
+  };
+  static_assert(sizeof(D) == 8);
+
+  // 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:     4 |   int i 
+  // CHECK-NEXT:       | [sizeof=8, align=4,
+  // CHECK-NEXT:       |  nvsize=8, nvalign=4]
+
+  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