[Clang] Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

Summary:
Add a new warning -Wuninitialized-const-reference as a subgroup of -Wuninitialized to address a bug filed here: https://bugs.llvm.org/show_bug.cgi?id=45624

This warning is controlled by -Wuninitialized and can be disabled by -Wno-uninitialized-const-reference.
The warning is diagnosed when passing uninitialized variables as const reference parameters to a function.

Differential Revision: https://reviews.llvm.org/D79895

clang/include/clang/Analysis/Analyses/UninitializedValues.h

@@ -110,6 +110,10 @@ public:
   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/include/clang/Basic/DiagnosticGroups.td

@@ -624,8 +624,10 @@ def Unicode  : DiagGroup<"unicode">;
 def UninitializedMaybe : DiagGroup<"conditional-uninitialized">;
 def UninitializedSometimes : DiagGroup<"sometimes-uninitialized">;
 def UninitializedStaticSelfInit : DiagGroup<"static-self-init">;
+def UninitializedConstReference : DiagGroup<"uninitialized-const-reference">;
 def Uninitialized  : DiagGroup<"uninitialized", [UninitializedSometimes,
-                                                 UninitializedStaticSelfInit]>;
+                                                 UninitializedStaticSelfInit,
+                                                 UninitializedConstReference]>;
 def IgnoredPragmaIntrinsic : DiagGroup<"ignored-pragma-intrinsic">;
 // #pragma optimize is often used to avoid to work around MSVC codegen bugs or
 // to disable inlining. It's not completely clear what alternative to suggest

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2111,6 +2111,10 @@ def err_list_init_in_parens : Error<
   "cannot initialize %select{non-class|reference}0 type %1 with a "
   "parenthesized initializer list">;
 
+def warn_uninit_const_reference : Warning<
+  "variable %0 is uninitialized when passed as a const reference argument "
+  "here">, InGroup<UninitializedConstReference>, DefaultIgnore;
+
 def warn_unsequenced_mod_mod : Warning<
   "multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
 def warn_unsequenced_mod_use : Warning<

clang/lib/Analysis/UninitializedValues.cpp

@@ -268,6 +268,7 @@ public:
     Init,
     Use,
     SelfInit,
+    ConstRefUse,
     Ignore
   };
 
@@ -413,14 +414,16 @@ void ClassifyRefs::VisitCallExpr(CallExpr *CE) {
     return;
   }
 
-  // If a value is passed by const pointer or by const reference to a function,
+  // If a value is passed by const pointer to a function,
   // we should not assume that it is initialized by the call, and we
   // conservatively do not assume that it is used.
