https://github.com/igorkudrin created https://github.com/llvm/llvm-project/pull/147898
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. ``` >From a813a4c665579b3654b287ff64c8180139879a38 Mon Sep 17 00:00:00 2001 From: Igor Kudrin <ikud...@accesssoftek.com> Date: Wed, 9 Jul 2025 21:19:40 -0700 Subject: [PATCH] [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}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits