[clang] [Clang] Fix synthesis of defaulted operator==/<=> when class has an anonymous struct member (PR #93380)
Mital Ashok via cfe-commits
cfe-commits at lists.llvm.org
Sat May 25 08:17:36 PDT 2024
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.<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;
template<typename 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
>From afb148f97eee38cdaa867cad4138bf7d7a6f6132 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
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<FieldDecl *> AnonymousStructs;
+
+private:
+ struct AnonymousStructContextRAII {
+ DefaultedComparisonVisitor &Self;
+#ifndef NDEBUG
+ FieldDecl *FD;
+#endif
+ AnonymousStructContextRAII(AnonymousStructContextRAII &&) = delete;
+ explicit AnonymousStructContextRAII(DefaultedComparisonVisitor &Self,
+ 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,
- CXXScopeSpec(), Field, Found, NameInfo)};
+ auto Reference = [&](FieldDecl *FD) -> bool {
+ DeclAccessPair Found = DeclAccessPair::make(FD, FD->getAccess());
+ DeclarationNameInfo NameInfo(FD->getDeclName(), Loc);
+
+ Obj.first =
+ S.BuildFieldReferenceExpr(Obj.first.get(), /*IsArrow=*/false, Loc,
+ CXXScopeSpec(), FD, Found, NameInfo);
+ Obj.second =
+ S.BuildFieldReferenceExpr(Obj.second.get(), /*IsArrow=*/false, Loc,
+ CXXScopeSpec(), FD, Found, NameInfo);
+ if (Obj.first.isInvalid() || Obj.second.isInvalid()) {
+ Obj.first = Obj.second = ExprError();
+ return true;
+ }
+ return false;
+ };
+
+ for (FieldDecl *AnonFD : AnonymousStructs) {
+ if (Reference(AnonFD))
+ return Obj;
+ }
+
+ Reference(Field);
+ return Obj;
}
// FIXME: When expanding a subobject, register a note in the code synthesis
diff --git a/clang/test/SemaCXX/anonymous-struct.cpp b/clang/test/SemaCXX/anonymous-struct.cpp
index e1db98d2b2f50..4418c40207f19 100644
--- a/clang/test/SemaCXX/anonymous-struct.cpp
+++ b/clang/test/SemaCXX/anonymous-struct.cpp
@@ -3,6 +3,18 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
+namespace std {
+#if __cplusplus >= 202002L
+ struct strong_ordering {
+ int n;
+ constexpr operator int() const { return n; }
+ static const strong_ordering less, equal, greater;
+ };
+ constexpr strong_ordering strong_ordering::less{-1},
+ strong_ordering::equal{0}, strong_ordering::greater{1};
+#endif
+}
+
struct S {
S();
#if __cplusplus <= 199711L
@@ -191,7 +203,7 @@ typedef struct { // expected-warning {{anonymous non-C-compatible type}}
} A; // expected-note {{given name 'A' for linkage purposes by this typedef}}
}
-#if __cplusplus > 201103L
+#if __cplusplus >= 201103L
namespace GH58800 {
struct A {
union {
@@ -207,3 +219,68 @@ A GetA() {
}
}
#endif
+
+#if __cplusplus >= 202002L
+namespace GH92497 {
+struct S {
+ struct {
+ bool b;
+ };
+
+ constexpr bool operator==(const S&) const noexcept;
+};
+constexpr bool S::operator==(const S&) const noexcept = default;
+bool f(S const& a, S const& b) { return a == b; }
+static_assert(S{} == S{});
+
+template<typename T>
+struct A {
+ struct {
+ T x;
+ };
+ friend constexpr bool operator==(const A&, const A&) = default;
+// expected-note at -1 2 {{candidate function has been implicitly deleted}}
+ friend constexpr bool operator<=>(const A&, const A&) = default;
+// expected-note at -1 2 {{candidate function has been implicitly deleted}}
+};
+
+static_assert(A<int>{} == A<int>{});
+static_assert(A<int>{} <= A<int>{});
+
+struct eq_but_not_cmp {
+ constexpr bool operator==(eq_but_not_cmp) const { return true; }
+};
+static_assert(A<eq_but_not_cmp>{} == A<eq_but_not_cmp>{});
+static_assert(A<eq_but_not_cmp>{} <=> A<eq_but_not_cmp>{});
+// expected-error at -1 {{overload resolution selected deleted operator '<=>'}}
+
+struct cmp_but_not_eq {
+ constexpr bool operator==(cmp_but_not_eq) = delete;
+ constexpr std::strong_ordering operator<=>(cmp_but_not_eq) const { return std::strong_ordering::equal; }
+};
+static_assert(A<cmp_but_not_eq>{} == A<cmp_but_not_eq>{});
+// expected-error at -1 {{overload resolution selected deleted operator '=='}}
+static_assert((A<cmp_but_not_eq>{} <=> A<cmp_but_not_eq>{}) == std::strong_ordering::equal);
+
+struct neither {};
+static_assert(A<neither>{} == A<neither>{});
+// expected-error at -1 {{overload resolution selected deleted operator '=='}}
+static_assert(A<neither>{} <=> A<neither>{});
+// expected-error at -1 {{overload resolution selected deleted operator '<=>'}}
+
+struct B {
+ struct {
+ struct {
+ struct {
+ bool b;
+ };
+ };
+ };
+ friend constexpr bool operator==(const B&, const B&) = default;
+ friend constexpr bool operator<=>(const B&, const B&) = default;
+};
+
+static_assert(B{} == B{});
+static_assert(B{} <= B{});
+}
+#endif
More information about the cfe-commits
mailing list