[clang] 90453f4 - [Clang][Sema] Warn unused cxx vardecl which entirely consists condition expr of if/while/for construct (#87348)

[via cfe-commits]

Author: Youngsuk Kim
Date: 2024-04-04T20:45:18-04:00
New Revision: 90453f4a9a8955ac612959504941153aa376cb0c

URL: https://github.com/llvm/llvm-project/commit/90453f4a9a8955ac612959504941153aa376cb0c
DIFF: https://github.com/llvm/llvm-project/commit/90453f4a9a8955ac612959504941153aa376cb0c.diff

LOG: [Clang][Sema] Warn unused cxx vardecl which entirely consists condition expr of if/while/for construct (#87348)

Emit `-Wunused-but-set-variable` warning on C++ variables whose
declaration (with initializer) entirely consist the condition expression
of a if/while/for construct but are not actually used in the body of the
if/while/for construct.

Fixes #41447

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/AST/Decl.h
    clang/lib/Sema/SemaDecl.cpp
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8fc925350849cd..92032c02644b82 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -334,6 +334,10 @@ Improvements to Clang's diagnostics
 - Clang now emits ``unused argument`` warning when the -fmodule-output flag is used
   with an input that is not of type c++-module.
 
+- Clang emits a ``-Wunused-but-set-variable`` warning on C++ variables whose declaration
+  (with initializer) entirely consist the condition expression of a if/while/for construct
+  but are not actually used in the body of the if/while/for construct. Fixes #GH41447
+
 Improvements to Clang's time-trace
 ----------------------------------
 

diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index a5879591f4c659..5f1f83bb00282f 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -1100,6 +1100,9 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
 
     LLVM_PREFERRED_TYPE(bool)
     unsigned EscapingByref : 1;
+
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned IsCXXCondDecl : 1;
   };
 
   union {
@@ -1589,6 +1592,15 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
     NonParmVarDeclBits.EscapingByref = true;
   }
 
+  bool isCXXCondDecl() const {
+    return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.IsCXXCondDecl;
+  }
+
+  void setCXXCondDecl() {
+    assert(!isa<ParmVarDecl>(this));
+    NonParmVarDeclBits.IsCXXCondDecl = true;
+  }
+
   /// Determines if this variable's alignment is dependent.
   bool hasDependentAlignment() const;
 

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5c1152896559b5..cbd84dd2974446 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2188,8 +2188,21 @@ void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD,
 
   assert(iter->getSecond() >= 0 &&
          "Found a negative number of references to a VarDecl");
-  if (iter->getSecond() != 0)
-    return;
+  if (int RefCnt = iter->getSecond(); RefCnt > 0) {
+    // Assume the given VarDecl is "used" if its ref count stored in
+    // `RefMinusAssignments` is positive, with one exception.
+    //
+    // For a C++ variable whose decl (with initializer) entirely consist the
+    // condition expression of a if/while/for construct,
+    // Clang creates a DeclRefExpr for the condition expression rather than a
+    // BinaryOperator of AssignmentOp. Thus, the C++ variable's ref
+    // count stored in `RefMinusAssignment` equals 1 when the variable is never
+    // used in the body of the if/while/for construct.
+    bool UnusedCXXCondDecl = VD->isCXXCondDecl() && (RefCnt == 1);
+    if (!UnusedCXXCondDecl)
+      return;
+  }
+
   unsigned DiagID = isa<ParmVarDecl>(VD) ? diag::warn_unused_but_set_parameter
                                          : diag::warn_unused_but_set_variable;
   DiagReceiver(VD->getLocation(), PDiag(DiagID) << VD);

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index f32ff396f8a543..068a2e4f04fa50 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18564,6 +18564,9 @@ DeclResult Sema::ActOnCXXConditionDeclaration(Scope *S, Declarator &D) {
     return true;
   }
 
+  if (auto *VD = dyn_cast<VarDecl>(Dcl))
+    VD->setCXXCondDecl();
+
   return Dcl;
 }
 

diff  --git a/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp b/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
index 418baa78aa964b..eaedb53bf4726b 100644
--- a/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ b/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -69,3 +69,11 @@ template <typename T> void f5() {
   SWarnUnused swu;
   ++swu;
 }
+
+void f6() {
+  if (int x = 123) {} // expected-warning{{variable 'x' set but not used}}
+
+  while (int x = 123) {} // expected-warning{{variable 'x' set but not used}}
+
+  for (; int x = 123;) {} // expected-warning{{variable 'x' set but not used}}
+}