[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
Mital Ashok via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 1 05:02:58 PDT 2024
https://github.com/MitalAshok updated https://github.com/llvm/llvm-project/pull/92103
>From 5908130604728b9aa9b70eeb2523d368df08e68d Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Tue, 14 May 2024 08:28:19 +0100
Subject: [PATCH 1/6] [Clang] Fix definition of layout-compatible to ignore
empty classes
Also changes the behaviour of __builtin_is_layout_compatible
None of the historic nor the current definition of layout-compatible classes mention anything about base classes (other than implicitly through being standard-layout) and are defined in terms of members, not direct members.
---
clang/include/clang/AST/DeclCXX.h | 7 ++++
clang/lib/AST/DeclCXX.cpp | 36 +++++++++++++++++
clang/lib/Sema/SemaChecking.cpp | 63 +++++++++---------------------
clang/test/SemaCXX/type-traits.cpp | 11 ++++++
llvm/include/llvm/ADT/STLExtras.h | 6 +++
5 files changed, 79 insertions(+), 44 deletions(-)
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index fb52ac804849d..60a5005c51a5e 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1229,6 +1229,13 @@ class CXXRecordDecl : public RecordDecl {
/// C++11 [class]p7, specifically using the C++11 rules without any DRs.
bool isCXX11StandardLayout() const { return data().IsCXX11StandardLayout; }
+ /// If this is a standard-layout class or union, any and all data members will
+ /// be declared in the same type.
+ ///
+ /// This retrieves the type if this class has any data members,
+ /// or the current class if there is no class with fields.
+ const CXXRecordDecl *getStandardLayoutBaseWithFields() const;
+
/// Determine whether this class, or any of its class subobjects,
/// contains a mutable field.
bool hasMutableFields() const { return data().HasMutableFields; }
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 75c441293d62e..ab032a4595d46 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -561,6 +561,42 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
data().StructuralIfLiteral = false;
}
+const CXXRecordDecl *CXXRecordDecl::getStandardLayoutBaseWithFields() const {
+#ifndef NDEBUG
+ {
+ assert(
+ isStandardLayout() &&
+ "getStandardLayoutBaseWithFields called on a non-standard-layout type");
+ unsigned NumberOfBasesWithFields = 0;
+ if (!field_empty())
+ ++NumberOfBasesWithFields;
+ std::set<const CXXRecordDecl *> UniqueBases;
+ forallBases([&](const CXXRecordDecl *Base) -> bool {
+ if (!Base->field_empty())
+ ++NumberOfBasesWithFields;
+ assert(
+ UniqueBases.insert(Base->getCanonicalDecl()).second &&
+ "Standard layout struct has multiple base classes of the same type");
+ return true;
+ });
+ assert(NumberOfBasesWithFields <= 1 &&
+ "Standard layout struct has fields declared in more than one class");
+ }
+#endif
+ if (!field_empty())
+ return this;
+ const CXXRecordDecl *Result = this;
+ forallBases([&](const CXXRecordDecl *Base) -> bool {
+ if (!Base->field_empty()) {
+ // This is the base where the fields are declared; return early
+ Result = Base;
+ return false;
+ }
+ return true;
+ });
+ return Result;
+}
+
bool CXXRecordDecl::hasConstexprDestructor() const {
auto *Dtor = getDestructor();
return Dtor ? Dtor->isConstexpr() : defaultedDestructorIsConstexpr();
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecd1821651140..acd142c1932ad 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19084,7 +19084,8 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2);
/// Check if two enumeration types are layout-compatible.
-static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
+static bool isLayoutCompatible(ASTContext &C, const EnumDecl *ED1,
+ const EnumDecl *ED2) {
// C++11 [dcl.enum] p8:
// Two enumeration types are layout-compatible if they have the same
// underlying type.
@@ -19095,8 +19096,8 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
/// Check if two fields are layout-compatible.
/// Can be used on union members, which are exempt from alignment requirement
/// of common initial sequence.
-static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
- FieldDecl *Field2,
+static bool isLayoutCompatible(ASTContext &C, const FieldDecl *Field1,
+ const FieldDecl *Field2,
bool AreUnionMembers = false) {
[[maybe_unused]] const Type *Field1Parent =
Field1->getParent()->getTypeForDecl();
@@ -19139,52 +19140,26 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
/// Check if two standard-layout structs are layout-compatible.
/// (C++11 [class.mem] p17)
-static bool isLayoutCompatibleStruct(ASTContext &C, RecordDecl *RD1,
- RecordDecl *RD2) {
- // If both records are C++ classes, check that base classes match.
- if (const CXXRecordDecl *D1CXX = dyn_cast<CXXRecordDecl>(RD1)) {
- // If one of records is a CXXRecordDecl we are in C++ mode,
- // thus the other one is a CXXRecordDecl, too.
- const CXXRecordDecl *D2CXX = cast<CXXRecordDecl>(RD2);
- // Check number of base classes.
- if (D1CXX->getNumBases() != D2CXX->getNumBases())
- return false;
+static bool isLayoutCompatibleStruct(ASTContext &C, const RecordDecl *RD1,
+ const RecordDecl *RD2) {
+ // Get to the class where the fields are declared
+ if (const CXXRecordDecl *D1CXX = dyn_cast<CXXRecordDecl>(RD1))
+ RD1 = D1CXX->getStandardLayoutBaseWithFields();
- // Check the base classes.
- for (CXXRecordDecl::base_class_const_iterator
- Base1 = D1CXX->bases_begin(),
- BaseEnd1 = D1CXX->bases_end(),
- Base2 = D2CXX->bases_begin();
- Base1 != BaseEnd1;
- ++Base1, ++Base2) {
- if (!isLayoutCompatible(C, Base1->getType(), Base2->getType()))
- return false;
- }
- } else if (const CXXRecordDecl *D2CXX = dyn_cast<CXXRecordDecl>(RD2)) {
- // If only RD2 is a C++ class, it should have zero base classes.
- if (D2CXX->getNumBases() > 0)
- return false;
- }
+ if (const CXXRecordDecl *D2CXX = dyn_cast<CXXRecordDecl>(RD2))
+ RD2 = D2CXX->getStandardLayoutBaseWithFields();
// Check the fields.
- RecordDecl::field_iterator Field2 = RD2->field_begin(),
- Field2End = RD2->field_end(),
- Field1 = RD1->field_begin(),
- Field1End = RD1->field_end();
- for ( ; Field1 != Field1End && Field2 != Field2End; ++Field1, ++Field2) {
- if (!isLayoutCompatible(C, *Field1, *Field2))
- return false;
- }
- if (Field1 != Field1End || Field2 != Field2End)
- return false;
-
- return true;
+ return llvm::equal(RD1->fields(), RD2->fields(),
+ [&C](const FieldDecl *F1, const FieldDecl *F2) -> bool {
+ return isLayoutCompatible(C, F1, F2);
+ });
}
/// Check if two standard-layout unions are layout-compatible.
/// (C++11 [class.mem] p18)
-static bool isLayoutCompatibleUnion(ASTContext &C, RecordDecl *RD1,
- RecordDecl *RD2) {
+static bool isLayoutCompatibleUnion(ASTContext &C, const RecordDecl *RD1,
+ const RecordDecl *RD2) {
llvm::SmallPtrSet<FieldDecl *, 8> UnmatchedFields;
for (auto *Field2 : RD2->fields())
UnmatchedFields.insert(Field2);
@@ -19209,8 +19184,8 @@ static bool isLayoutCompatibleUnion(ASTContext &C, RecordDecl *RD1,
return UnmatchedFields.empty();
}
-static bool isLayoutCompatible(ASTContext &C, RecordDecl *RD1,
- RecordDecl *RD2) {
+static bool isLayoutCompatible(ASTContext &C, const RecordDecl *RD1,
+ const RecordDecl *RD2) {
if (RD1->isUnion() != RD2->isUnion())
return false;
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index f2fd45762abf8..14a075e01a8b1 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -1734,6 +1734,11 @@ struct CStructWithFMA2 {
int f[];
};
+template<int N>
+struct UniqueEmpty {};
+template<typename... Bases>
+struct D : Bases... {};
+
void is_layout_compatible(int n)
{
static_assert(__is_layout_compatible(void, void));
@@ -1837,6 +1842,12 @@ void is_layout_compatible(int n)
static_assert(!__is_layout_compatible(EnumClassLayout, int));
static_assert(!__is_layout_compatible(EnumForward, int));
static_assert(!__is_layout_compatible(EnumClassForward, int));
+ static_assert(__is_layout_compatible(CStruct, D<CStruct>));
+ static_assert(__is_layout_compatible(CStruct, D<UniqueEmpty<0>, CStruct>));
+ static_assert(__is_layout_compatible(CStruct, D<UniqueEmpty<0>, D<UniqueEmpty<1>, CStruct>, D<UniqueEmpty<2>>>));
+ static_assert(__is_layout_compatible(CStruct, D<CStructWithQualifiers>));
+ static_assert(__is_layout_compatible(CStruct, D<UniqueEmpty<0>, CStructWithQualifiers>));
+ static_assert(__is_layout_compatible(CStructWithQualifiers, D<UniqueEmpty<0>, D<UniqueEmpty<1>, CStruct>, D<UniqueEmpty<2>>>));
}
namespace IPIBO {
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index 08a708e5c5871..b6282380eacc0 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -2027,6 +2027,12 @@ template <typename L, typename R> bool equal(L &&LRange, R &&RRange) {
adl_end(RRange));
}
+template <typename L, typename R, typename BinaryPredicate>
+bool equal(L &&LRange, R &&RRange, BinaryPredicate P) {
+ return std::equal(adl_begin(LRange), adl_end(LRange), adl_begin(RRange),
+ adl_end(RRange), P);
+}
+
/// Returns true if all elements in Range are equal or when the Range is empty.
template <typename R> bool all_equal(R &&Range) {
auto Begin = adl_begin(Range);
>From dfb4d2a6b9cf42e3432a3c52b1dc1ef3cb2749ca Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Tue, 21 May 2024 12:42:36 +0100
Subject: [PATCH 2/6] Use EXPENSIVE_CHECKS instead of !NDEBUG
---
clang/lib/AST/DeclCXX.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index ab032a4595d46..d5f5b1e0f0dc8 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -562,15 +562,15 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
}
const CXXRecordDecl *CXXRecordDecl::getStandardLayoutBaseWithFields() const {
-#ifndef NDEBUG
+ assert(
+ isStandardLayout() &&
+ "getStandardLayoutBaseWithFields called on a non-standard-layout type");
+#ifdef EXPENSIVE_CHECKS
{
- assert(
- isStandardLayout() &&
- "getStandardLayoutBaseWithFields called on a non-standard-layout type");
unsigned NumberOfBasesWithFields = 0;
if (!field_empty())
++NumberOfBasesWithFields;
- std::set<const CXXRecordDecl *> UniqueBases;
+ llvm::SmallPtrSet<const CXXRecordDecl *, 8> UniqueBases;
forallBases([&](const CXXRecordDecl *Base) -> bool {
if (!Base->field_empty())
++NumberOfBasesWithFields;
>From adc82e6b190e86da13b10d4f08ed97a47d2a4b9d Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Tue, 21 May 2024 12:46:47 +0100
Subject: [PATCH 3/6] More const
---
clang/lib/Sema/SemaChecking.cpp | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index acd142c1932ad..7dd346bf3c447 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19081,10 +19081,10 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
//===--- Layout compatibility ----------------------------------------------//
-static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2);
+static bool isLayoutCompatible(const ASTContext &C, QualType T1, QualType T2);
/// Check if two enumeration types are layout-compatible.
-static bool isLayoutCompatible(ASTContext &C, const EnumDecl *ED1,
+static bool isLayoutCompatible(const ASTContext &C, const EnumDecl *ED1,
const EnumDecl *ED2) {
// C++11 [dcl.enum] p8:
// Two enumeration types are layout-compatible if they have the same
@@ -19096,7 +19096,7 @@ static bool isLayoutCompatible(ASTContext &C, const EnumDecl *ED1,
/// Check if two fields are layout-compatible.
/// Can be used on union members, which are exempt from alignment requirement
/// of common initial sequence.
-static bool isLayoutCompatible(ASTContext &C, const FieldDecl *Field1,
+static bool isLayoutCompatible(const ASTContext &C, const FieldDecl *Field1,
const FieldDecl *Field2,
bool AreUnionMembers = false) {
[[maybe_unused]] const Type *Field1Parent =
@@ -19140,7 +19140,7 @@ static bool isLayoutCompatible(ASTContext &C, const FieldDecl *Field1,
/// Check if two standard-layout structs are layout-compatible.
/// (C++11 [class.mem] p17)
-static bool isLayoutCompatibleStruct(ASTContext &C, const RecordDecl *RD1,
+static bool isLayoutCompatibleStruct(const ASTContext &C, const RecordDecl *RD1,
const RecordDecl *RD2) {
// Get to the class where the fields are declared
if (const CXXRecordDecl *D1CXX = dyn_cast<CXXRecordDecl>(RD1))
@@ -19158,16 +19158,15 @@ static bool isLayoutCompatibleStruct(ASTContext &C, const RecordDecl *RD1,
/// Check if two standard-layout unions are layout-compatible.
/// (C++11 [class.mem] p18)
-static bool isLayoutCompatibleUnion(ASTContext &C, const RecordDecl *RD1,
+static bool isLayoutCompatibleUnion(const ASTContext &C, const RecordDecl *RD1,
const RecordDecl *RD2) {
- llvm::SmallPtrSet<FieldDecl *, 8> UnmatchedFields;
+ llvm::SmallPtrSet<const FieldDecl *, 8> UnmatchedFields;
for (auto *Field2 : RD2->fields())
UnmatchedFields.insert(Field2);
for (auto *Field1 : RD1->fields()) {
- llvm::SmallPtrSet<FieldDecl *, 8>::iterator
- I = UnmatchedFields.begin(),
- E = UnmatchedFields.end();
+ auto I = UnmatchedFields.begin();
+ auto E = UnmatchedFields.end();
for ( ; I != E; ++I) {
if (isLayoutCompatible(C, Field1, *I, /*IsUnionMember=*/true)) {
@@ -19184,7 +19183,7 @@ static bool isLayoutCompatibleUnion(ASTContext &C, const RecordDecl *RD1,
return UnmatchedFields.empty();
}
-static bool isLayoutCompatible(ASTContext &C, const RecordDecl *RD1,
+static bool isLayoutCompatible(const ASTContext &C, const RecordDecl *RD1,
const RecordDecl *RD2) {
if (RD1->isUnion() != RD2->isUnion())
return false;
@@ -19196,7 +19195,7 @@ static bool isLayoutCompatible(ASTContext &C, const RecordDecl *RD1,
}
/// Check if two types are layout-compatible in C++11 sense.
-static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2) {
+static bool isLayoutCompatible(const ASTContext &C, QualType T1, QualType T2) {
if (T1.isNull() || T2.isNull())
return false;
>From 0a72debfdea2a3e2f80540dbc215c180b80e798d Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sat, 22 Jun 2024 09:29:02 +0100
Subject: [PATCH 4/6] Reword comment, move
CXXRecordDecl::getStandardLayoutBaseWithFields
---
clang/include/clang/AST/DeclCXX.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 60a5005c51a5e..0923736a95f97 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1210,6 +1210,13 @@ class CXXRecordDecl : public RecordDecl {
return D.HasPublicFields || D.HasProtectedFields || D.HasPrivateFields;
}
+ /// If this is a standard-layout class or union, any and all data members will
+ /// be declared in the same type.
+ ///
+ /// This retrieves the type where any fields are declared,
+ /// or the current class if there is no class with fields.
+ const CXXRecordDecl *getStandardLayoutBaseWithFields() const;
+
/// Whether this class is polymorphic (C++ [class.virtual]),
/// which means that the class contains or inherits a virtual function.
bool isPolymorphic() const { return data().Polymorphic; }
@@ -1229,13 +1236,6 @@ class CXXRecordDecl : public RecordDecl {
/// C++11 [class]p7, specifically using the C++11 rules without any DRs.
bool isCXX11StandardLayout() const { return data().IsCXX11StandardLayout; }
- /// If this is a standard-layout class or union, any and all data members will
- /// be declared in the same type.
- ///
- /// This retrieves the type if this class has any data members,
- /// or the current class if there is no class with fields.
- const CXXRecordDecl *getStandardLayoutBaseWithFields() const;
-
/// Determine whether this class, or any of its class subobjects,
/// contains a mutable field.
bool hasMutableFields() const { return data().HasMutableFields; }
>From dedf30c33b52f5ea18298246bd7cfe614f7448f9 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Wed, 31 Jul 2024 18:25:33 +0100
Subject: [PATCH 5/6] Add release note
Will be ABI breaking for Clang 20
---
clang/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3c2e0282d1c72..acd885ced08a4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -160,6 +160,9 @@ Bug Fixes in This Version
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- ``__is_layout_compatible`` no longer requires the empty bases to be the same in two
+ standard-layout classes. It now only compares non-static data members.
+
Bug Fixes to Attribute Support
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>From 79997d6342f5735fa714aca39fdbc320b3a74211 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Thu, 1 Aug 2024 12:59:39 +0100
Subject: [PATCH 6/6] Revert "Add release note"
This reverts commit dedf30c33b52f5ea18298246bd7cfe614f7448f9.
---
clang/docs/ReleaseNotes.rst | 3 ---
1 file changed, 3 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index acd885ced08a4..3c2e0282d1c72 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -160,9 +160,6 @@ Bug Fixes in This Version
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-- ``__is_layout_compatible`` no longer requires the empty bases to be the same in two
- standard-layout classes. It now only compares non-static data members.
-
Bug Fixes to Attribute Support
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
More information about the cfe-commits
mailing list