[clang] [Clang] Skip invalid fields when synthesizing defaulted comparisons (PR #221570)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Akash Manna (akash-manna-sky)

<details>
<summary>Changes</summary>

Fixes #<!-- -->194605

`CheckFieldDecl` already rejects `int [[clang::address_space(1)]] i;` and marks the field invalid, but `DefaultedComparisonVisitor` still visited it when synthesizing the defaulted `operator==`. `BuildFieldReferenceExpr` then tripped the `!MemberQuals.hasAddressSpace()` assertion. Both `__is_trivially_equality_comparable(S)` and a plain `a == b` reach that path.

`visitSubobjects` now skips invalid fields, the same way `SpecialMemberVisitor` and the copy-assignment synthesis already do. That covers the analyzer and the synthesizer together, and the assertion stays in place since it still holds for valid code.


---
Full diff: https://github.com/llvm/llvm-project/pull/221570.diff


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.md (+4) 
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+3) 
- (modified) clang/test/SemaCXX/cxx20-default-compare.cpp (+11) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 042d7112dbe7d..7f7d9e4813b91 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -628,6 +628,10 @@ features cannot lower the translation-unit ABI level;
 - Fixed a crash when a coroutine keyword appeared inside a mem-initializer on a
   function that is not a constructor. (#GH194298)
 
+- Fixed an assertion when a defaulted comparison operator was synthesized for a
+  class with an invalid non-static data member, such as one qualified with an
+  address space. (#GH194605)
+
 #### Bug Fixes to AST Handling
 
 - Fixed a non-deterministic ordering of unused local typedefs that made
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 4457ec58902d0..28beb44af987b 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8203,6 +8203,9 @@ class DefaultedComparisonVisitor {
       //   Unnamed bit-fields are not members ...
       if (Field->isUnnamedBitField())
         continue;
+      // Skip invalid fields; they have already been diagnosed.
+      if (Field->isInvalidDecl())
+        continue;
       // Recursively expand anonymous structs.
       if (Field->isAnonymousStructOrUnion()) {
         if (visitSubobjects(Results, Field->getType()->getAsCXXRecordDecl(),
diff --git a/clang/test/SemaCXX/cxx20-default-compare.cpp b/clang/test/SemaCXX/cxx20-default-compare.cpp
index c569e9d866970..fd5b703c566d1 100644
--- a/clang/test/SemaCXX/cxx20-default-compare.cpp
+++ b/clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -79,3 +79,14 @@ struct S {
 
 bool b = (S{} < S{}); // expected-error {{object of type 'S' cannot be compared because its 'operator<=>' is implicitly deleted}}
 }
+
+namespace GH194605 {
+struct S {
+  int [[clang::address_space(1)]] i; // expected-error {{field may not be qualified with an address space}}
+  bool operator==(const S &) const = default;
+};
+
+static_assert(!__is_trivially_equality_comparable(S));
+
+bool f(const S &a, const S &b) { return a == b; }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/221570