+  // If a value is passed by const reference to a function,
+  // it should already be initialized.
   for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
        I != E; ++I) {
     if ((*I)->isGLValue()) {
       if ((*I)->getType().isConstQualified())
-        classify((*I), Ignore);
+        classify((*I), ConstRefUse);
     } else if (isPointerToConst((*I)->getType())) {
       const Expr *Ex = stripCasts(DC->getParentASTContext(), *I);
       const auto *UO = dyn_cast<UnaryOperator>(Ex);
@@ -469,6 +472,7 @@ public:
         handler(handler) {}
 
   void reportUse(const Expr *ex, const VarDecl *vd);
+  void reportConstRefUse(const Expr *ex, const VarDecl *vd);
 
   void VisitBinaryOperator(BinaryOperator *bo);
   void VisitBlockExpr(BlockExpr *be);
@@ -667,6 +671,12 @@ void TransferFunctions::reportUse(const Expr *ex, const VarDecl *vd) {
     handler.handleUseOfUninitVariable(vd, getUninitUse(ex, vd, v));
 }
 
+void TransferFunctions::reportConstRefUse(const Expr *ex, const VarDecl *vd) {
+  Value v = vals[vd];
+  if (isUninitialized(v))
+    handler.handleConstRefUseOfUninitVariable(vd, getUninitUse(ex, vd, v));
+}
+
 void TransferFunctions::VisitObjCForCollectionStmt(ObjCForCollectionStmt *FS) {
   // This represents an initialization of the 'element' value.
   if (const auto *DS = dyn_cast<DeclStmt>(FS->getElement())) {
@@ -734,7 +744,10 @@ void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *dr) {
     vals[cast<VarDecl>(dr->getDecl())] = Initialized;
     break;
   case ClassifyRefs::SelfInit:
-      handler.handleSelfInit(cast<VarDecl>(dr->getDecl()));
+    handler.handleSelfInit(cast<VarDecl>(dr->getDecl()));
+    break;
+  case ClassifyRefs::ConstRefUse:
+    reportConstRefUse(dr, cast<VarDecl>(dr->getDecl()));
     break;
   }
 }
@@ -867,6 +880,12 @@ 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

@@ -974,6 +974,14 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
         << Use.getUser()->getSourceRange();
 }
 
+/// Diagnose uninitialized const reference usages.
+static bool 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
 /// uninitialized variable. This manages the different forms of diagnostic
 /// emitted for particular types of uses. Returns true if the use was diagnosed
@@ -1506,13 +1514,14 @@ 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(const VarDecl *vd) {
-    MappedType &V = uses[vd];
+  MappedType &getUses(UsesMap &um, const VarDecl *vd) {
+    MappedType &V = um[vd];
     if (!V.getPointer())
       V.setPointer(new UsesVec());
     return V;
@@ -1520,11 +1529,17 @@ public:
 
   void handleUseOfUninitVariable(const VarDecl *vd,
                                  const UninitUse &use) override {
-    getUses(vd).getPointer()->push_back(use);
+    getUses(uses, vd).getPointer()->push_back(use);
+  }
+
+  void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+                                         const UninitUse &use) override {
+    getUses(constRefUses, vd).getPointer()->push_back(use);
   }
 
   void handleSelfInit(const VarDecl *vd) override {
-    getUses(vd).setInt(true);
+    getUses(uses, vd).setInt(true);
+    getUses(constRefUses, vd).setInt(true);
   }
 
   void flushDiagnostics() {
@@ -1571,6 +1586,32 @@ public:
     }
 
     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:
@@ -2184,7 +2225,8 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
 
   if (!Diags.isIgnored(diag::warn_uninit_var, D->getBeginLoc()) ||
       !Diags.isIgnored(diag::warn_sometimes_uninit_var, D->getBeginLoc()) ||
-      !Diags.isIgnored(diag::warn_maybe_uninit_var, D->getBeginLoc())) {
+      !Diags.isIgnored(diag::warn_maybe_uninit_var, D->getBeginLoc()) ||
+      !Diags.isIgnored(diag::warn_uninit_const_reference, D->getBeginLoc())) {
     if (CFG *cfg = AC.getCFG()) {
       UninitValsDiagReporter reporter(S);
       UninitVariablesAnalysisStats stats;

clang/test/Misc/warning-wall.c

@@ -55,6 +55,7 @@ CHECK-NEXT:    -Wtrigraphs
 CHECK-NEXT:    -Wuninitialized
 CHECK-NEXT:      -Wsometimes-uninitialized
 CHECK-NEXT:      -Wstatic-self-init
+CHECK-NEXT:      -Wuninitialized-const-reference
 CHECK-NEXT:    -Wunknown-pragmas
 CHECK-NEXT:    -Wunused
 CHECK-NEXT:      -Wunused-argument

clang/test/SemaCXX/uninit-variables.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -fsyntax-only -fcxx-exceptions %s -verify -std=c++1y
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wno-uninitialized-const-reference -fsyntax-only -fcxx-exceptions %s -verify -std=c++1y
 
 // Stub out types for 'typeid' to work.
 namespace std { class type_info {}; }

clang/test/SemaCXX/uninitialized.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value -Wno-unused-lambda-capture -std=c++1z -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value -Wno-unused-lambda-capture -Wno-uninitialized-const-reference -std=c++1z -verify %s
 
 // definitions for std::move
 namespace std {

clang/test/SemaCXX/warn-uninitialized-const-reference.cpp

@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized-const-reference -verify %s
+
+class A {
+public:
+  int i;
+  A(){};
+  A(const A &a){};
+  A(int i) {}
+  bool operator!=(const A &);
+};
+
+A const_ref_use_A(const A &a);
+int const_ref_use(const int &i);
+A const_use_A(const A a);
+int const_use(const int i);
+
+void f() {
+  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}}
+  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}}
+  A a3(const_ref_use_A(a3));    // expected-warning {{variable 'a3' is uninitialized when passed as a const reference argument here}}
+  A a4 = a3 != a4;              // expected-warning {{variable 'a4' is uninitialized when used within its own initialization}} expected-warning {{variable 'a4' is uninitialized when passed as a const reference argument here}}
+  int n = n;                    // expected-warning {{variable 'n' is uninitialized when used within its own initialization}}
+  const_ref_use(n);
+
+  A a5;
+  const_ref_use_A(a5);
+}