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

[zequanwu]

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.

[rnk]

Arthur recently went through the process of refining a diagnostic, so he should be able to help guide you in this if you have more questions.

clang/lib/Analysis/UninitializedValues.cpp

426

Based on Richard's comment here:
https://bugs.llvm.org/show_bug.cgi?id=45624#c7
... this should be its own classification kind, perhaps ConstRefUse or something like that.

This will allow us to put this finding under it's own diagnostic group, maybe named something like -Wuninitialized-const-reference, and then users can enable or disable it separately. It is likely that this new behavior will find many new instances over existing codebases, for example Chromium, that compile with -Wuninitialized enabled. So, if we don't leave a way for users to disable the new warnings (-Wno-uninitialized-const-ref), it is harder for them (us) to update the compiler.

[zequanwu]

Add new warning as subgroup of Uninitialized.

[zequanwu]

Use another mapvector constRefUses to store uninitialized const reference uses so that the new warning can be easily disable by -Wno-initialized-const-reference

[aeubanks]

The summary of this change should be "add warning..." not "fix warning..."

Could you add more detail into the commit description? About newly introduced diagnostic groups, etc.

Also, I thought I saw tests in a previous revision but now they seem to be gone?

[zequanwu]

Since the new warning is controlled by -Wuninitialized, I disabled it in existing test case and added a separate test case for -Wuninitialized-const-reference.

[aeubanks]

clang/include/clang/Basic/DiagnosticSemaKinds.td

2110

s/passes/passed

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

24

For my knowledge, is this to make sure that this other warning takes precedence over the one introduced in this change? If it is, a comment would be nice.

[rnk]

clang/lib/Sema/AnalysisBasedWarnings.cpp

1517

If possible, it would be nice to avoid having a second map.

[zequanwu]

clang/lib/Sema/AnalysisBasedWarnings.cpp

1517

I use second map to let the new warning be orthogonal to existing warnings. That means when we have both -Wuninitialized and -Wno-uninitialized-const-reference, the old test cases about uninitialized variables should all passed. Otherwise, I need to somehow detect the presence of -Wno-uninitialized-const-reference, which I don't see a way to do that. Consider this code if we have only one map:

void consume_const_ref(const int &n);
int test_const_ref() {
  int n;
  consume_const_ref(n);
  return n;
}

No matter if the flag`-Wno-uninitialized-const-reference` is present or not, we still need to add n in consume_const_ref(n); to uses map and then let S.Diag to decide to diagnose it or not depend on the presence of -Wno-uninitialized-const-reference. If the flag is given, S.Diag will just ignore the warning, but uninit var warning will diagnose if Use.getKind() returns Always in https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/AnalysisBasedWarnings.cpp#L814 . That is not desired. Since the original behavior is to ignore the n in consume_const_ref(n); by not adding it to the uses map.

If there is a way to detect the presence of -Wno-uninitialized-const-reference or to know if Sema::Diag(SourceLocation Loc, unsigned DiagID) actually diagnosed a warning, the use of second map can be avoid.

[rsmith]

clang/lib/Sema/AnalysisBasedWarnings.cpp

1517

Interesting testcase :)

Ideally we want the set of diagnostics from a particular compilation to be the same you'd get by building with -Weverything and then filtering out the ones you didn't want. So it's better to not ask whether -Wuninitialized-const-reference is enabled or not.

I think we only populate this map after we've found an uninitialized use; if so, the cost of a second map seems likely to be acceptable.

FYI, there is a mechanism to ask the "is this diagnostic enabled?" question, but generally we'd prefer if it's only used to avoid doing work that would be redundant if the warning is disabled. You can use Diag.isIgnored(diag::warn_..., Loc) to determine if a diagnostic with the given ID would be suppressed at the given location. (In fact, it looks like this is already being called in a later change in this file.)

1590–1600

Do we want any idiomatic-self-init special-case handling here? For example:

void f(const int&);
void g() {
  int a = a;
  f(a);
}

Following the logic above, should that warn on the int a = a; not on the f(a) call? Or should we warn only on the f(a) call itself in this case? It seems like it might be surprising to say that a is "uninitialized" here, since an initializer was provided, even though it was a garbage one.

[xbolva00]

Can you provide some compile time data with warning enabled/disabled?

[zequanwu]

Fix typo.
Diagnose self-init warning if the self-init variable is used as const reference later.

[zequanwu]

clang/lib/Sema/AnalysisBasedWarnings.cpp

1517

I think second map is necessary. So, Diag.isIgnored is not abused.

1590–1600

For this case, I think we should warn at int a = a, like the comment in DiagnoseUninitializedUse said, https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/AnalysisBasedWarnings.cpp#L986-L996

f(a) is considered as accessing a.

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

24

No, original behavior of -Wuninitialized for line 24 is silence. Now the new warning will be emitted because it is const use of uninitialized variable.

[zequanwu]

In D79895#2042992, @xbolva00 wrote:

Can you provide some compile time data with warning enabled/disabled?

I compiled the test case with warning enabled and disabled. Since it is controlled by -Wuninitialzed, it will diagnose when the flag is given.

$ clang -ftime-report -fsyntax-only warn-uninitialized-const-reference.cpp
===-------------------------------------------------------------------------===
                          Clang front-end time report
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0094 seconds (0.0094 wall clock)

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.0091 (100.0%)   0.0003 (100.0%)   0.0094 (100.0%)   0.0094 (100.0%)  Clang front-end timer
   0.0091 (100.0%)   0.0003 (100.0%)   0.0094 (100.0%)   0.0094 (100.0%)  Total

$ clang -ftime-report -Wuninitialized -fsyntax-only warn-uninitialized-const-reference.cpp
===-------------------------------------------------------------------------===
                          Clang front-end time report
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0131 seconds (0.0131 wall clock)

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.0046 (100.0%)   0.0086 (100.0%)   0.0131 (100.0%)   0.0131 (100.0%)  Clang front-end timer
   0.0046 (100.0%)   0.0086 (100.0%)   0.0131 (100.0%)   0.0131 (100.0%)  Total

$ clang -ftime-report -Wuninitialized -Wno-uninitialized-const-reference -fsyntax-only warn-uninitialized-const-reference.cpp
===-------------------------------------------------------------------------===
                          Clang front-end time report
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0109 seconds (0.0109 wall clock)

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.0073 (100.0%)   0.0036 (100.0%)   0.0109 (100.0%)   0.0109 (100.0%)  Clang front-end timer
   0.0073 (100.0%)   0.0036 (100.0%)   0.0109 (100.0%)   0.0109 (100.0%)  Total

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

24

I meant "const reference use". It was typo.

[aeubanks]

One last nit, otherwise lgtm. But I'm not super familiar with this area, it'd be good to get another review from somebody more familiar.

clang/lib/Sema/AnalysisBasedWarnings.cpp

1522–1523

this is very confusing since it shadows the uses member variable from UninitValsDiagReporter

1590–1600

Doesn't DiagnoseUninitializedUse say that we shouldn't warn at int a = a?

[zequanwu]

rename parameter uses to um

clang/lib/Sema/AnalysisBasedWarnings.cpp

1590–1600

The last sentence of the comment says that int a = a left a as uninitialized state, and we should warn at int a = a if there is an access of a in uninitialized state. Otherwise, we don't need to warn.

[zequanwu]

@rsmith Are you okay with this patch?

[hans]

Very nice! I only have minor comments.

Also I'm curious to see what this will find in Chromium. I guess we'll find out :-)

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

118

nit: May put this after handleUseOfUninitVariable() instead, since it's very similar.

clang/include/clang/Basic/DiagnosticSemaKinds.td

2110

Minor detail, but I think it should say "argument" instead of "parameter".

My understanding is that parameters are names used when declaring/defining functions, e, g.
int foo(int x) { ... }
here x is a parameter.

And arguments are used when calling functions:
foo(42);
here 42 is an argument.

