[clang] Combine ConstRefUse with other warnings for uninitialized values (#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.
```

Author: igorkudrin

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.

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.

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -987,11 +987,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
@@ -1533,33 +1532,24 @@ 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;
   }
 
   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) {
@@ -1582,13 +1572,21 @@ 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();
           return a.getUser()->getBeginLoc() < b.getUser()->getBeginLoc();
         });
 
         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;
 
@@ -1604,32 +1602,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:

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@-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}}
+}

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}}