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 <mi...@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@-1 2 {{candidate function has been implicitly deleted}}
+  friend constexpr bool operator<=>(const A&, const A&) = default;
+// expected-note@-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@-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@-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@-1 {{overload resolution selected deleted operator '=='}}
+static_assert(A<neither>{} <=> A<neither>{});
+// expected-error@-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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to