[clang] [clang] Combine ConstRefUse with other warnings for uninitialized values (PR #147898)
Igor Kudrin via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 10 15:03:26 PDT 2025
https://github.com/igorkudrin updated https://github.com/llvm/llvm-project/pull/147898
>From a813a4c665579b3654b287ff64c8180139879a38 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Wed, 9 Jul 2025 21:19:40 -0700
Subject: [PATCH 1/2] [clang] Combine ConstRefUse with other warnings for
uninitialized values
This helps to avoid duplicating warnings in cases like:
```
> cat test.cpp
void bar(int);
void foo(const int &);
void test(bool a) {
int v = v;
if (a)
bar(v);
else
foo(v);
}
> clang++.exe test.cpp -fsyntax-only -Wuninitialized
test.cpp:4:11: warning: variable 'v' is uninitialized when used within its own initialization [-Wuninitialized]
4 | int v = v;
| ~ ^
test.cpp:4:11: warning: variable 'v' is uninitialized when used within its own initialization [-Wuninitialized]
4 | int v = v;
| ~ ^
2 warnings generated.
```
---
.../Analysis/Analyses/UninitializedValues.h | 10 ++--
clang/lib/Analysis/UninitializedValues.cpp | 13 ++---
clang/lib/Sema/AnalysisBasedWarnings.cpp | 54 +++++--------------
.../test/SemaCXX/uninitialized-self-init.cpp | 13 +++++
.../warn-uninitialized-const-reference.cpp | 2 +-
5 files changed, 38 insertions(+), 54 deletions(-)
create mode 100644 clang/test/SemaCXX/uninitialized-self-init.cpp
diff --git a/clang/include/clang/Analysis/Analyses/UninitializedValues.h b/clang/include/clang/Analysis/Analyses/UninitializedValues.h
index a2b37deddcec2..b151bc3f58321 100644
--- a/clang/include/clang/Analysis/Analyses/UninitializedValues.h
+++ b/clang/include/clang/Analysis/Analyses/UninitializedValues.h
@@ -47,6 +47,9 @@ class UninitUse {
/// Does this use always see an uninitialized value?
bool AlwaysUninit;
+ /// Is this use a const reference to this variable?
+ bool ConstRefUse = false;
+
/// This use is always uninitialized if it occurs after any of these branches
/// is taken.
SmallVector<Branch, 2> UninitBranches;
@@ -61,10 +64,13 @@ class UninitUse {
void setUninitAfterCall() { UninitAfterCall = true; }
void setUninitAfterDecl() { UninitAfterDecl = true; }
+ void setConstRefUse() { ConstRefUse = true; }
/// Get the expression containing the uninitialized use.
const Expr *getUser() const { return User; }
+ bool isConstRefUse() const { return ConstRefUse; }
+
/// The kind of uninitialized use.
enum Kind {
/// The use might be uninitialized.
@@ -110,10 +116,6 @@ class UninitVariablesHandler {
virtual void handleUseOfUninitVariable(const VarDecl *vd,
const UninitUse &use) {}
- /// Called when the uninitialized variable is used as const refernce argument.
- virtual void handleConstRefUseOfUninitVariable(const VarDecl *vd,
- const UninitUse &use) {}
-
/// Called when the uninitialized variable analysis detects the
/// idiom 'int x = x'. All other uses of 'x' within the initializer
/// are handled by handleUseOfUninitVariable.
diff --git a/clang/lib/Analysis/UninitializedValues.cpp b/clang/lib/Analysis/UninitializedValues.cpp
index b2a68b6c39a7e..045ee137e6b08 100644
--- a/clang/lib/Analysis/UninitializedValues.cpp
+++ b/clang/lib/Analysis/UninitializedValues.cpp
@@ -675,8 +675,11 @@ void TransferFunctions::reportUse(const Expr *ex, const VarDecl *vd) {
void TransferFunctions::reportConstRefUse(const Expr *ex, const VarDecl *vd) {
Value v = vals[vd];
- if (isAlwaysUninit(v))
- handler.handleConstRefUseOfUninitVariable(vd, getUninitUse(ex, vd, v));
+ if (isAlwaysUninit(v)) {
+ auto use = getUninitUse(ex, vd, v);
+ use.setConstRefUse();
+ handler.handleUseOfUninitVariable(vd, use);
+ }
}
void TransferFunctions::VisitObjCForCollectionStmt(ObjCForCollectionStmt *FS) {
@@ -891,12 +894,6 @@ struct PruneBlocksHandler : public UninitVariablesHandler {
hadAnyUse = true;
}
- void handleConstRefUseOfUninitVariable(const VarDecl *vd,
- const UninitUse &use) override {
- hadUse[currentBlock] = true;
- hadAnyUse = true;
- }
-
/// Called when the uninitialized variable analysis detects the
/// idiom 'int x = x'. All other uses of 'x' within the initializer
/// are handled by handleUseOfUninitVariable.
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 7420ba2d461c6..03aefba472bb5 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -985,11 +985,10 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
}
/// Diagnose uninitialized const reference usages.
-static bool DiagnoseUninitializedConstRefUse(Sema &S, const VarDecl *VD,
+static void DiagnoseUninitializedConstRefUse(Sema &S, const VarDecl *VD,
const UninitUse &Use) {
S.Diag(Use.getUser()->getBeginLoc(), diag::warn_uninit_const_reference)
<< VD->getDeclName() << Use.getUser()->getSourceRange();
- return true;
}
/// DiagnoseUninitializedUse -- Helper function for diagnosing uses of an
@@ -1531,14 +1530,13 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
// order of diagnostics when calling flushDiagnostics().
typedef llvm::MapVector<const VarDecl *, MappedType> UsesMap;
UsesMap uses;
- UsesMap constRefUses;
public:
UninitValsDiagReporter(Sema &S) : S(S) {}
~UninitValsDiagReporter() override { flushDiagnostics(); }
- MappedType &getUses(UsesMap &um, const VarDecl *vd) {
- MappedType &V = um[vd];
+ MappedType &getUses(const VarDecl *vd) {
+ MappedType &V = uses[vd];
if (!V.getPointer())
V.setPointer(new UsesVec());
return V;
@@ -1546,18 +1544,10 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
void handleUseOfUninitVariable(const VarDecl *vd,
const UninitUse &use) override {
- getUses(uses, vd).getPointer()->push_back(use);
- }
-
- void handleConstRefUseOfUninitVariable(const VarDecl *vd,
- const UninitUse &use) override {
- getUses(constRefUses, vd).getPointer()->push_back(use);
+ getUses(vd).getPointer()->push_back(use);
}
- void handleSelfInit(const VarDecl *vd) override {
- getUses(uses, vd).setInt(true);
- getUses(constRefUses, vd).setInt(true);
- }
+ void handleSelfInit(const VarDecl *vd) override { getUses(vd).setInt(true); }
void flushDiagnostics() {
for (const auto &P : uses) {
@@ -1580,6 +1570,9 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
// guaranteed to produce them in line/column order, this will provide
// a stable ordering.
llvm::sort(*vec, [](const UninitUse &a, const UninitUse &b) {
+ // Move ConstRef uses to the back.
+ if (a.isConstRefUse() != b.isConstRefUse())
+ return b.isConstRefUse();
// Prefer a more confident report over a less confident one.
if (a.getKind() != b.getKind())
return a.getKind() > b.getKind();
@@ -1587,6 +1580,11 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
});
for (const auto &U : *vec) {
+ if (U.isConstRefUse()) {
+ DiagnoseUninitializedConstRefUse(S, vd, U);
+ break;
+ }
+
// If we have self-init, downgrade all uses to 'may be uninitialized'.
UninitUse Use = hasSelfInit ? UninitUse(U.getUser(), false) : U;
@@ -1602,32 +1600,6 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
}
uses.clear();
-
- // Flush all const reference uses diags.
- for (const auto &P : constRefUses) {
- const VarDecl *vd = P.first;
- const MappedType &V = P.second;
-
- UsesVec *vec = V.getPointer();
- bool hasSelfInit = V.getInt();
-
- if (!vec->empty() && hasSelfInit && hasAlwaysUninitializedUse(vec))
- DiagnoseUninitializedUse(S, vd,
- UninitUse(vd->getInit()->IgnoreParenCasts(),
- /* isAlwaysUninit */ true),
- /* alwaysReportSelfInit */ true);
- else {
- for (const auto &U : *vec) {
- if (DiagnoseUninitializedConstRefUse(S, vd, U))
- break;
- }
- }
-
- // Release the uses vector.
- delete vec;
- }
-
- constRefUses.clear();
}
private:
diff --git a/clang/test/SemaCXX/uninitialized-self-init.cpp b/clang/test/SemaCXX/uninitialized-self-init.cpp
new file mode 100644
index 0000000000000..0131e0e308051
--- /dev/null
+++ b/clang/test/SemaCXX/uninitialized-self-init.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
+
+void bar(int);
+void foo(const int &);
+
+// Test that the warning about self initialization is generated only once.
+void test(bool a) {
+ int v = v; // expected-warning {{variable 'v' is uninitialized when used within its own initialization}}
+ if (a)
+ bar(v);
+ else
+ foo(v);
+}
diff --git a/clang/test/SemaCXX/warn-uninitialized-const-reference.cpp b/clang/test/SemaCXX/warn-uninitialized-const-reference.cpp
index d24b561441d8f..7204d6525cef9 100644
--- a/clang/test/SemaCXX/warn-uninitialized-const-reference.cpp
+++ b/clang/test/SemaCXX/warn-uninitialized-const-reference.cpp
@@ -27,7 +27,7 @@ int const_use(const int i);
void f(int a) {
int i;
const_ref_use(i); // expected-warning {{variable 'i' is uninitialized when passed as a const reference argument here}}
- int j = j + const_ref_use(j); // expected-warning {{variable 'j' is uninitialized when used within its own initialization}} expected-warning {{variable 'j' is uninitialized when passed as a const reference argument here}}
+ int j = j + const_ref_use(j); // expected-warning {{variable 'j' is uninitialized when used within its own initialization}}
A a1 = const_ref_use_A(a1); // expected-warning {{variable 'a1' is uninitialized when passed as a const reference argument here}}
int k = const_use(k); // expected-warning {{variable 'k' is uninitialized when used within its own initialization}}
A a2 = const_use_A(a2); // expected-warning {{variable 'a2' is uninitialized when used within its own initialization}}
>From 02f2f38a6fe6007facaea8c6a9cddec49a68dead Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Thu, 10 Jul 2025 15:02:50 -0700
Subject: [PATCH 2/2] fixup! Add a test for diagnostic priorities
---
.../SemaCXX/uninitialized-multiple-uses.cpp | 25 +++++++++++++++++++
.../test/SemaCXX/uninitialized-self-init.cpp | 13 ----------
2 files changed, 25 insertions(+), 13 deletions(-)
create mode 100644 clang/test/SemaCXX/uninitialized-multiple-uses.cpp
delete mode 100644 clang/test/SemaCXX/uninitialized-self-init.cpp
diff --git a/clang/test/SemaCXX/uninitialized-multiple-uses.cpp b/clang/test/SemaCXX/uninitialized-multiple-uses.cpp
new file mode 100644
index 0000000000000..a6a4ad39d0be0
--- /dev/null
+++ b/clang/test/SemaCXX/uninitialized-multiple-uses.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
+
+void use_val(int);
+void use_const_ref(const int &);
+
+// Test that the warning about self initialization is generated only once.
+void test_self_init_1warning(bool a) {
+ int v = v; // expected-warning {{variable 'v' is uninitialized when used within its own initialization}}
+ if (a)
+ use_val(v);
+ else
+ use_const_ref(v);
+}
+
+// Test that the diagnostic for using an uninitialized variable directly has a
+// higher priority than using the same variable via a const reference.
+void test_prioritize_use_over_const_ref(bool a) {
+ int v; // expected-note {{initialize the variable 'v' to silence this warning}}
+ if (a) // expected-warning {{variable 'v' is used uninitialized whenever 'if' condition is false}}
+ // expected-note at -1 {{remove the 'if' if its condition is always true}}
+ v = 2;
+ else
+ use_const_ref(v);
+ use_val(v); // expected-note {{uninitialized use occurs here}}
+}
diff --git a/clang/test/SemaCXX/uninitialized-self-init.cpp b/clang/test/SemaCXX/uninitialized-self-init.cpp
deleted file mode 100644
index 0131e0e308051..0000000000000
--- a/clang/test/SemaCXX/uninitialized-self-init.cpp
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
-
-void bar(int);
-void foo(const int &);
-
-// Test that the warning about self initialization is generated only once.
-void test(bool a) {
- int v = v; // expected-warning {{variable 'v' is uninitialized when used within its own initialization}}
- if (a)
- bar(v);
- else
- foo(v);
-}
More information about the cfe-commits
mailing list