(https://en.wikipedia.org/wiki/Parameter_(computer_programming))

clang/lib/Analysis/UninitializedValues.cpp

891

I'd probably move this to right after handleUseOfUninitVariable() since they're similar.

clang/lib/Sema/AnalysisBasedWarnings.cpp

1590

nit: capital f and period at the end.

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

5

nit: I think 2 spaces for indentation is more common in this code (applies also below).

[zequanwu]

Rebase and address comments.

[MaskRay]

clang/lib/Analysis/UninitializedValues.cpp

676

This should use isAlwaysUninit. I fixed it in 7096e04a6831d4668c39b388ccd166f84de69191

Otherwise there are multiple false positives in stage2 build of llvm-project, e.g.

if (a < 42)
  var = 1;
if (a < 42)
  const_ref_use(var);

[zequanwu]

clang/lib/Analysis/UninitializedValues.cpp

676

Thanks for the fix

[sylvestre.ledru]

@zequanwu could you please update the release notes? thanks
https://github.com/llvm/llvm-project/blob/master/clang/docs/ReleaseNotes.rst

[zequanwu]

In D79895#2073698, @sylvestre.ledru wrote:

@zequanwu could you please update the release notes? thanks
https://github.com/llvm/llvm-project/blob/master/clang/docs/ReleaseNotes.rst

Done.

[nick]

This diagnostic bring headaches because frequently -Wunused-variable suppression is done via no-op pseudo-consuming function like [boost::ignore_unused](https://www.boost.org/doc/libs/1_73_0/libs/core/doc/html/core/ignore_unused.html). Particularly, it fires in Boost here https://github.com/boostorg/concept_check/blob/e69c81326d5a4359ac53f9c6fe53fc2baf24df50/include/boost/concept_check.hpp#L135-L141.
Is it possible to make the diagnostic not fire for empty body consuming functions?

[zequanwu]

In D79895#2107604, @nick wrote:

This diagnostic bring headaches because frequently -Wunused-variable suppression is done via no-op pseudo-consuming function like [boost::ignore_unused](https://www.boost.org/doc/libs/1_73_0/libs/core/doc/html/core/ignore_unused.html). Particularly, it fires in Boost here https://github.com/boostorg/concept_check/blob/e69c81326d5a4359ac53f9c6fe53fc2baf24df50/include/boost/concept_check.hpp#L135-L141.
Is it possible to make the diagnostic not fire for empty body consuming functions?

This warning can be turned off by the flag -Wno-uninitialized-const-reference. I don't think we can just make the diagnostic not fire for empty body consuming functions, if the function declaration and definition are in different translation units.

[nick]

This warning can be turned off by the flag -Wno-uninitialized-const-reference.

Suggesting to turn off the warning should be the last resort. I am pointing to the false positives for large existing code bases from -Wall diagnostic.

I don't think we can just make the diagnostic not fire for empty body consuming functions, if the function declaration and definition are in different translation units.

I am talking about the particular situation that is involves only inline functions with empty bodies. boost::ignore_unused-like functions are obviously come with definition.

[aeubanks]

In D79895#2107796, @nick wrote:

This warning can be turned off by the flag -Wno-uninitialized-const-reference.

Suggesting to turn off the warning should be the last resort. I am pointing to the false positives for large existing code bases from -Wall diagnostic.

I don't think we can just make the diagnostic not fire for empty body consuming functions, if the function declaration and definition are in different translation units.

I am talking about the particular situation that is involves only inline functions with empty bodies. boost::ignore_unused-like functions are obviously come with definition.

I feel like doing interprocedural analysis for this is overkill. What is the benefit of boost::ignore_unused(foo); rather than the more common (void) foo;? Any examples?

[hans]

In D79895#2107604, @nick wrote:

This diagnostic bring headaches because frequently -Wunused-variable suppression is done via no-op pseudo-consuming function like [boost::ignore_unused](https://www.boost.org/doc/libs/1_73_0/libs/core/doc/html/core/ignore_unused.html).

I haven't seen boost::ignore_unused before. In my experience, the idiomatic way of ignoring an unused variable in C/C++ is to cast it to void, as Arthur said.

Particularly, it fires in Boost here https://github.com/boostorg/concept_check/blob/e69c81326d5a4359ac53f9c6fe53fc2baf24df50/include/boost/concept_check.hpp#L135-L141.
Is it possible to make the diagnostic not fire for empty body consuming functions?

The combination of having an uninitialized variable and explicitly marking it unused like this seems to me like it would be unusual.

[nick]

I feel like doing interprocedural analysis for this is overkill. What is the benefit of boost::ignore_unused(foo); rather than the more common (void) foo;? Any examples?

I haven't seen boost::ignore_unused before. In my experience, the idiomatic way of ignoring an unused variable in C/C++ is to cast it to void, as Arthur said.

This is a weak argument to have false positives, don't you agree? You may have not seen it, but it exists and is used: https://github.com/search?q=%22boost%3A%3Aignore_unused%22+NOT+%22Boost+Software+License%22&type=Code

The combination of having an uninitialized variable and explicitly marking it unused like this seems to me like it would be unusual.

It could be a variable used exclusively inside an assert or some other conditionally no-op macro-function.

[hans]

In D79895#2109414, @nick wrote:

I feel like doing interprocedural analysis for this is overkill. What is the benefit of boost::ignore_unused(foo); rather than the more common (void) foo;? Any examples?

I haven't seen boost::ignore_unused before. In my experience, the idiomatic way of ignoring an unused variable in C/C++ is to cast it to void, as Arthur said.

This is a weak argument to have false positives, don't you agree? You may have not seen it, but it exists and is used: https://github.com/search?q=%22boost%3A%3Aignore_unused%22+NOT+%22Boost+Software+License%22&type=Code

There are plenty of warnings which have false positives on non-idiomatic code though. The question is how common this pattern of using a function to ignore an unused variable is. We didn't see it in the code bases I work with, so is boost a special case, or an example of a common practice? If it's just boost, fixing the code seems better (it will compile faster too).

[nick]

We didn't see it in the code bases I work with, so is boost a special case, or an example of a common practice?

I do not have resources to make such statistics, but there are compilers where casting to void is not enough to suppress the warning. https://herbsutter.com/2009/10/18/mailbag-shutting-up-compiler-warnings/#comment-1509 https://godbolt.org/z/pS_iQ3

If it's just boost, fixing the code seems better

Have you tried to push a fix for a warning in Boost? If it was that simple. A part of my warning-fixing PRs are not merged in years. And considering that fixing this warning will reintroduce warnings for other compilers I probably will have a bad luck with this one too.

(it will compile faster too).

Should I open a PR with replacing std::forward with static_cast because it compiles faster? :-)

[aeubanks]

In D79895#2110345, @nick wrote:

We didn't see it in the code bases I work with, so is boost a special case, or an example of a common practice?

I do not have resources to make such statistics, but there are compilers where casting to void is not enough to suppress the warning. https://herbsutter.com/2009/10/18/mailbag-shutting-up-compiler-warnings/#comment-1509 https://godbolt.org/z/pS_iQ3

If it's just boost, fixing the code seems better

Have you tried to push a fix for a warning in Boost? If it was that simple. A part of my warning-fixing PRs are not merged in years. And considering that fixing this warning will reintroduce warnings for other compilers I probably will have a bad luck with this one too.

(it will compile faster too).

Should I open a PR with replacing std::forward with static_cast because it compiles faster? :-)

Perhaps asking for more opinions in cfe-dev will be better.

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

public:
   /// Called when the uninitialized variable is used at the given expression.
   virtual void handleUseOfUninitVariable(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.
   virtual void handleSelfInit(const VarDecl *vd) {}
+
+  virtual void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+                                                 const UninitUse &use) {}
 };
 
 struct UninitVariablesAnalysisStats {
   unsigned NumVariablesAnalyzed;
   unsigned NumBlockVisits;
 };
 
 void runUninitializedVariablesAnalysis(const DeclContext &dc, const CFG &cfg,
                                        AnalysisDeclContext &ac,
                                        UninitVariablesHandler &handler,
                                        UninitVariablesAnalysisStats &stats);
 
 } // namespace clang
 
 #endif // LLVM_CLANG_ANALYSIS_ANALYSES_UNINITIALIZEDVALUES_H

