[Clang][Sema] Warn unused cxx vardecl which entirely consists condition expr of if/while/for construct

[JOE1994]

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

[llvmbot]

@llvm/pr-subscribers-clang

Author: Youngsuk Kim (JOE1994)

Changes

Fixes #41447

---

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

4 Files Affected:

• (modified) clang/include/clang/AST/Decl.h (+12)
• (modified) clang/lib/Sema/SemaDecl.cpp (+6-2)
• (modified) clang/lib/Sema/SemaDeclCXX.cpp (+3)
• (modified) clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp (+6)

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 8b44d24f5273aa..62f71658fa3b5a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2188,8 +2188,12 @@ 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 (iter->getSecond() != 0) {
+    bool UnusedCXXCondDecl = VD->isCXXCondDecl() && (iter->getSecond() == 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 e9fecaea84b021..d4d69beeba5a05 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18563,6 +18563,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..cb066e209cb453 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,9 @@ template <typename T> void f5() {
   SWarnUnused swu;
   ++swu;
 }
+
+void f6() {
+  if (int x = 123) { // expected-warning{{variable 'x' set but not used}}
+    ;
+  }
+}

[cor3ntin]

Thanks for this change. It looks reasonable.

Can you provide:

• A description in the github issue
• A release note entry
• More tests (ie the tests in the issue

[shafik]

Just to reenforce what was mentioned already. Your PR summary should briefly describe the problem and the approach your solution takes to fixing it. This is important b/c this is what usually ends up in the git log and that is critical for folks triaging build issues up and downstream without having to do more digging to understand a commit.

[JOE1994]

@cor3ntin @shafik Thank you so much for your feedback!
I've pushed an extra commit with more explanation in comments along with more test cases.
I also added an entry to Clang release notes.

[cor3ntin]

LGTM modulo nitpicks. And we still need a more descriptive commit message. Please edit #87348 (comment)

[erichkeane]

Please add the updated 'commit message' to the github interface as well, as that is the one that gets actually merged.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[JOE1994]

• Rebased on top of main to resolve merge conflicts in ReleaseNotes.
• Applied clang-format
• Updated commit message to the github interface.