[clang] fd11cf4 - [clang] Extend -Wunused-but-set-variable to static globals (#178342)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 23 21:33:38 PDT 2026
Author: John Paul Jepko
Date: 2026-03-24T12:33:33+08:00
New Revision: fd11cf430e5a9fd11f93bdcc929a1ce8dfa60f37
URL: https://github.com/llvm/llvm-project/commit/fd11cf430e5a9fd11f93bdcc929a1ce8dfa60f37
DIFF: https://github.com/llvm/llvm-project/commit/fd11cf430e5a9fd11f93bdcc929a1ce8dfa60f37.diff
LOG: [clang] Extend -Wunused-but-set-variable to static globals (#178342)
This PR extends the capability of -Wunused-but-set-variable to track and
diagnose static global variables that are assigned values within a
function but whose values are never used. This change complements
-Wunused-variable, which detects static globals that are neither set nor
used.
I created this change with the help of claude for some initial guidance.
Fixes #148361
Added:
clang/test/Sema/Inputs/warn-unused-but-set-static-global-header.h
clang/test/Sema/warn-unused-but-set-static-global.c
clang/test/Sema/warn-unused-but-set-static-global.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Decl.h
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/C/C2y/n3622.c
clang/test/SemaCXX/warn-variable-not-needed.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6e6c580fcb0ff..6a2632543d337 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -230,6 +230,9 @@ Attribute Changes in Clang
Improvements to Clang's diagnostics
-----------------------------------
+- ``-Wunused-but-set-variable`` now diagnoses file-scope variables with
+ internal linkage (``static`` storage class) that are assigned but never used. (#GH148361)
+
- Added ``-Wlifetime-safety`` to enable lifetime safety analysis,
a CFG-based intra-procedural analysis that detects use-after-free and related
temporal safety bugs. See the
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index c3cd74a5b34db..076d9ba935583 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -1212,6 +1212,21 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
&& !isFileVarDecl();
}
+ /// Returns true if this is a file-scope variable with internal linkage.
+ bool isInternalLinkageFileVar() const {
+ // Calling isExternallyVisible() can trigger linkage computation/caching,
+ // which may produce stale results when a decl's DeclContext changes after
+ // creation (e.g., OpenMP declare mapper variables), so here we determine
+ // it syntactically instead.
+ if (!isFileVarDecl())
+ return false;
+ // Linkage is determined by enclosing class/namespace for static data
+ // members.
+ if (getStorageClass() == SC_Static && !isStaticDataMember())
+ return true;
+ return isInAnonymousNamespace();
+ }
+
/// Returns true if a variable has extern or __private_extern__
/// storage.
bool hasExternalStorage() const {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 832e46286194a..a214a7aa9147b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -973,6 +973,10 @@ class Sema final : public SemaBase {
/// unit because its type has no linkage and it's not extern "C".
bool isExternalWithNoLinkageType(const ValueDecl *VD) const;
+ /// Determines whether the given source location is in the main file
+ /// and we're in a context where we should warn about unused entities.
+ bool isMainFileLoc(SourceLocation Loc) const;
+
/// Obtain a sorted list of functions that are undefined but ODR-used.
void getUndefinedButUsed(
SmallVectorImpl<std::pair<NamedDecl *, SourceLocation>> &Undefined);
@@ -7009,6 +7013,9 @@ class Sema final : public SemaBase {
/// Increment when we find a reference; decrement when we find an ignored
/// assignment. Ultimately the value is 0 if every reference is an ignored
/// assignment.
+ ///
+ /// Uses canonical VarDecl as key so in-class decls and out-of-class defs of
+ /// static data members get tracked as a single entry.
llvm::DenseMap<const VarDecl *, int> RefsMinusAssignments;
/// Used to control the generation of ExprWithCleanups.
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 826277c445a1d..620b1352841d1 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -956,6 +956,12 @@ bool Sema::isExternalWithNoLinkageType(const ValueDecl *VD) const {
!isFunctionOrVarDeclExternC(VD);
}
+bool Sema::isMainFileLoc(SourceLocation Loc) const {
+ if (TUKind != TU_Complete || getLangOpts().IsHeaderFile)
+ return false;
+ return SourceMgr.isInMainFile(Loc);
+}
+
/// Obtains a sorted list of functions and variables that are undefined but
/// ODR-used.
void Sema::getUndefinedButUsed(
@@ -1604,6 +1610,40 @@ void Sema::ActOnEndOfTranslationUnit() {
emitAndClearUnusedLocalTypedefWarnings();
}
+ if (!Diags.isIgnored(diag::warn_unused_but_set_variable, SourceLocation())) {
+ // Diagnose unused-but-set static globals in a deterministic order.
+ // Not tracking shadowing info for static globals; there's nothing to
+ // shadow.
+ struct LocAndDiag {
+ SourceLocation Loc;
+ PartialDiagnostic PD;
+ };
+ SmallVector<LocAndDiag, 16> DeclDiags;
+ auto addDiag = [&DeclDiags](SourceLocation Loc, PartialDiagnostic PD) {
+ DeclDiags.push_back(LocAndDiag{Loc, std::move(PD)});
+ };
+
+ // For -Wunused-but-set-variable we only care about variables that were
+ // referenced by the TU end.
+ for (const auto &Ref : RefsMinusAssignments) {
+ const VarDecl *VD = Ref.first;
+ // Only diagnose internal linkage file vars defined in the main file to
+ // match -Wunused-variable behavior and avoid false positives from
+ // headers.
+ if (VD->isInternalLinkageFileVar() && isMainFileLoc(VD->getLocation()))
+ DiagnoseUnusedButSetDecl(VD, addDiag);
+ }
+
+ llvm::sort(DeclDiags,
+ [](const LocAndDiag &LHS, const LocAndDiag &RHS) -> bool {
+ // Sorting purely for determinism; matches behavior in
+ // Sema::ActOnPopScope.
+ return LHS.Loc < RHS.Loc;
+ });
+ for (const LocAndDiag &D : DeclDiags)
+ Diag(D.Loc, D.PD);
+ }
+
if (!Diags.isIgnored(diag::warn_unused_private_field, SourceLocation())) {
// FIXME: Load additional unused private field candidates from the external
// source.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 52946e0ca3cb1..c862c090c50a8 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1912,14 +1912,6 @@ bool Sema::mightHaveNonExternalLinkage(const DeclaratorDecl *D) {
return !D->isExternallyVisible();
}
-// FIXME: This needs to be refactored; some other isInMainFile users want
-// these semantics.
-static bool isMainFileLoc(const Sema &S, SourceLocation Loc) {
- if (S.TUKind != TU_Complete || S.getLangOpts().IsHeaderFile)
- return false;
- return S.SourceMgr.isInMainFile(Loc);
-}
-
bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
assert(D);
@@ -1946,7 +1938,7 @@ bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
return false;
} else {
// 'static inline' functions are defined in headers; don't warn.
- if (FD->isInlined() && !isMainFileLoc(*this, FD->getLocation()))
+ if (FD->isInlined() && !isMainFileLoc(FD->getLocation()))
return false;
}
@@ -1957,7 +1949,7 @@ bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
// Constants and utility variables are defined in headers with internal
// linkage; don't warn. (Unlike functions, there isn't a convenient marker
// like "inline".)
- if (!isMainFileLoc(*this, VD->getLocation()))
+ if (!isMainFileLoc(VD->getLocation()))
return false;
if (Context.DeclMustBeEmitted(VD))
@@ -1971,7 +1963,7 @@ bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
VD->getMemberSpecializationInfo() && !VD->isOutOfLine())
return false;
- if (VD->isInline() && !isMainFileLoc(*this, VD->getLocation()))
+ if (VD->isInline() && !isMainFileLoc(VD->getLocation()))
return false;
} else {
return false;
@@ -2215,6 +2207,11 @@ void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD,
return;
}
+ // Don't warn on volatile file-scope variables. They are visible beyond their
+ // declaring function and writes to them could be observable side effects.
+ if (VD->getType().isVolatileQualified() && VD->isFileVarDecl())
+ return;
+
// Don't warn about __block Objective-C pointer variables, as they might
// be assigned in the block but not used elsewhere for the purpose of lifetime
// extension.
@@ -2227,7 +2224,7 @@ void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD,
if (VD->hasAttr<ObjCPreciseLifetimeAttr>() && Ty->isObjCObjectPointerType())
return;
- auto iter = RefsMinusAssignments.find(VD);
+ auto iter = RefsMinusAssignments.find(VD->getCanonicalDecl());
if (iter == RefsMinusAssignments.end())
return;
@@ -2305,9 +2302,11 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
DiagnoseUnusedDecl(D, addDiag);
if (const auto *RD = dyn_cast<RecordDecl>(D))
DiagnoseUnusedNestedTypedefs(RD, addDiag);
- if (VarDecl *VD = dyn_cast<VarDecl>(D)) {
+ // Wait until end of TU to diagnose internal linkage file vars.
+ if (auto *VD = dyn_cast<VarDecl>(D);
+ VD && !VD->isInternalLinkageFileVar()) {
DiagnoseUnusedButSetDecl(VD, addDiag);
- RefsMinusAssignments.erase(VD);
+ RefsMinusAssignments.erase(VD->getCanonicalDecl());
}
}
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2b0a786302aab..fa1cd1ebb52de 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -20477,8 +20477,15 @@ static void DoMarkVarDeclReferenced(
bool UsableInConstantExpr =
Var->mightBeUsableInConstantExpressions(SemaRef.Context);
- if (Var->isLocalVarDeclOrParm() && !Var->hasExternalStorage()) {
- RefsMinusAssignments.insert({Var, 0}).first->getSecond()++;
+ // Only track variables with internal linkage or local scope.
+ // Use canonical decl so in-class declarations and out-of-class definitions
+ // of static data members in anonymous namespaces are tracked as a single
+ // entry.
+ const VarDecl *CanonVar = Var->getCanonicalDecl();
+ if ((CanonVar->isLocalVarDeclOrParm() ||
+ CanonVar->isInternalLinkageFileVar()) &&
+ !CanonVar->hasExternalStorage()) {
+ RefsMinusAssignments.insert({CanonVar, 0}).first->getSecond()++;
}
// C++20 [expr.const]p12:
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 5553f25546c38..08bc6aa483844 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7504,7 +7504,7 @@ static void MaybeDecrementCount(
if ((IsCompoundAssign || isIncrementDecrementUnaryOp) &&
VD->getType().isVolatileQualified())
return;
- auto iter = RefsMinusAssignments.find(VD);
+ auto iter = RefsMinusAssignments.find(VD->getCanonicalDecl());
if (iter == RefsMinusAssignments.end())
return;
iter->getSecond()--;
diff --git a/clang/test/C/C2y/n3622.c b/clang/test/C/C2y/n3622.c
index 95b92e8f235a8..d90b0c51d3ccf 100644
--- a/clang/test/C/C2y/n3622.c
+++ b/clang/test/C/C2y/n3622.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -verify=good -pedantic -Wall -std=c2y %s
-// RUN: %clang_cc1 -verify=compat,expected -pedantic -Wall -Wpre-c2y-compat -std=c2y %s
-// RUN: %clang_cc1 -verify=ped,expected -pedantic -Wall -std=c23 %s
-// RUN: %clang_cc1 -verify=ped,expected -pedantic -Wall -std=c17 %s
+// RUN: %clang_cc1 -verify=good -pedantic -Wall -Wno-unused-but-set-variable -std=c2y %s
+// RUN: %clang_cc1 -verify=compat,expected -pedantic -Wall -Wno-unused-but-set-variable -Wpre-c2y-compat -std=c2y %s
+// RUN: %clang_cc1 -verify=ped,expected -pedantic -Wall -Wno-unused-but-set-variable -std=c23 %s
+// RUN: %clang_cc1 -verify=ped,expected -pedantic -Wall -Wno-unused-but-set-variable -std=c17 %s
// good-no-diagnostics
/* WG14 N3622: Clang 22
diff --git a/clang/test/Sema/Inputs/warn-unused-but-set-static-global-header.h b/clang/test/Sema/Inputs/warn-unused-but-set-static-global-header.h
new file mode 100644
index 0000000000000..40637d644f717
--- /dev/null
+++ b/clang/test/Sema/Inputs/warn-unused-but-set-static-global-header.h
@@ -0,0 +1,3 @@
+// Header file for testing that header-defined static globals don't warn.
+static int header_set_unused = 0;
+static void header_init() { header_set_unused = 1; }
diff --git a/clang/test/Sema/warn-unused-but-set-static-global.c b/clang/test/Sema/warn-unused-but-set-static-global.c
new file mode 100644
index 0000000000000..7c4d20a42b2d2
--- /dev/null
+++ b/clang/test/Sema/warn-unused-but-set-static-global.c
@@ -0,0 +1,132 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-but-set-variable -I %S/Inputs -verify %s
+
+// Test that header-defined static globals don't warn.
+#include "warn-unused-but-set-static-global-header.h"
+
+#define NULL (void*)0
+
+void *set(int size);
+void func_call(void *);
+
+static int set_unused; // expected-warning {{variable 'set_unused' set but not used}}
+static int set_and_used;
+static int only_used;
+static int addr_taken;
+extern int external_var; // No warning (external linkage).
+extern int global_var; // No warning (not static).
+
+void f1() {
+ set_unused = 1;
+ set_and_used = 2;
+
+ int x = set_and_used;
+ (void)x;
+
+ int y = only_used;
+ (void)y;
+
+ int *p = &addr_taken;
+ (void)p;
+
+ external_var = 3;
+ global_var = 4;
+}
+
+// Test across multiple functions.
+static int set_used1;
+static int set_used2;
+
+static int set1; // expected-warning {{variable 'set1' set but not used}}
+static int set2; // expected-warning {{variable 'set2' set but not used}}
+
+void f2() {
+ set1 = 1;
+ set_used1 = 1;
+
+ int x = set_used2;
+ (void)x;
+}
+
+void f3() {
+ set2 = 2;
+ set_used2 = 2;
+
+ int x = set_used1;
+ (void)x;
+}
+
+static volatile int vol_set;
+void f4() {
+ vol_set = 1;
+}
+
+// Read and use
+static int compound; // expected-warning{{variable 'compound' set but not used}}
+static volatile int vol_compound;
+static int unary; // expected-warning{{variable 'unary' set but not used}}
+static volatile int vol_unary;
+void f5() {
+ compound += 1;
+ vol_compound += 1;
+ unary++;
+ vol_unary++;
+}
+
+struct S {
+ int i;
+};
+static struct S s_set; // expected-warning{{variable 's_set' set but not used}}
+static struct S s_used;
+void f6() {
+ struct S t;
+ s_set = t;
+ t = s_used;
+}
+
+// Multiple assignments
+static int multi; // expected-warning{{variable 'multi' set but not used}}
+void f7() {
+ multi = 1;
+ multi = 2;
+ multi = 3;
+}
+
+// Unused pointers
+static int *unused_ptr; // expected-warning{{variable 'unused_ptr' set but not used}}
+static char *str_ptr; // expected-warning{{variable 'str_ptr' set but not used}}
+void f8() {
+ unused_ptr = set(5);
+ str_ptr = "hello";
+}
+
+// Used pointers
+void a(void *);
+static int *used_ptr;
+static int *param_ptr;
+static int *null_check_ptr;
+void f9() {
+ used_ptr = set(5);
+ *used_ptr = 5;
+
+ param_ptr = set(5);
+ func_call(param_ptr);
+
+ null_check_ptr = set(5);
+ if (null_check_ptr == NULL) {}
+}
+
+// Function pointers (unused)
+static void (*unused_func_ptr)(); // expected-warning {{variable 'unused_func_ptr' set but not used}}
+void SetUnusedCallback(void (*f)()) {
+ unused_func_ptr = f;
+}
+
+// Function pointers (used)
+static void (*used_func_ptr)();
+void SetUsedCallback(void (*f)()) {
+ used_func_ptr = f;
+}
+void CallUsedCallback() {
+ if (used_func_ptr)
+ used_func_ptr();
+}
diff --git a/clang/test/Sema/warn-unused-but-set-static-global.cpp b/clang/test/Sema/warn-unused-but-set-static-global.cpp
new file mode 100644
index 0000000000000..1ddd7eb72ce29
--- /dev/null
+++ b/clang/test/Sema/warn-unused-but-set-static-global.cpp
@@ -0,0 +1,165 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-but-set-variable -verify -std=c++17 %s
+
+static thread_local int tl_set_unused; // expected-warning {{variable 'tl_set_unused' set but not used}}
+static thread_local int tl_set_and_used;
+thread_local int tl_no_static_set_unused;
+
+// Warning should respect attributes.
+[[maybe_unused]] static int with_maybe_unused;
+__attribute__((unused)) static int with_unused_attr;
+
+void f0() {
+ tl_set_unused = 1;
+ tl_set_and_used = 2;
+ int x = tl_set_and_used;
+ (void)x;
+
+ tl_no_static_set_unused = 3;
+
+ with_maybe_unused = 4;
+ with_unused_attr = 5;
+}
+
+// Named namespace.
+namespace test {
+ static int set_unused; // expected-warning {{variable 'set_unused' set but not used}}
+ static int set_and_used;
+
+ void f1() {
+ set_unused = 1;
+ set_and_used = 2;
+ int x = set_and_used;
+ (void)x;
+ }
+}
+
+// Nested named namespace.
+namespace outer {
+namespace inner {
+static int nested_unused; // expected-warning {{variable 'nested_unused' set but not used}}
+void f2() {
+ nested_unused = 5;
+}
+}
+}
+
+// Anonymous namespace (all vars have internal linkage).
+namespace {
+ int anon_ns_unused; // expected-warning {{variable 'anon_ns_unused' set but not used}}
+ static int anon_ns_static_unused; // expected-warning {{variable 'anon_ns_static_unused' set but not used}}
+
+ void f3() {
+ anon_ns_unused = 1;
+ anon_ns_static_unused = 2;
+ }
+}
+
+// Function pointers at file scope.
+static void (*unused_func_ptr)(); // expected-warning {{variable 'unused_func_ptr' set but not used}}
+void SetUnusedCallback(void (*f)()) {
+ unused_func_ptr = f;
+}
+
+static void (*used_func_ptr)();
+void SetUsedCallback(void (*f)()) {
+ used_func_ptr = f;
+}
+void CallUsedCallback() {
+ if (used_func_ptr)
+ used_func_ptr();
+}
+
+// Static data members (external linkage, should not warn).
+class MyClass {
+public:
+ static int unused_static_member;
+ static int used_static_member;
+};
+
+int MyClass::unused_static_member = 0;
+int MyClass::used_static_member = 0;
+
+void f4() {
+ MyClass::unused_static_member = 10;
+
+ MyClass::used_static_member = 20;
+ int x = MyClass::used_static_member;
+ (void)x;
+}
+
+// Static data members in a named namespace (external linkage, should not warn).
+namespace named {
+ struct NamedClass {
+ static int w;
+ };
+ int NamedClass::w = 0;
+}
+
+void f5() {
+ named::NamedClass::w = 4;
+}
+
+// Static data members in anonymous namespace (internal linkage, should warn).
+namespace {
+ class AnonClass {
+ public:
+ static int unused_member; // expected-warning {{variable 'unused_member' set but not used}}
+ static int used_member;
+ };
+
+ int AnonClass::unused_member = 0;
+ int AnonClass::used_member = 0;
+}
+
+void f6() {
+ AnonClass::unused_member = 3;
+ AnonClass::used_member = 4;
+ int y = AnonClass::used_member;
+ (void)y;
+}
+
+// Static data members in nested anonymous namespace (internal linkage, should warn).
+namespace outer2 {
+ namespace {
+ struct NestedAnonClass {
+ static int v; // expected-warning {{variable 'v' set but not used}}
+ };
+ int NestedAnonClass::v = 0;
+ }
+}
+
+void f7() {
+ outer2::NestedAnonClass::v = 5;
+}
+
+// Static data members set inside methods, read outside.
+namespace {
+ struct SetInMethod {
+ static int x;
+ static int y; // expected-warning {{variable 'y' set but not used}}
+ void setX() { x = 1; }
+ void setY() { y = 1; }
+ };
+ int SetInMethod::x;
+ int SetInMethod::y;
+}
+
+void f8() {
+ SetInMethod s;
+ s.setX();
+ s.setY();
+ int v = SetInMethod::x;
+ (void)v; // only x is read
+}
+
+// External linkage static data members set inside methods.
+struct ExtSetInMethod {
+ static int x;
+ void set() { x = 1; }
+};
+int ExtSetInMethod::x;
+
+void f9() {
+ ExtSetInMethod e;
+ e.set();
+}
diff --git a/clang/test/SemaCXX/warn-variable-not-needed.cpp b/clang/test/SemaCXX/warn-variable-not-needed.cpp
index 103be189068f8..272c8998d15c0 100644
--- a/clang/test/SemaCXX/warn-variable-not-needed.cpp
+++ b/clang/test/SemaCXX/warn-variable-not-needed.cpp
@@ -18,7 +18,7 @@ namespace test2 {
};
namespace {
struct foo : bah {
- static char bar;
+ static char bar; // expected-warning {{variable 'bar' set but not used}}
virtual void zed();
};
void foo::zed() {
More information about the cfe-commits
mailing list