[clang] Implement [[msvc::no_unique_address]] (PR #65675)
Amy Huang via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 12 12:59:53 PDT 2023
https://github.com/amykhuang updated https://github.com/llvm/llvm-project/pull/65675:
>From 923a43cd6386f6e57023fd8928eed0dc0ab04d57 Mon Sep 17 00:00:00 2001
From: Amy Huang <akhuang at google.com>
Date: Fri, 21 Jul 2023 16:30:30 -0700
Subject: [PATCH 1/2] Implement [[msvc::no_unique_address]]
This attribute should match the behavior of MSVC's [[msvc::no_unique_address]] attribute.
Bug: https://github.com/llvm/llvm-project/issues/49358
Differential Revision: https://reviews.llvm.org/D157762
---
clang/include/clang/Basic/Attr.td | 8 +-
clang/include/clang/Basic/AttrDocs.td | 4 +
clang/lib/AST/Decl.cpp | 8 +
clang/lib/AST/RecordLayoutBuilder.cpp | 191 +++++++++--
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 +
clang/lib/Sema/SemaDeclAttr.cpp | 17 +
clang/test/Layout/ms-no-unique-address.cpp | 338 ++++++++++++++++++++
7 files changed, 540 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Layout/ms-no-unique-address.cpp
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index c95db7e8049d47a..23e56cda0f67e9d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1798,11 +1798,13 @@ def ArmMveStrictPolymorphism : TypeAttr, TargetSpecificAttr<TargetARM> {
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 {
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index f11ea89d14bad0d..21e6373611272b5 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1405,6 +1405,10 @@ Example usage:
``[[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. Currently there is no guarantee of ABI
+compatibility or stability.
}];
}
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 60c80f2b075336b..d1770330070a519 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2152e69732d65c9 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2545,7 +2545,10 @@ struct MicrosoftRecordLayoutBuilder {
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 +2565,11 @@ struct MicrosoftRecordLayoutBuilder {
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 +2602,12 @@ struct MicrosoftRecordLayoutBuilder {
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 +2831,8 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
// 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 +2854,8 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
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 +2880,21 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
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 +2907,8 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
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 +2936,9 @@ static bool recordUsesEBO(const RecordDecl *RD) {
}
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.
@@ -2927,7 +2954,7 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
if (UseExternalLayout) {
FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset);
if (BaseOffset > Size) {
- Size = BaseOffset;
+ DataSize = Size = BaseOffset;
}
}
@@ -2937,14 +2964,97 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
BaseOffset = CharUnits::Zero();
} else {
// Otherwise, lay the base out at the end of the MDC.
- BaseOffset = Size = Size.alignTo(Info.Alignment);
+ BaseOffset = DataSize = Size = Size.alignTo(Info.Alignment);
}
+
+ // Place in EmptySubobjects map but don't check the position? MSVC seems to
+ // not allow fields to overlap at the end of a virtual base, but they can
+ // overlap with other bass.
+ EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset);
}
+
Bases.insert(std::make_pair(BaseDecl, BaseOffset));
Size += BaseLayout.getNonVirtualSize();
+ DataSize = Size;
PreviousBaseLayout = &BaseLayout;
}
+BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo(
+ const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) {
+ // This is copied directly from ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo.
+ 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 +3066,48 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) {
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();
+ const CXXRecordDecl *ParentClass = cast<CXXRecordDecl>(FD->getParent());
+ bool HasBases =
+ !ParentClass->bases().empty() || !ParentClass->vbases().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)) {
+ if (FieldOffset == CharUnits::Zero() && DataSize != CharUnits::Zero() &&
+ HasBases) {
+ // MSVC appears to only do this when there are base classes;
+ // otherwise it overlaps no_unique_address fields in non-zero offsets.
+ FieldOffset = DataSize.alignTo(Info.Alignment);
+ } else {
+ 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);
}
@@ -2999,17 +3139,17 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
auto NewSize = Context.toCharUnitsFromBits(
llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) +
Context.toBits(Info.Size));
- Size = std::max(Size, NewSize);
+ DataSize = 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;
}
@@ -3029,13 +3169,13 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) {
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);
}
}
@@ -3055,7 +3195,7 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) {
// It is possible that there were no fields or bases located after vbptr,
// so the size was not adjusted before.
if (Size < FieldStart)
- Size = FieldStart;
+ DataSize = Size = FieldStart;
return;
}
// Make sure that the amount we push the fields back by is a multiple of the
@@ -3103,6 +3243,7 @@ void MicrosoftRecordLayoutBuilder::injectVFPtr(const CXXRecordDecl *RD) {
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 +3277,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
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.
@@ -3154,7 +3295,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
VBases.insert(std::make_pair(BaseDecl,
ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp)));
- Size = BaseOffset + BaseLayout.getNonVirtualSize();
+ DataSize = Size = BaseOffset + BaseLayout.getNonVirtualSize();
PreviousBaseLayout = &BaseLayout;
}
}
@@ -3304,8 +3445,9 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const {
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 +3459,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const {
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,
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 270ff11559417d9..792ff3ce45cf3a3 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -743,6 +743,7 @@ void CGRecordLowering::calculateZeroInit() {
void CGRecordLowering::clipTailPadding() {
std::vector<MemberInfo>::iterator Prior = Members.begin();
CharUnits Tail = getSize(Prior->Data);
+
for (std::vector<MemberInfo>::iterator Member = Prior + 1,
MemberEnd = Members.end();
Member != MemberEnd; ++Member) {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 59e0f3e83cfdd80..370dfdeb800b38f 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8368,6 +8368,20 @@ static void handleNoMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
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);
@@ -9273,6 +9287,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
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);
diff --git a/clang/test/Layout/ms-no-unique-address.cpp b/clang/test/Layout/ms-no-unique-address.cpp
new file mode 100644
index 000000000000000..b03c63cf9bc67f0
--- /dev/null
+++ b/clang/test/Layout/ms-no-unique-address.cpp
@@ -0,0 +1,338 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-windows-msvc -fms-compatibility -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]
+
+ struct J {
+ [[msvc::no_unique_address]] A4 a;
+ A4 b;
+ };
+ static_assert(sizeof(J) == 16);
+
+ // MSVC puts a and b at the same offset.
+ // 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]
+
+ 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 Empty {};
+ struct alignas(8) 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 Empty {
+ [[msvc::no_unique_address]] Empty a;
+ };
+ static_assert(sizeof(D) == 16);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct VBases::D
+ // CHECK-NEXT: 0 | (D vbtable pointer)
+ // CHECK-NEXT: 9 | struct VBases::Empty a
+ // CHECK-NEXT: 16 | struct VBases::Empty (virtual base) (empty)
+ // CHECK-NEXT: | [sizeof=16, align=8,
+ // CHECK-NEXT: | nvsize=16, nvalign=8]
+
+ struct E : virtual V {
+ [[msvc::no_unique_address]] B b;
+ };
+ static_assert(sizeof(E) == 24);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct VBases::E
+ // CHECK-NEXT: 0 | (E 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; };
+ struct F : virtual A {
+ [[msvc::no_unique_address]] A a;
+ [[msvc::no_unique_address]] X x;
+ };
+ static_assert(sizeof(F) == 32);
+
+ // MSVC places x after a and the total size is 48.
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct VBases::F
+ // CHECK-NEXT: 0 | (F 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]
+
+ struct G : virtual Empty {
+ int i;
+ [[msvc::no_unique_address]] A a;
+ };
+ static_assert(sizeof(G) == 16);
+
+ // MSVC places a at offset 12.
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct VBases::G
+ // CHECK-NEXT: 0 | (G vbtable pointer)
+ // CHECK-NEXT: 8 | int i
+ // CHECK-NEXT: 8 | struct VBases::A a (empty)
+ // CHECK-NEXT: 16 | struct VBases::Empty (virtual base) (empty)
+ // CHECK-NEXT: | [sizeof=16, align=8,
+ // CHECK-NEXT: | nvsize=16, nvalign=8]
+}
>From b336a0b149bc3b1a7742b301a702b71c7afd16da Mon Sep 17 00:00:00 2001
From: Amy Huang <akhuang at google.com>
Date: Tue, 12 Sep 2023 12:50:35 -0700
Subject: [PATCH 2/2] Address comments and update some test cases
---
clang/lib/AST/Decl.cpp | 18 +--
clang/lib/AST/RecordLayoutBuilder.cpp | 137 +++---------------
clang/test/Layout/ms-no-unique-address.cpp | 32 ++--
clang/test/Preprocessor/has_attribute.cpp | 2 +-
.../test/SemaCXX/cxx2a-no-unique-address.cpp | 2 +-
5 files changed, 45 insertions(+), 146 deletions(-)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index d1770330070a519..c99d5f6c19a15d6 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,19 +4505,15 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
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;
+ if (!Ctx.getTargetInfo().getCXXABI().isMicrosoft())
+ return true;
+
+ // MS ABI: nonzero if class type with class type fields
+ return !llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+ return Field->getType()->getAs<RecordType>();
+ });
}
bool FieldDecl::isPotentiallyOverlapping() const {
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 2152e69732d65c9..2f5b3be413a7b9e 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2565,11 +2565,7 @@ struct MicrosoftRecordLayoutBuilder {
void layoutNonVirtualBase(const CXXRecordDecl *RD,
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout &BaseLayout,
- const ASTRecordLayout *&PreviousBaseLayout,
- BaseSubobjectInfo *Base);
- BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
- bool IsVirtual,
- BaseSubobjectInfo *Derived);
+ const ASTRecordLayout *&PreviousBaseLayout);
void injectVFPtr(const CXXRecordDecl *RD);
void injectVBPtr(const CXXRecordDecl *RD);
/// Lays out the fields of the record. Also rounds size up to
@@ -2831,8 +2827,6 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
// 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.
@@ -2854,8 +2848,7 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase();
}
// Lay out the base.
- layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout,
- BaseInfo);
+ layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout);
}
// Figure out if we need a fresh VFPtr for this class.
if (RD->isPolymorphic()) {
@@ -2882,18 +2875,9 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
for (const CXXBaseSpecifier &Base : RD->bases()) {
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;
- }
+ if (Base.isVirtual())
continue;
- }
// Only lay out bases without extendable VFPtrs on the second pass.
if (BaseLayout.hasExtendableVFPtr()) {
@@ -2907,8 +2891,7 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase();
}
// Lay out the base.
- layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout,
- BaseInfo);
+ layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout);
VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize();
}
// Set our VBPtroffset if we know it at this point.
@@ -2938,7 +2921,7 @@ static bool recordUsesEBO(const RecordDecl *RD) {
void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
const CXXRecordDecl *RD, const CXXRecordDecl *BaseDecl,
const ASTRecordLayout &BaseLayout,
- const ASTRecordLayout *&PreviousBaseLayout, BaseSubobjectInfo *Base) {
+ const ASTRecordLayout *&PreviousBaseLayout) {
// 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.
@@ -2954,7 +2937,7 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
if (UseExternalLayout) {
FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset);
if (BaseOffset > Size) {
- DataSize = Size = BaseOffset;
+ Size = BaseOffset;
}
}
@@ -2964,13 +2947,8 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
BaseOffset = CharUnits::Zero();
} else {
// Otherwise, lay the base out at the end of the MDC.
- BaseOffset = DataSize = Size = Size.alignTo(Info.Alignment);
+ BaseOffset = Size = Size.alignTo(Info.Alignment);
}
-
- // Place in EmptySubobjects map but don't check the position? MSVC seems to
- // not allow fields to overlap at the end of a virtual base, but they can
- // overlap with other bass.
- EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset);
}
Bases.insert(std::make_pair(BaseDecl, BaseOffset));
@@ -2979,82 +2957,6 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
PreviousBaseLayout = &BaseLayout;
}
-BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo(
- const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) {
- // This is copied directly from ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo.
- 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())
@@ -3075,10 +2977,6 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) {
bool IsOverlappingEmptyField = FD->isPotentiallyOverlapping() &&
FieldClass->isEmpty() &&
FieldClass->fields().empty();
- const CXXRecordDecl *ParentClass = cast<CXXRecordDecl>(FD->getParent());
- bool HasBases =
- !ParentClass->bases().empty() || !ParentClass->vbases().empty();
-
CharUnits FieldOffset = CharUnits::Zero();
if (UseExternalLayout) {
@@ -3091,6 +2989,9 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) {
FieldOffset = DataSize.alignTo(Info.Alignment);
while (!EmptySubobjects->CanPlaceFieldAtOffset(FD, FieldOffset)) {
+ const CXXRecordDecl *ParentClass = cast<CXXRecordDecl>(FD->getParent());
+ bool HasBases = ParentClass && (!ParentClass->bases().empty() ||
+ !ParentClass->vbases().empty());
if (FieldOffset == CharUnits::Zero() && DataSize != CharUnits::Zero() &&
HasBases) {
// MSVC appears to only do this when there are base classes;
@@ -3139,20 +3040,21 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
auto NewSize = Context.toCharUnitsFromBits(
llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) +
Context.toBits(Info.Size));
- DataSize = Size = std::max(Size, NewSize);
+ Size = std::max(Size, NewSize);
Alignment = std::max(Alignment, Info.Alignment);
} else if (IsUnion) {
placeFieldAtOffset(CharUnits::Zero());
- DataSize = Size = std::max(Size, Info.Size);
+ 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);
- DataSize = Size = FieldOffset + Info.Size;
+ Size = FieldOffset + Info.Size;
Alignment = std::max(Alignment, Info.Alignment);
RemainingBitsInField = Context.toBits(Info.Size) - Width;
}
+ DataSize = Size;
}
void
@@ -3169,15 +3071,16 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) {
ElementInfo Info = getAdjustedElementInfo(FD);
if (IsUnion) {
placeFieldAtOffset(CharUnits::Zero());
- DataSize = Size = std::max(Size, Info.Size);
+ 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);
- DataSize = Size = FieldOffset;
+ Size = FieldOffset;
Alignment = std::max(Alignment, Info.Alignment);
}
+ DataSize = Size;
}
void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) {
@@ -3195,7 +3098,7 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) {
// It is possible that there were no fields or bases located after vbptr,
// so the size was not adjusted before.
if (Size < FieldStart)
- DataSize = Size = FieldStart;
+ Size = FieldStart;
return;
}
// Make sure that the amount we push the fields back by is a multiple of the
@@ -3277,7 +3180,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
if ((PreviousBaseLayout && PreviousBaseLayout->endsWithZeroSizedObject() &&
BaseLayout.leadsWithZeroSizedBase() && !recordUsesEBO(RD)) ||
HasVtordisp) {
- DataSize = Size = Size.alignTo(VtorDispAlignment) + VtorDispSize;
+ Size = Size.alignTo(VtorDispAlignment) + VtorDispSize;
Alignment = std::max(VtorDispAlignment, Alignment);
}
// Insert the virtual base.
@@ -3295,7 +3198,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
VBases.insert(std::make_pair(BaseDecl,
ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp)));
- DataSize = Size = BaseOffset + BaseLayout.getNonVirtualSize();
+ Size = BaseOffset + BaseLayout.getNonVirtualSize();
PreviousBaseLayout = &BaseLayout;
}
}
diff --git a/clang/test/Layout/ms-no-unique-address.cpp b/clang/test/Layout/ms-no-unique-address.cpp
index b03c63cf9bc67f0..c5dd80aa5fce4d6 100644
--- a/clang/test/Layout/ms-no-unique-address.cpp
+++ b/clang/test/Layout/ms-no-unique-address.cpp
@@ -148,27 +148,27 @@ namespace Empty {
~OversizedEmpty();
[[msvc::no_unique_address]] A a;
};
- static_assert(sizeof(OversizedEmpty) == 2);
+ static_assert(sizeof(OversizedEmpty) == 1);
// 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]
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: | [sizeof=1, align=1,
+ // CHECK-NEXT: | nvsize=1, nvalign=1]
struct HasOversizedEmpty {
[[msvc::no_unique_address]] OversizedEmpty m;
};
- static_assert(sizeof(HasOversizedEmpty) == 2);
+ static_assert(sizeof(HasOversizedEmpty) == 1);
// 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]
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: | [sizeof=1, align=1,
+ // CHECK-NEXT: | nvsize=1, nvalign=1]
struct EmptyWithNonzeroDSize {
[[msvc::no_unique_address]] A a;
@@ -279,7 +279,7 @@ namespace VBases {
// CHECK:*** Dumping AST Record Layout
// CHECK: 0 | struct VBases::D
// CHECK-NEXT: 0 | (D vbtable pointer)
- // CHECK-NEXT: 9 | struct VBases::Empty a
+ // CHECK-NEXT: 8 | struct VBases::Empty a
// CHECK-NEXT: 16 | struct VBases::Empty (virtual base) (empty)
// CHECK-NEXT: | [sizeof=16, align=8,
// CHECK-NEXT: | nvsize=16, nvalign=8]
@@ -305,20 +305,20 @@ namespace VBases {
[[msvc::no_unique_address]] A a;
[[msvc::no_unique_address]] X x;
};
- static_assert(sizeof(F) == 32);
+ static_assert(sizeof(F) == 24);
// MSVC places x after a and the total size is 48.
// CHECK:*** Dumping AST Record Layout
// CHECK: 0 | struct VBases::F
// CHECK-NEXT: 0 | (F vbtable pointer)
- // CHECK-NEXT: 16 | struct VBases::A a (empty)
+ // CHECK-NEXT: 8 | 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]
+ // CHECK-NEXT: 16 | struct VBases::A a (empty)
+ // CHECK-NEXT: 24 | struct VBases::A (virtual base) (empty)
+ // CHECK-NEXT: 24 | struct VBases::A (virtual base) (empty)
+ // CHECK-NEXT: | [sizeof=24, align=8,
+ // CHECK-NEXT: | nvsize=24, nvalign=8]
struct G : virtual Empty {
int i;
diff --git a/clang/test/Preprocessor/has_attribute.cpp b/clang/test/Preprocessor/has_attribute.cpp
index bf0f9b3bc4a8f08..53b3792d5664797 100644
--- a/clang/test/Preprocessor/has_attribute.cpp
+++ b/clang/test/Preprocessor/has_attribute.cpp
@@ -55,7 +55,7 @@ CXX11(unlikely)
// CHECK: likely: 201803L
// CHECK: maybe_unused: 201603L
// ITANIUM: no_unique_address: 201803L
-// WINDOWS: no_unique_address: 0
+// WINDOWS: no_unique_address: 201803L
// CHECK: nodiscard: 201907L
// CHECK: noreturn: 200809L
// CHECK: unlikely: 201803L
diff --git a/clang/test/SemaCXX/cxx2a-no-unique-address.cpp b/clang/test/SemaCXX/cxx2a-no-unique-address.cpp
index 44d6d3acdddef70..49f987dc3865fd1 100644
--- a/clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ b/clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
+// RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-windows
[[no_unique_address]] int a; // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
[[no_unique_address]] void f(); // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
More information about the cfe-commits
mailing list