clang/include/clang/Basic/DiagnosticGroups.td

 
 def : DiagGroup<"type-limits">;
 def UndefinedReinterpretCast : DiagGroup<"undefined-reinterpret-cast">;
 def ReinterpretBaseClass : DiagGroup<"reinterpret-base-class">;
 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
 // (#pragma clang optimize, noinline) so suggest nothing for now.
 def IgnoredPragmaOptimize : DiagGroup<"ignored-pragma-optimize">;
 def UnknownPragmas : DiagGroup<"unknown-pragmas">;
 def IgnoredPragmas : DiagGroup<"ignored-pragmas",
     [IgnoredPragmaIntrinsic, IgnoredPragmaOptimize]>;

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def note_uninit_fixit_remove_cond : Note<
   "remove the %select{'%1' if its condition|condition if it}0 "
   "is always %select{false|true}2">;
 def err_init_incomplete_type : Error<"initialization of incomplete type %0">;
 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 parameter "
+  "here">, InGroup<UninitializedConstReference>, DefaultIgnore;
+
 def warn_unsequenced_mod_mod : Warning<
   "multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
 def warn_unsequenced_mod_use : Warning<
   "unsequenced modification and access to %0">, InGroup<Unsequenced>;
 
 def select_initialized_entity_kind : TextSubstitution<
   "%select{copying variable|copying parameter|"
   "returning object|initializing statement expression result|"

