[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
Endilll wrote: /cherry-pick 5d7357cc9ee84578e7142c5fa7c03b1331cba6d2 https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
Endilll wrote: /cherry-pick https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
https://github.com/Endilll closed https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
MitalAshok wrote: @Endilll @AaronBallman Removed the release note and the tests have passed again. Would appreciate if someone else could start the backport process afterwards since I don't know how to do it. https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
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 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 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 standa
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
@@ -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. rjmccall wrote: Is the plan to remove the release note from LLVM 20 if we're able to backport the fix? https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
@@ -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. Endilll wrote: That is correct, but the feature made into the 19 branch, whereas this fix did not (I hope we can backport it), hence a release note. https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
@@ -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. rjmccall wrote: Hmm. I was under the impression that this feature had not been previously released; is that incorrect? https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
https://github.com/rjmccall commented: The code changes look right to me. https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
https://github.com/rjmccall edited https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
MitalAshok wrote: I am also looking to cherry-pick this for Clang 19 so there won't be a difference between Clang 19 (when `__is_layout_compatible` was added) and Clang 20 https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
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 Date: Tue, 14 May 2024 08:28:19 +0100 Subject: [PATCH 1/5] [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 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 standa
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
MitalAshok wrote: Ping https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
MitalAshok wrote: https://github.com/llvm/llvm-project/blob/b0ae923ada836fa2c9114ac2c5afb39466f49fe0/clang/docs/ReleaseNotes.rst#L210-L212 Should be enough for ReleaseNotes, since not much has changed since Clang 18 https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
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 Date: Tue, 14 May 2024 08:28:19 +0100 Subject: [PATCH 1/4] [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 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 standa
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
rjmccall wrote: Changing a type trait is generally always an ABI problem, but yeah, if this hasn't been released yet, we're still free to fix basic bugs. https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
dwblaikie wrote: Ah, OK - guess this might not be an ABI issue, then - carry on :) (I'll leave it to other Clang-y folks to do the rest of the review, the ABI issues were my only concern) https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
MitalAshok wrote: Clang only uses `Sema::isLayoutCompatible` in two places: to implement `__is_layout_compatible` and [Clang type safety checking attributes](https://clang.llvm.org/docs/AttributeReference.html#type-safety-checking), which warn if a pointer does not point to a layout compatible type. This only affects compiler warnings. The type safety attributes were introduced by e4a5a90e8d6bc6a5ed0bf5f152a658c680b94867 and they were exposed as `__is_layout_compatible` by d5922cf72cc18a7ac9f7afd1941ee2f7773d8469 this February, so I don't think it was available in Clang 18 https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
dwblaikie wrote: > @dwblaikie This patch will bring Clang in line with GCC and MSVC: > https://godbolt.org/z/nj715zbsW would the change be ABI incompatible with previous versions of clang? If so, then it'll need to be versioned in Clang's ClangABI handling, so that platforms (like Sony's Playstation, and Apple's platforms) that are only interested in Clang ABI compatibility can retain the old behavior. @rjmccall might have an interest in whether this is an ABI thing Apple cares about @pogo59 not sure who over at Sony's the best person to check on sony ABI things https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
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 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 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 standa
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
@@ -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 UniqueBases; MitalAshok wrote: @Endilll `llvm::SmallPtrSet` seemed more appropriate here https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
MitalAshok wrote: @dwblaikie This patch will bring Clang in line with GCC and MSVC: https://godbolt.org/z/nj715zbsW https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
@@ -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 UniqueBases; Endilll wrote: Have you considered `llvm::DenseSet`? https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
https://github.com/Endilll commented: Thank you for working on this! https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
@@ -561,6 +561,42 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) { data().StructuralIfLiteral = false; } +const CXXRecordDecl *CXXRecordDecl::getStandardLayoutBaseWithFields() const { +#ifndef NDEBUG Endilll wrote: We have `EXPENSIVE_CHECKS` macro to guard expensive checks. https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
@@ -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"); Endilll wrote: I think this assert should be on the default code path, not just for expensive checks. https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
https://github.com/Endilll edited https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
dwblaikie wrote: How's this compare with other implementations clang is trying to be compatible with (gcc (in the normal clang driver mode) and msvc (in clang-cl mode))? Would this be an ABI break against them, or is this intended as an ABI fix to align better with them? Or some third option? https://github.com/llvm/llvm-project/pull/92103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
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 Date: Tue, 14 May 2024 08:28:19 +0100 Subject: [PATCH] [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 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-l
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
https://github.com/MitalAshok updated https://github.com/llvm/llvm-project/pull/92103 >From 74e133215e7ba9049fb021eb9bbb130347496503 Mon Sep 17 00:00:00 2001 From: Mital Ashok Date: Tue, 14 May 2024 08:28:19 +0100 Subject: [PATCH] [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..f29592e1e7520 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 per C++11 rules, + /// 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..a101b61ad7392 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( +isCXX11StandardLayout() && +"getStandardLayoutBaseWithFields called on a non-standard-layout type"); +unsigned NumberOfBasesWithFields = 0; +if (!field_empty()) + ++NumberOfBasesWithFields; +std::set 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, /// Che
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Mital Ashok (MitalAshok) Changes 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. --- Full diff: https://github.com/llvm/llvm-project/pull/92103.diff 5 Files Affected: - (modified) clang/include/clang/AST/DeclCXX.h (+7) - (modified) clang/lib/AST/DeclCXX.cpp (+36) - (modified) clang/lib/Sema/SemaChecking.cpp (+19-44) - (modified) clang/test/SemaCXX/type-traits.cpp (+11) - (modified) llvm/include/llvm/ADT/STLExtras.h (+6) ``diff diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index fb52ac804849d..f29592e1e7520 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 per C++11 rules, + /// 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..a101b61ad7392 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( +isCXX11StandardLayout() && +"getStandardLayoutBaseWithFields called on a non-standard-layout type"); +unsigned NumberOfBasesWithFields = 0; +if (!field_empty()) + ++NumberOfBasesWithFields; +std::set 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, -
[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)
https://github.com/MitalAshok created https://github.com/llvm/llvm-project/pull/92103 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. >From 147f0a7bb7644bd16584f89e02edbdf4abd81855 Mon Sep 17 00:00:00 2001 From: Mital Ashok Date: Tue, 14 May 2024 08:28:19 +0100 Subject: [PATCH] [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..f29592e1e7520 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 per C++11 rules, + /// 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..a101b61ad7392 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( +isCXX11StandardLayout() && +"getStandardLayoutBaseWithFields called on a non-standard-layout type"); +unsigned NumberOfBasesWithFields = 0; +if (!field_empty()) + ++NumberOfBasesWithFields; +std::set 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, +