[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)

2024-08-01 Thread Vlad Serebrennikov via cfe-commits

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)

2024-08-01 Thread Vlad Serebrennikov via cfe-commits

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)

2024-08-01 Thread Vlad Serebrennikov via cfe-commits

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)

2024-08-01 Thread Mital Ashok via cfe-commits

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)

2024-08-01 Thread Mital Ashok via cfe-commits

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)

2024-07-31 Thread John McCall via cfe-commits


@@ -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)

2024-07-31 Thread Vlad Serebrennikov via cfe-commits


@@ -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)

2024-07-31 Thread John McCall via cfe-commits


@@ -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)

2024-07-31 Thread John McCall via cfe-commits

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)

2024-07-31 Thread John McCall via cfe-commits

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)

2024-07-31 Thread Mital Ashok via cfe-commits

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)

2024-07-31 Thread Mital Ashok via cfe-commits

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)

2024-07-08 Thread Mital Ashok via cfe-commits

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)

2024-06-22 Thread Mital Ashok via cfe-commits

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)

2024-06-22 Thread Mital Ashok via cfe-commits

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)

2024-05-29 Thread John McCall via cfe-commits

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)

2024-05-29 Thread David Blaikie via cfe-commits

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)

2024-05-28 Thread Mital Ashok via cfe-commits

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)

2024-05-28 Thread David Blaikie via cfe-commits

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)

2024-05-21 Thread Mital Ashok via cfe-commits

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)

2024-05-21 Thread Mital Ashok via cfe-commits


@@ -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)

2024-05-21 Thread Mital Ashok via cfe-commits

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)

2024-05-21 Thread Vlad Serebrennikov via cfe-commits


@@ -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)

2024-05-21 Thread Vlad Serebrennikov via cfe-commits

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)

2024-05-21 Thread Vlad Serebrennikov via cfe-commits


@@ -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)

2024-05-21 Thread Vlad Serebrennikov via cfe-commits


@@ -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)

2024-05-21 Thread Vlad Serebrennikov via cfe-commits

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)

2024-05-20 Thread David Blaikie via cfe-commits

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)

2024-05-14 Thread Mital Ashok via cfe-commits

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)

2024-05-14 Thread Mital Ashok via cfe-commits

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)

2024-05-14 Thread via cfe-commits

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)

2024-05-14 Thread Mital Ashok via cfe-commits

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,
+