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 <mi...@mitalashok.co.uk> Date: Tue, 14 May 2024 08:28:19 +0100 Subject: [PATCH 1/3] [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 <mi...@mitalashok.co.uk> Date: Tue, 21 May 2024 12:42:36 +0100 Subject: [PATCH 2/3] 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 <mi...@mitalashok.co.uk> Date: Tue, 21 May 2024 12:46:47 +0100 Subject: [PATCH 3/3] 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits