[clang] [Clang] Fix -Wunused-private-field false negative with defaulted comparison operators (PR #116871)

[Chris White via cfe-commits]

https://github.com/whiteio updated https://github.com/llvm/llvm-project/pull/116871

>From 29330456191334afb6cd82ae44f496c522985877 Mon Sep 17 00:00:00 2001
From: Chris White <chriswhiteiodev at gmail.com>
Date: Tue, 19 Nov 2024 20:06:28 +0000
Subject: [PATCH 1/3] [Clang] Fix -Wunused-private-field false negative with
 defaulted comparison operators

Fix -Wunused-private-field suppressing warnings when defaulted comparison
operators are declared as friend functions. The warning should only be
suppressed for comparison operators that are class members.

Fixes #116270
---
 clang/lib/Sema/SemaDeclCXX.cpp                   |  2 +-
 clang/test/SemaCXX/warn-unused-private-field.cpp | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 7e8e321c4b90e6..7434a29191cdb5 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7535,7 +7535,7 @@ void Sema::CheckExplicitlyDefaultedFunction(Scope *S, FunctionDecl *FD) {
     return;
   }
 
-  if (DefKind.isComparison())
+  if (DefKind.isComparison() && isa<CXXRecordDecl>(FD->getDeclContext()))
     UnusedPrivateFields.clear();
 
   if (DefKind.isSpecialMember()
diff --git a/clang/test/SemaCXX/warn-unused-private-field.cpp b/clang/test/SemaCXX/warn-unused-private-field.cpp
index bf104b1a76a656..4448d4715faad0 100644
--- a/clang/test/SemaCXX/warn-unused-private-field.cpp
+++ b/clang/test/SemaCXX/warn-unused-private-field.cpp
@@ -40,6 +40,17 @@ class FriendEqDefaultCompareOutOfClass {
 
 bool operator==(const FriendEqDefaultCompareOutOfClass &, const FriendEqDefaultCompareOutOfClass &) = default;
 
+class UnusedConstPrivateField {
+ public:
+  UnusedConstPrivateField() : unused_(0) {}
+ private:
+  const int unused_; // expected-warning{{private field 'unused_' is not used}}
+};
+
+class FriendEqDefaultCompare {
+  friend auto operator==(FriendEqDefaultCompare, FriendEqDefaultCompare) -> bool = default;
+};
+
 #endif
 
 class NotFullyDefined {

>From a7885a848d7a439659af33d69a13b245c725c690 Mon Sep 17 00:00:00 2001
From: Chris White <chriswhiteiodev at gmail.com>
Date: Wed, 20 Nov 2024 18:34:52 +0000
Subject: [PATCH 2/3] Address review comments

---
 clang/lib/Sema/SemaDeclCXX.cpp                 | 11 +++++++++--
 .../test/SemaCXX/warn-unused-private-field.cpp | 18 ++++++++++++++----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 7434a29191cdb5..f7a0b3c059ec91 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7535,8 +7535,15 @@ void Sema::CheckExplicitlyDefaultedFunction(Scope *S, FunctionDecl *FD) {
     return;
   }
 
-  if (DefKind.isComparison() && isa<CXXRecordDecl>(FD->getDeclContext()))
-    UnusedPrivateFields.clear();
+  if (DefKind.isComparison()) {
+    auto PT = FD->getParamDecl(0)->getType();
+    if (const CXXRecordDecl *RD =
+            PT.getNonReferenceType()->getAsCXXRecordDecl()) {
+      for (FieldDecl *Field : RD->fields()) {
+        UnusedPrivateFields.remove(Field);
+      }
+    }
+  }
 
   if (DefKind.isSpecialMember()
           ? CheckExplicitlyDefaultedSpecialMember(cast<CXXMethodDecl>(FD),
diff --git a/clang/test/SemaCXX/warn-unused-private-field.cpp b/clang/test/SemaCXX/warn-unused-private-field.cpp
index 4448d4715faad0..9eccfc20430439 100644
--- a/clang/test/SemaCXX/warn-unused-private-field.cpp
+++ b/clang/test/SemaCXX/warn-unused-private-field.cpp
@@ -41,16 +41,26 @@ class FriendEqDefaultCompareOutOfClass {
 bool operator==(const FriendEqDefaultCompareOutOfClass &, const FriendEqDefaultCompareOutOfClass &) = default;
 
 class UnusedConstPrivateField {
- public:
-  UnusedConstPrivateField() : unused_(0) {}
- private:
-  const int unused_; // expected-warning{{private field 'unused_' is not used}}
+public:
+    UnusedConstPrivateField() : unused_(0) {}
+private:
+    const int unused_; // expected-warning{{private field 'unused_' is not used}}
+};
+
+class HasUnusedField {
+  int unused_; // expected-warning{{private field 'unused_' is not used}}
 };
 
 class FriendEqDefaultCompare {
+  int used;
   friend auto operator==(FriendEqDefaultCompare, FriendEqDefaultCompare) -> bool = default;
 };
 
+class UnrelatedFriendEqDefaultCompare {
+  friend auto operator==(UnrelatedFriendEqDefaultCompare, UnrelatedFriendEqDefaultCompare) -> bool = default;
+  int operator<=>(const UnrelatedFriendEqDefaultCompare &) const = default;
+};
+
 #endif
 
 class NotFullyDefined {

>From d95b9d995a19c10d394474fd67cde4d834476cad Mon Sep 17 00:00:00 2001
From: Chris White <chriswhiteiodev at gmail.com>
Date: Sat, 23 Nov 2024 14:55:47 +0000
Subject: [PATCH 3/3] Update release notes

---
 clang/docs/ReleaseNotes.rst                      | 14 ++++++++++++++
 clang/test/SemaCXX/warn-unused-private-field.cpp |  7 -------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 191b4cc0ce07ad..30fbdc9769c958 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -641,6 +641,20 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses dangling references for C++20's parenthesized aggregate initialization (#101957).
 
+- Fixed a bug where Clang would not emit ``-Wunused-private-field`` warnings when an unrelated class 
+  defined a defaulted comparison operator (#GH116270).
+
+  .. code-block:: c++
+
+    class A {
+    private:
+      int a; // warning: private field 'a' is not used, no diagnostic previously
+    };
+
+    class C {
+      bool operator==(const C&) = default;
+    };
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/test/SemaCXX/warn-unused-private-field.cpp b/clang/test/SemaCXX/warn-unused-private-field.cpp
index 9eccfc20430439..0cc6f687f1b35f 100644
--- a/clang/test/SemaCXX/warn-unused-private-field.cpp
+++ b/clang/test/SemaCXX/warn-unused-private-field.cpp
@@ -40,13 +40,6 @@ class FriendEqDefaultCompareOutOfClass {
 
 bool operator==(const FriendEqDefaultCompareOutOfClass &, const FriendEqDefaultCompareOutOfClass &) = default;
 
-class UnusedConstPrivateField {
-public:
-    UnusedConstPrivateField() : unused_(0) {}
-private:
-    const int unused_; // expected-warning{{private field 'unused_' is not used}}
-};
-
 class HasUnusedField {
   int unused_; // expected-warning{{private field 'unused_' is not used}}
 };