clang/lib/Analysis/UninitializedValues.cpp

 /// DeclRefExpr which isn't explicitly classified will be assumed to have
 /// escaped the analysis and will be treated as an initialization.
 class ClassifyRefs : public StmtVisitor<ClassifyRefs> {
 public:
   enum Class {
     Init,
     Use,
     SelfInit,
+    ConstRefUse,
     Ignore
   };
 
 private:
   const DeclContext *DC;
   llvm::DenseMap<const DeclRefExpr *, Class> Classification;
 
   bool isTrackedVar(const VarDecl *VD) const {
void ClassifyRefs::VisitCallExpr(CallExpr *CE) {
   // Classify arguments to std::move as used.
   if (CE->isCallToStdMove()) {
     // RecordTypes are handled in SemaDeclCXX.cpp.
     if (!CE->getArg(0)->getType()->isRecordType())
       classify(CE->getArg(0), Use);
     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);
       if (UO && UO->getOpcode() == UO_AddrOf)
         Ex = UO->getSubExpr();
       classify(Ex, Ignore);
     }
   }
TransferFunctions(CFGBlockValues &vals, const CFG &cfg,
                     const CFGBlock *block, AnalysisDeclContext &ac,
                     const ClassifyRefs &classification,
                     UninitVariablesHandler &handler)
       : vals(vals), cfg(cfg), block(block), ac(ac),
         classification(classification), objCNoRet(ac.getASTContext()),
         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);
   void VisitCallExpr(CallExpr *ce);
   void VisitDeclRefExpr(DeclRefExpr *dr);
   void VisitDeclStmt(DeclStmt *ds);
   void VisitGCCAsmStmt(GCCAsmStmt *as);
   void VisitObjCForCollectionStmt(ObjCForCollectionStmt *FS);

 } // namespace
 
 void TransferFunctions::reportUse(const Expr *ex, const VarDecl *vd) {
   Value v = vals[vd];
   if (isUninitialized(v))
     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())) {
     const auto *VD = cast<VarDecl>(DS->getSingleDecl());
     if (isTrackedVar(VD))
       vals[VD] = Initialized;
   }
 }
case ClassifyRefs::Ignore:
     break;
   case ClassifyRefs::Use:
     reportUse(dr, cast<VarDecl>(dr->getDecl()));
     break;
   case ClassifyRefs::Init:
     vals[cast<VarDecl>(dr->getDecl())] = Initialized;
     break;
   case ClassifyRefs::SelfInit:
-      handler.handleSelfInit(cast<VarDecl>(dr->getDecl()));
     break;
