llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-analysis
Author: Igor Kudrin (igorkudrin)
<details>
<summary>Changes</summary>
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.
```
---
Full diff: https://github.com/llvm/llvm-project/pull/147898.diff
5 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/UninitializedValues.h (+6-4)
- (modified) clang/lib/Analysis/UninitializedValues.cpp (+5-8)
- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+13-41)
- (added) clang/test/SemaCXX/uninitialized-self-init.cpp (+13)
- (modified) clang/test/SemaCXX/warn-uninitialized-const-reference.cpp (+1-1)
``````````diff
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}}
``````````
</details>
https://github.com/llvm/llvm-project/pull/147898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
