[clang] [Clang] Fix synthesis of defaulted operator==/<=> when class has an anonymous struct member (PR #93380)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Mital Ashok (MitalAshok)
Changes
Fixes #92497
The existing implementation did try to drill into anonymous structs and compare
them member by member, but it did not properly build up the member access
expressions to include the anonymous struct members.
Note that this is different behaviour from GCC's anonymous struct extension
where the defaulted comparison would compare `LHS.anonymous struct`
with `RHS.anonymous struct`, which would fail if there is no overload
for those types.
Example of the difference:
```c++
struct X {
struct {
bool b;
};
bool c;
templatetypename T
friend constexpr bool operator==(T L, T R) {
// With GCC, T is the type of the anonymous struct
// With Clang, this operator isn't used
// (L.b and R.b are compared directly in the below operator==)
static_assert(sizeof(T) == sizeof(bool));
return L.b == R.b;
}
friend constexpr bool operator==(const X L, const X R) = default;
};
static_assert(X{} == X{});
```
This appears to be consistent with the Microsoft extension for anonymous structs
---
Full diff: https://github.com/llvm/llvm-project/pull/93380.diff
3 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+2)
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+57-6)
- (modified) clang/test/SemaCXX/anonymous-struct.cpp (+78-1)
``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d023f53754cb3..7985ab35439d2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -794,6 +794,8 @@ Bug Fixes to C++ Support
Fixes (#GH87210), (GH89541).
- Clang no longer tries to check if an expression is immediate-escalating in
an unevaluated context.
Fixes (#GH91308).
+- Fix defaulted ``operator==``/``operator<=>`` not working properly with
+ classes that have anonymous struct members (#GH92497).
Bug Fixes to AST Handling
^
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8ab429e2a136e..f573012ceacb9 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7948,7 +7948,13 @@ class DefaultedComparisonVisitor {
case DefaultedComparisonKind::Equal:
case DefaultedComparisonKind::ThreeWay:
+ assert(
+ AnonymousStructs.empty() &&
+ "Anonymous structs stack should be empty before visiting
subobjects");
getDerived().visitSubobjects(Results, RD, ParamLvalType.getQualifiers());
+ assert(AnonymousStructs.empty() &&
+ "Anonymous structs stack should become empty again after visiting
"
+ "subobjects");
return Results;
case DefaultedComparisonKind::NotEqual:
@@ -7985,6 +7991,7 @@ class DefaultedComparisonVisitor {
continue;
// Recursively expand anonymous structs.
if (Field->isAnonymousStructOrUnion()) {
+AnonymousStructContextRAII R(*this, Field);
if (visitSubobjects(Results, Field->getType()->getAsCXXRecordDecl(),
Quals))
return true;
@@ -8027,6 +8034,30 @@ class DefaultedComparisonVisitor {
FunctionDecl *FD;
DefaultedComparisonKind DCK;
UnresolvedSet<16> Fns;
+ std::vector AnonymousStructs;
+
+private:
+ struct AnonymousStructContextRAII {
+DefaultedComparisonVisitor
+#ifndef NDEBUG
+FieldDecl *FD;
+#endif
+AnonymousStructContextRAII(AnonymousStructContextRAII &&) = delete;
+explicit AnonymousStructContextRAII(DefaultedComparisonVisitor ,
+FieldDecl *FD)
+: Self(Self) {
+#ifndef NDEBUG
+ this->FD = FD;
+#endif
+ Self.AnonymousStructs.push_back(FD);
+}
+~AnonymousStructContextRAII() {
+ assert(!Self.AnonymousStructs.empty() &&
+ Self.AnonymousStructs.back() == FD &&
+ "Invalid stack of anonymous structs");
+ Self.AnonymousStructs.pop_back();
+}
+ };
};
/// Information about a defaulted comparison, as determined by
@@ -8535,6 +8566,8 @@ class DefaultedComparisonSynthesizer
}
ExprPair getBase(CXXBaseSpecifier *Base) {
+assert(AnonymousStructs.empty() &&
+ "Visiting base class while inside an anonymous struct?");
ExprPair Obj = getCompleteObject();
if (Obj.first.isInvalid() || Obj.second.isInvalid())
return {ExprError(), ExprError()};
@@ -8550,12 +8583,30 @@ class DefaultedComparisonSynthesizer
if (Obj.first.isInvalid() || Obj.second.isInvalid())
return {ExprError(), ExprError()};
-DeclAccessPair Found = DeclAccessPair::make(Field, Field->getAccess());
-DeclarationNameInfo NameInfo(Field->getDeclName(), Loc);
-return {S.BuildFieldReferenceExpr(Obj.first.get(), /*IsArrow=*/false, Loc,
- CXXScopeSpec(), Field, Found, NameInfo),
-S.BuildFieldReferenceExpr(Obj.second.get(), /*IsArrow=*/false, Loc,
-
[clang] [Clang] Fix synthesis of defaulted operator==/<=> when class has an anonymous struct member (PR #93380)
https://github.com/MitalAshok created
https://github.com/llvm/llvm-project/pull/93380
Fixes #92497
The existing implementation did try to drill into anonymous structs and compare
them member by member, but it did not properly build up the member access
expressions to include the anonymous struct members.
Note that this is different behaviour from GCC's anonymous struct extension
where the defaulted comparison would compare `LHS.` with
`RHS.`, which would fail if there is no overload for those
types.
Example of the difference:
```c++
struct X {
struct {
bool b;
};
bool c;
template
friend constexpr bool operator==(T& L, T& R) {
// With GCC, T is the type of the anonymous struct
// With Clang, this operator isn't used
// (L.b and R.b are compared directly in the below operator==)
static_assert(sizeof(T) == sizeof(bool));
return L.b == R.b;
}
friend constexpr bool operator==(const X& L, const X& R) = default;
};
static_assert(X{} == X{});
```
This appears to be consistent with the Microsoft extension for anonymous structs
>From afb148f97eee38cdaa867cad4138bf7d7a6f6132 Mon Sep 17 00:00:00 2001
From: Mital Ashok
Date: Sat, 25 May 2024 15:49:10 +0100
Subject: [PATCH] [Clang] Fix synthesis of defaulted operator==/<=> when class
has an anonymous struct member
---
clang/docs/ReleaseNotes.rst | 2 +
clang/lib/Sema/SemaDeclCXX.cpp | 63 ++--
clang/test/SemaCXX/anonymous-struct.cpp | 79 -
3 files changed, 137 insertions(+), 7 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d023f53754cb3..7985ab35439d2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -794,6 +794,8 @@ Bug Fixes to C++ Support
Fixes (#GH87210), (GH89541).
- Clang no longer tries to check if an expression is immediate-escalating in
an unevaluated context.
Fixes (#GH91308).
+- Fix defaulted ``operator==``/``operator<=>`` not working properly with
+ classes that have anonymous struct members (#GH92497).
Bug Fixes to AST Handling
^
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8ab429e2a136e..f573012ceacb9 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7948,7 +7948,13 @@ class DefaultedComparisonVisitor {
case DefaultedComparisonKind::Equal:
case DefaultedComparisonKind::ThreeWay:
+ assert(
+ AnonymousStructs.empty() &&
+ "Anonymous structs stack should be empty before visiting
subobjects");
getDerived().visitSubobjects(Results, RD, ParamLvalType.getQualifiers());
+ assert(AnonymousStructs.empty() &&
+ "Anonymous structs stack should become empty again after visiting
"
+ "subobjects");
return Results;
case DefaultedComparisonKind::NotEqual:
@@ -7985,6 +7991,7 @@ class DefaultedComparisonVisitor {
continue;
// Recursively expand anonymous structs.
if (Field->isAnonymousStructOrUnion()) {
+AnonymousStructContextRAII R(*this, Field);
if (visitSubobjects(Results, Field->getType()->getAsCXXRecordDecl(),
Quals))
return true;
@@ -8027,6 +8034,30 @@ class DefaultedComparisonVisitor {
FunctionDecl *FD;
DefaultedComparisonKind DCK;
UnresolvedSet<16> Fns;
+ std::vector AnonymousStructs;
+
+private:
+ struct AnonymousStructContextRAII {
+DefaultedComparisonVisitor
+#ifndef NDEBUG
+FieldDecl *FD;
+#endif
+AnonymousStructContextRAII(AnonymousStructContextRAII &&) = delete;
+explicit AnonymousStructContextRAII(DefaultedComparisonVisitor ,
+FieldDecl *FD)
+: Self(Self) {
+#ifndef NDEBUG
+ this->FD = FD;
+#endif
+ Self.AnonymousStructs.push_back(FD);
+}
+~AnonymousStructContextRAII() {
+ assert(!Self.AnonymousStructs.empty() &&
+ Self.AnonymousStructs.back() == FD &&
+ "Invalid stack of anonymous structs");
+ Self.AnonymousStructs.pop_back();
+}
+ };
};
/// Information about a defaulted comparison, as determined by
@@ -8535,6 +8566,8 @@ class DefaultedComparisonSynthesizer
}
ExprPair getBase(CXXBaseSpecifier *Base) {
+assert(AnonymousStructs.empty() &&
+ "Visiting base class while inside an anonymous struct?");
ExprPair Obj = getCompleteObject();
if (Obj.first.isInvalid() || Obj.second.isInvalid())
return {ExprError(), ExprError()};
@@ -8550,12 +8583,30 @@ class DefaultedComparisonSynthesizer
if (Obj.first.isInvalid() || Obj.second.isInvalid())
return {ExprError(), ExprError()};
-DeclAccessPair Found = DeclAccessPair::make(Field, Field->getAccess());
-DeclarationNameInfo NameInfo(Field->getDeclName(), Loc);
-return {S.BuildFieldReferenceExpr(Obj.first.get(), /*IsArrow=*/false,