+  case ClassifyRefs::ConstRefUse:
+    reportConstRefUse(dr, cast<VarDecl>(dr->getDecl()));
+    break;
   }
 }
 
 void TransferFunctions::VisitBinaryOperator(BinaryOperator *BO) {
   if (BO->getOpcode() == BO_Assign) {
     FindVarResult Var = findVar(BO->getLHS());
     if (const VarDecl *VD = Var.getDecl())
       vals[VD] = Initialized;
struct PruneBlocksHandler : public UninitVariablesHandler {
 
   /// Called when the uninitialized variable analysis detects the
   /// idiom 'int x = x'.  All other uses of 'x' within the initializer
   /// are handled by handleUseOfUninitVariable.
   void handleSelfInit(const VarDecl *vd) override {
     hadUse[currentBlock] = true;
     hadAnyUse = true;
   }
+
+  void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+                                         const UninitUse &use) override {
+    hadUse[currentBlock] = true;
+    hadAnyUse = true;
+  }
 };
 
 } // namespace
 
 void clang::runUninitializedVariablesAnalysis(
     const DeclContext &dc,
     const CFG &cfg,
     AnalysisDeclContext &ac,

clang/lib/Sema/AnalysisBasedWarnings.cpp

static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
   }
 
   if (!Diagnosed)
     S.Diag(Use.getUser()->getBeginLoc(), diag::warn_maybe_uninit_var)
         << VD->getDeclName() << IsCapturedByBlock
         << 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
 /// as a warning. If a particular use is one we omit warnings for, returns
 /// false.
 static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
                                      const UninitUse &Use,
                                      bool alwaysReportSelfInit = false) {
class UninitValsDiagReporter : public UninitVariablesHandler {
   Sema &S;
   typedef SmallVector<UninitUse, 2> UsesVec;
   typedef llvm::PointerIntPair<UsesVec *, 1, bool> MappedType;
   // Prefer using MapVector to DenseMap, so that iteration order will be
   // the same as insertion order. This is needed to obtain a deterministic
   // 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;
   }
 
   void handleUseOfUninitVariable(const VarDecl *vd,
                                  const UninitUse &use) override {
-    getUses(vd).getPointer()->push_back(use);
+    getUses(uses, 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 handleConstRefUseOfUninitVariable(const VarDecl *vd,
+                                         const UninitUse &use) override {
+    getUses(constRefUses, vd).getPointer()->push_back(use);
   }
 
   void flushDiagnostics() {
     for (const auto &P : uses) {
       const VarDecl *vd = P.first;
       const MappedType &V = P.second;
 
       UsesVec *vec = V.getPointer();
for (const auto &P : uses) {
         }
       }
 
       // Release the uses vector.
       delete vec;
     }
 
     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:
   static bool hasAlwaysUninitializedUse(const UsesVec* vec) {
     return std::any_of(vec->begin(), vec->end(), [](const UninitUse &U) {
       return U.getKind() == UninitUse::Always ||
              U.getKind() == UninitUse::AfterCall ||
              U.getKind() == UninitUse::AfterDecl;
AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
   if (P.enableConsumedAnalysis) {
     consumed::ConsumedWarningsHandler WarningHandler(S);
     consumed::ConsumedAnalyzer Analyzer(WarningHandler);
     Analyzer.run(AC);
   }
 
   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;
       std::memset(&stats, 0, sizeof(UninitVariablesAnalysisStats));
       runUninitializedVariablesAnalysis(*cast<DeclContext>(D), *cfg, AC,
                                         reporter, stats);
 
       if (S.CollectStats && stats.NumVariablesAnalyzed > 0) {

clang/test/Misc/warning-wall.c

 CHECK-NEXT:      -Wtautological-overlap-compare
 CHECK-NEXT:      -Wtautological-bitwise-compare
 CHECK-NEXT:      -Wtautological-undefined-compare
 CHECK-NEXT:      -Wtautological-objc-bool-compare
 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
 CHECK-NEXT:      -Wunused-function
 CHECK-NEXT:        -Wunneeded-internal-declaration
 CHECK-NEXT:      -Wunused-label
 CHECK-NEXT:      -Wunused-private-field
 CHECK-NEXT:      -Wunused-lambda-capture

clang/test/SemaCXX/uninit-variables.cpp

-// 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 {}; }
 
 int test1_aux(int &x);
 int test1() {
   int x;
   test1_aux(x);

clang/test/SemaCXX/uninitialized.cpp

-// 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 {
 inline namespace foo {
 template <class T> struct remove_reference { typedef T type; };
 template <class T> struct remove_reference<T&> { typedef T type; };
 template <class T> struct remove_reference<T&&> { typedef T type; };
 

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

+// 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 parameter 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 parameter here}}
+    A a1 = const_ref_use_A(a1); // expected-warning {{variable 'a1' is uninitialized when passed as a const reference parameter 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 parameter 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 parameter 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);
+}