Skip to content

Commit 90453f4

Browse files
authored
[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
1 parent ab80d00 commit 90453f4

File tree

5 files changed

+42
-2
lines changed

5 files changed

+42
-2
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,10 @@ Improvements to Clang's diagnostics
334334
- Clang now emits ``unused argument`` warning when the -fmodule-output flag is used
335335
with an input that is not of type c++-module.
336336

337+
- Clang emits a ``-Wunused-but-set-variable`` warning on C++ variables whose declaration
338+
(with initializer) entirely consist the condition expression of a if/while/for construct
339+
but are not actually used in the body of the if/while/for construct. Fixes #GH41447
340+
337341
Improvements to Clang's time-trace
338342
----------------------------------
339343

clang/include/clang/AST/Decl.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,9 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
11001100

11011101
LLVM_PREFERRED_TYPE(bool)
11021102
unsigned EscapingByref : 1;
1103+
1104+
LLVM_PREFERRED_TYPE(bool)
1105+
unsigned IsCXXCondDecl : 1;
11031106
};
11041107

11051108
union {
@@ -1589,6 +1592,15 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
15891592
NonParmVarDeclBits.EscapingByref = true;
15901593
}
15911594

1595+
bool isCXXCondDecl() const {
1596+
return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.IsCXXCondDecl;
1597+
}
1598+
1599+
void setCXXCondDecl() {
1600+
assert(!isa<ParmVarDecl>(this));
1601+
NonParmVarDeclBits.IsCXXCondDecl = true;
1602+
}
1603+
15921604
/// Determines if this variable's alignment is dependent.
15931605
bool hasDependentAlignment() const;
15941606

clang/lib/Sema/SemaDecl.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2188,8 +2188,21 @@ void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD,
21882188

21892189
assert(iter->getSecond() >= 0 &&
21902190
"Found a negative number of references to a VarDecl");
2191-
if (iter->getSecond() != 0)
2192-
return;
2191+
if (int RefCnt = iter->getSecond(); RefCnt > 0) {
2192+
// Assume the given VarDecl is "used" if its ref count stored in
2193+
// `RefMinusAssignments` is positive, with one exception.
2194+
//
2195+
// For a C++ variable whose decl (with initializer) entirely consist the
2196+
// condition expression of a if/while/for construct,
2197+
// Clang creates a DeclRefExpr for the condition expression rather than a
2198+
// BinaryOperator of AssignmentOp. Thus, the C++ variable's ref
2199+
// count stored in `RefMinusAssignment` equals 1 when the variable is never
2200+
// used in the body of the if/while/for construct.
2201+
bool UnusedCXXCondDecl = VD->isCXXCondDecl() && (RefCnt == 1);
2202+
if (!UnusedCXXCondDecl)
2203+
return;
2204+
}
2205+
21932206
unsigned DiagID = isa<ParmVarDecl>(VD) ? diag::warn_unused_but_set_parameter
21942207
: diag::warn_unused_but_set_variable;
21952208
DiagReceiver(VD->getLocation(), PDiag(DiagID) << VD);

clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18564,6 +18564,9 @@ DeclResult Sema::ActOnCXXConditionDeclaration(Scope *S, Declarator &D) {
1856418564
return true;
1856518565
}
1856618566

18567+
if (auto *VD = dyn_cast<VarDecl>(Dcl))
18568+
VD->setCXXCondDecl();
18569+
1856718570
return Dcl;
1856818571
}
1856918572

clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,11 @@ template <typename T> void f5() {
6969
SWarnUnused swu;
7070
++swu;
7171
}
72+
73+
void f6() {
74+
if (int x = 123) {} // expected-warning{{variable 'x' set but not used}}
75+
76+
while (int x = 123) {} // expected-warning{{variable 'x' set but not used}}
77+
78+
for (; int x = 123;) {} // expected-warning{{variable 'x' set but not used}}
79+
}

0 commit comments

Comments
 (0)