Check possible warnings on global initializers for reachability

[Nathan-Huckleberry]

Summary

Create CFG for initializers and perform analysis based warnings on global variables

[pirama]

clang/include/clang/Sema/AnalysisBasedWarnings.h

111

Fix indentation.

clang/lib/Sema/AnalysisBasedWarnings.cpp

2007

Should PossiblyUnreachableDiags be const?

Also, fix indentation.

2015

How about returning early if PossiblyUnreachableDiags here is empty? That'll avoid putting the entire logic of this function in the true branch.

2031

getCFGReachablityAnalysis has a typo. It's missing an 'i'. Consider fixing this in a separate patch.

2051

Rewrite this as the else clause for if (AC.getCFG())? In the current structure, it's not immediately clear that flushDiagnostics is called iff AC.getCFG() fails to return a valid CFG.

Upon further reading, this seems to be refactored from another function below so it probably makes sense to leave it as-is.

[nickdesaulniers]

clang/include/clang/Sema/AnalysisBasedWarnings.h

101

git-clang-format HEAD~<number of commits>

The formal parameter should be abbreviated, maybe PUD?

110

How about making this a proper method of AnalysisBasedWarnings rather than a free floating function that's only ever called from other methods of AnalysisBasedWarnings? That way you don't have to pass in a Sema.

clang/lib/Analysis/AnalysisDeclContext.cpp

124

The const_cast doesn't look necessary here. Is it?

clang/lib/Sema/AnalysisBasedWarnings.cpp

2012

having the full namespace spelled out here in the definition smells funny. Is there a pair of namespace blocks below for clang and sema where the definition of emitPossiblyUnreachableDiags could be moved into?

Actually looking at the file, I see you simply matched the existing style, but line 49 currently has a using statement that should inject the clang namespace into the current scope. That's why you see sema:: used in other places in this translation unit without the clang namespace prefix. The whole file should remove the clang:: prefixes or additionally replace the using statement with explicit namespace blocks.

2260

PUD

2275

If you make emitPossiblyUnreachableDiags then it doesn't need all the prefixes.

clang/lib/Sema/SemaDeclCXX.cpp

347

Use punctuation in your comments.

clang/lib/Sema/SemaExpr.cpp

16678

Punctuation.

clang/test/Sema/warn-unreachable-warning-var-decl.cpp

39–40

ok and also ok can probably be removed?

[Nathan-Huckleberry]

clang/lib/Analysis/AnalysisDeclContext.cpp

124

VD->getInit returns a const Expr *. In order to remove the const_cast I would need to make getBody() const and every call to getBody(). The change required to add const seemed too large to be included in this patch.

[Nathan-Huckleberry]

• Small functional and formatting changes

[nickdesaulniers]

clang/include/clang/Sema/AnalysisBasedWarnings.h

94

Methods should be UpperCamelCased.

110

UpperCamelCase

clang/lib/Sema/SemaExpr.cpp

16668

does this need the clang:: qualifier?

16706

does this need clang::?

[Nathan-Huckleberry]

• Stylistic fixes of function names and removal of namespace prefixes

[rsmith]

clang/lib/Sema/SemaDeclCXX.cpp

352

We may need to delay the diagnostics here until the default argument is *used*: if a default argument references a template instantiation, the instantiation is not performed until that point, which may mean that our semantic checking can't complete correctly until use.

clang/lib/Sema/SemaExpr.cpp

16699

It's not correct to assume that the last declaration in the context is the right one to be considering here. Instead, you should track whether you're in a variable initializer (and for what) in Sema. Examples:

int x = ([]{}(), x); // in C++; last decl is the lambda

int y = (struct Q { int n; }){y}; // in C; last decl is the struct

[Nathan-Huckleberry]

• Add tracking of declaration of initializers in Sema.

[nickdesaulniers]

clang/include/clang/Sema/Sema.h

1689 ↗

(On Diff #211213)

might be nice to return the result, but maybe YAGNI?

[nickdesaulniers]

clang/include/clang/Sema/Sema.h

1698 ↗

(On Diff #211213)

Does:
return DeclForInitializer.empty() ? DeclForInitializer.back() : nullptr;
fit on one line?

clang/lib/Sema/AnalysisBasedWarnings.cpp

2088

I'm not sure that analyzed, block, or cra follow the naming conventions used throughout the codebase.

[Nathan-Huckleberry]

• Style fixes

[Nathan-Huckleberry]

• Style fixes

[nickdesaulniers]

clang/lib/Sema/AnalysisBasedWarnings.cpp

2061

is clang namespace required here?

[Nathan-Huckleberry]

clang/lib/Sema/SemaDeclCXX.cpp

352

Currently this patch really only works with globals. There are many places where the following check is made instead of calling ParseInitializer. These should probably be rewritten into a single function and do something similar with pushing/popping declarations. Not sure if that change should be made in this patch or not.

if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
            Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);
            DefArgResult = ParseBraceInitializer();
          } else
            DefArgResult = ParseAssignmentExpression();

[rsmith]

clang/include/clang/Sema/Sema.h

1684–1686 ↗

(On Diff #211224)

I don't think a simple list of these can work. Consider a case like:

auto x = [] {
  // something with a runtime diagnostic
};

Here, we'll have a DeclForInitializer set, so we'll suppress warnings on the body of the lambda, but we shouldn't: the body of the lambda is a completely different evaluation context from the initialization of the variable x.

Can you store the declaration on the ExpressionEvaluationContextRecord instead of storing a list of them directly on Sema? (You shouldn't need a list there, just a single pointer should work, since we can't parse two nested initializers without an intervening evaluation context.)

clang/lib/Sema/AnalysisBasedWarnings.cpp

2067

Please capitalize the names of variables.

clang/lib/Sema/SemaExpr.cpp

16680–16700

Please get rid of the pre-existing hacky use of ManglingContextDecl here and move this into the getDeclForInitializer block below.

[Nathan-Huckleberry]

• Use ExprEvalContext and remove mangling context code

[Nathan-Huckleberry]

clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp

360 ↗

(On Diff #215699)

These new warnings seem valid.

[rsmith]

clang/include/clang/Parse/Parser.h

1873 ↗

(On Diff #215699)

This should be done by calling a function on Sema (add an ActOnStartDeclInitializer or similar), not by directly modifying Sema's internal state.

1880 ↗

(On Diff #215699)

Please add an assertion when you first set this that the old value was null.

clang/lib/Sema/AnalysisBasedWarnings.cpp

2285

Consider grabbing VarDeclPossiblyUnreachableDiags[VD] first and avoiding constructing the AnalysisDeclContext at all in the (presumably overwhelmingly common case) that there are no such diagnostics.

clang/lib/Sema/SemaDecl.cpp

11853

We should (presumably) only do this for file-scope variables. The initializers for block-scope variables will be checked when checking the enclosing function (if the variables' declarations are reachable at all).

clang/lib/Sema/SemaExpr.cpp

4884

We should only do this once, when we instantiate the default argument, rather than once each time we use it. (Move this up to just before line 4856?)

16665–16666

This call to getCurFunctionOrMethodDecl() is wrong; it will skip over lambda-expressions. (This is why you're still seeing diagnostics in file-scope lambdas.) Something like this should work:

if (Stmts.empty()) {
  Diag(Loc, PD);
  return true;
}

if (FunctionScopeInfo *FSI = getCurFunction()) {
  FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
      PossiblyUnreachableDiag(PD, Loc, Stmts));
  return true;
}

// [handle VarDecl case]

16682–16684

No braces here please. Also, this appears to be wrong: we want the declaration with the initializer (which is always VD), not the definition (which might be a different declaration for a static data member), don't we?

16695–16700

What does this fixme mean?

clang/test/SemaTemplate/instantiate-static-var.cpp

9 ↗

(On Diff #215699)

We should not warn here (because this initializer is required to be a constant expression).

clang/include/clang/AST/DeclBase.h

public:
   /// getParent - Returns the containing DeclContext.
   DeclContext *getParent() {
     return cast<Decl>(this)->getDeclContext();
   }
   const DeclContext *getParent() const {
     return const_cast<DeclContext*>(this)->getParent();
   }
 
+  Decl *getLastDecl() const {
+    return LastDecl;
+  }
+
   /// getLexicalParent - Returns the containing lexical DeclContext. May be
   /// different from getParent, e.g.:
   ///
   ///   namespace A {
   ///      struct S;
   ///   }
   ///   struct A::S {}; // getParent() == namespace 'A'
   ///                   // getLexicalParent() == translation unit

clang/include/clang/Sema/AnalysisBasedWarnings.h

 //=- AnalysisBasedWarnings.h - Sema warnings based on libAnalysis -*- C++ -*-=//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines AnalysisBasedWarnings, a worker object used by Sema
 // that issues warnings based on dataflow-analysis.
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H
 #define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H
 
+#include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/MapVector.h"
 
 namespace clang {
 
 class BlockExpr;
 class Decl;
 class FunctionDecl;
 class ObjCMethodDecl;
 class QualType;
 class Sema;
+class AnalysisDeclContext;
 namespace sema {
   class FunctionScopeInfo;
+  class PossiblyUnreachableDiag;
 }
 
 namespace sema {
 
 class AnalysisBasedWarnings {
 public:
   class Policy {
     friend class AnalysisBasedWarnings;
     // The warnings to run.
     unsigned enableCheckFallThrough : 1;
     unsigned enableCheckUnreachable : 1;
     unsigned enableThreadSafetyAnalysis : 1;
     unsigned enableConsumedAnalysis : 1;
   public:
     Policy();
     void disableCheckFallThrough() { enableCheckFallThrough = 0; }
   };
 
 private:
   Sema &S;
   Policy DefaultPolicy;
 
   enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 };
   llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD;
+  llvm::MapVector<VarDecl*, SmallVector<PossiblyUnreachableDiag, 4>> VarDeclPossiblyUnreachableDiags;
 
   /// \name Statistics
   /// @{
 
   /// Number of function CFGs built and analyzed.
   unsigned NumFunctionsAnalyzed;
 
   /// Number of functions for which the CFG could not be successfully
private:
   unsigned NumUninitAnalysisBlockVisits;
 
   /// Max number of block visits during uninitialized use analysis of
   /// a single function.
   unsigned MaxUninitAnalysisBlockVisitsPerFunction;
 
   /// @}
 
 public:
   AnalysisBasedWarnings(Sema &s);
 
   void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
                      const Decl *D, QualType BlockType);
 
+  void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag
+                       PossiblyUnreachableDiag);
+
+  void IssueWarningsForRegisteredVarDecl(VarDecl *VD);
+
   Policy getDefaultPolicy() { return DefaultPolicy; }
 
   void PrintStats() const;
 };
 
+void emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC,
+SmallVector<PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags);
+
 }} // end namespace clang::sema
 
 #endif

clang/lib/Analysis/AnalysisDeclContext.cpp

if (Manager && Manager->synthesizeBodies()) {
         IsAutosynthesized = true;
       }
     }
     return Body;
   } else if (const auto *BD = dyn_cast<BlockDecl>(D))
     return BD->getBody();
   else if (const auto *FunTmpl = dyn_cast_or_null<FunctionTemplateDecl>(D))
     return FunTmpl->getTemplatedDecl()->getBody();
+  else if (const auto *VD = dyn_cast_or_null<VarDecl>(D)) {
+    if(VD->hasGlobalStorage()) {
+      return const_cast<Stmt*>(dyn_cast<Stmt>(VD->getInit()));
+    }
+  }
 
   llvm_unreachable("unknown code decl");
 }
 
 Stmt *AnalysisDeclContext::getBody() const {
   bool Tmp;
   return getBody(Tmp);
 }

clang/lib/Sema/AnalysisBasedWarnings.cpp

clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
 
   DefaultPolicy.enableThreadSafetyAnalysis =
     isEnabled(D, warn_double_lock);
 
   DefaultPolicy.enableConsumedAnalysis =
     isEnabled(D, warn_use_in_invalid_state);
 }
 
-static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
-  for (const auto &D : fscope->PossiblyUnreachableDiags)
+static void flushDiagnostics(Sema &S,
+SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags) {
+  for (const auto &D : PossiblyUnreachableDiags)
     S.Diag(D.Loc, D.PD);
 }
 
+void clang::sema::emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC,
+SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags) {
+
+  if (!PossiblyUnreachableDiags.empty()) {
+    bool analyzed = false;
+
+    // Register the expressions with the CFGBuilder.
+    for (const auto &D : PossiblyUnreachableDiags) {
+      for (const Stmt *S : D.Stmts)
+        AC.registerForcedBlockExpression(S);
+    }
+
+    if (AC.getCFG()) {
+      analyzed = true;
+      for (const auto &D : PossiblyUnreachableDiags) {
+        bool AllReachable = true;
+        for (const Stmt *S : D.Stmts) {
+          const CFGBlock *block = AC.getBlockForRegisteredExpression(S);
+          CFGReverseBlockReachabilityAnalysis *cra =
+              AC.getCFGReachablityAnalysis();
+          // FIXME: We should be able to assert that block is non-null, but
+          // the CFG analysis can skip potentially-evaluated expressions in
+          // edge cases; see test/Sema/vla-2.c.
+          if (block && cra) {
+            // Can this block be reached from the entrance?
+            if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) {
+              AllReachable = false;
+              break;
+            }
+          }
+          // If we cannot map to a basic block, assume the statement is
+          // reachable.
+        }
+
+        if (AllReachable)
+          S.Diag(D.Loc, D.PD);
+      }
+    }
+
+    if (!analyzed)
+      flushDiagnostics(S, PossiblyUnreachableDiags);
+  }
+}
+
 void clang::sema::
 AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
                                      sema::FunctionScopeInfo *fscope,
                                      const Decl *D, QualType BlockType) {
 
   // We avoid doing analysis-based warnings when there are errors for
   // two reasons:
   // (1) The CFGs often can't be constructed (if the body is invalid), so
   //     don't bother trying.
   // (2) The code already has problems; running the analysis just takes more
   //     time.
   DiagnosticsEngine &Diags = S.getDiagnostics();
 
   // Do not do any analysis if we are going to just ignore them.
   if (Diags.getIgnoreAllWarnings() ||
       (Diags.getSuppressSystemWarnings() &&
        S.SourceMgr.isInSystemHeader(D->getLocation())))
     return;
 
   // For code in dependent contexts, we'll do this at instantiation time.
   if (cast<DeclContext>(D)->isDependentContext())
     return;
 
   if (Diags.hasUncompilableErrorOccurred()) {
     // Flush out any possibly unreachable diagnostics.
-    flushDiagnostics(S, fscope);
+    flushDiagnostics(S, fscope->PossiblyUnreachableDiags);
     return;
   }
 
   const Stmt *Body = D->getBody();
   assert(Body);
 
   // Construct the analysis context with the specified CFG build options.
   AnalysisDeclContext AC(/* AnalysisDeclContextManager */ nullptr, D);
 
   // Don't generate EH edges for CallExprs as we'd like to avoid the n^2
   // explosion for destructors that can result and the compile time hit.
   AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true;
   AC.getCFGBuildOptions().AddEHEdges = false;
   AC.getCFGBuildOptions().AddInitializers = true;
   AC.getCFGBuildOptions().AddImplicitDtors = true;
AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
   llvm::Optional<LogicalErrorHandler> LEH;
   if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison,
                        D->getBeginLoc())) {
     LEH.emplace(S);
     AC.getCFGBuildOptions().Observer = &*LEH;
   }
 
   // Emit delayed diagnostics.
-  if (!fscope->PossiblyUnreachableDiags.empty()) {
-    bool analyzed = false;
-
-    // Register the expressions with the CFGBuilder.
-    for (const auto &D : fscope->PossiblyUnreachableDiags) {
-      for (const Stmt *S : D.Stmts)
-        AC.registerForcedBlockExpression(S);
-    }
-
-    if (AC.getCFG()) {
-      analyzed = true;
-      for (const auto &D : fscope->PossiblyUnreachableDiags) {
-        bool AllReachable = true;
-        for (const Stmt *S : D.Stmts) {
-          const CFGBlock *block = AC.getBlockForRegisteredExpression(S);
-          CFGReverseBlockReachabilityAnalysis *cra =
-              AC.getCFGReachablityAnalysis();
-          // FIXME: We should be able to assert that block is non-null, but
-          // the CFG analysis can skip potentially-evaluated expressions in
-          // edge cases; see test/Sema/vla-2.c.
-          if (block && cra) {
-            // Can this block be reached from the entrance?
-            if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) {
-              AllReachable = false;
-              break;
-            }
-          }
-          // If we cannot map to a basic block, assume the statement is
-          // reachable.
-        }
-
-        if (AllReachable)
-          S.Diag(D.Loc, D.PD);
-      }
-    }
-
-    if (!analyzed)
-      flushDiagnostics(S, fscope);
-  }
+  emitPossiblyUnreachableDiags(S, AC, fscope->PossiblyUnreachableDiags);
 
   // Warning: check missing 'return'
   if (P.enableCheckFallThrough) {
     const CheckFallThroughDiagnostics &CD =
         (isa<BlockDecl>(D)
              ? CheckFallThroughDiagnostics::MakeForBlock()
              : (isa<CXXMethodDecl>(D) &&
                 cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call &&
if (CFG *cfg = AC.getCFG()) {
       MaxCFGBlocksPerFunction = std::max(MaxCFGBlocksPerFunction,
                                          cfg->getNumBlockIDs());
     } else {
       ++NumFunctionsWithBadCFGs;
     }
   }
 }
 
+void clang::sema::AnalysisBasedWarnings::RegisterVarDeclWarning(VarDecl *VD,
+    clang::sema::PossiblyUnreachableDiag PossiblyUnreachableDiag) {
+  VarDeclPossiblyUnreachableDiags[VD].emplace_back(PossiblyUnreachableDiag);
+}
+
+void clang::sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl(VarDecl *VD) {
+  AnalysisDeclContext AC(nullptr, VD);
+
+  AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true;
+  AC.getCFGBuildOptions().AddEHEdges = false;
+  AC.getCFGBuildOptions().AddInitializers = true;
+  AC.getCFGBuildOptions().AddImplicitDtors = true;
+  AC.getCFGBuildOptions().AddTemporaryDtors = true;
+  AC.getCFGBuildOptions().AddCXXNewAllocator = false;
+  AC.getCFGBuildOptions().AddCXXDefaultInitExprInCtors = true;
+
+  clang::sema::emitPossiblyUnreachableDiags(S, AC, VarDeclPossiblyUnreachableDiags[VD]);
+}
+
 void clang::sema::AnalysisBasedWarnings::PrintStats() const {
   llvm::errs() << "\n*** Analysis Based Warnings Stats:\n";
 
   unsigned NumCFGsBuilt = NumFunctionsAnalyzed - NumFunctionsWithBadCFGs;
   unsigned AvgCFGBlocksPerFunction =
       !NumCFGsBuilt ? 0 : NumCFGBlocks/NumCFGsBuilt;
   llvm::errs() << NumFunctionsAnalyzed << " functions analyzed ("
                << NumFunctionsWithBadCFGs << " w/o CFGs).\n"
                << "  " << NumCFGBlocks << " CFG blocks built.\n"
                << "  " << AvgCFGBlocksPerFunction
                << " average CFG blocks per function.\n"
                << "  " << MaxCFGBlocksPerFunction
                << " max CFG blocks per function.\n";
 
   unsigned AvgUninitVariablesPerFunction = !NumUninitAnalysisFunctions ? 0
       : NumUninitAnalysisVariables/NumUninitAnalysisFunctions;

clang/lib/Sema/SemaDecl.cpp

 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/HeaderSearch.h" // TODO: Sema shouldn't depend on Lex
 #include "clang/Lex/Lexer.h" // TODO: Extract static functions to fix layering.
 #include "clang/Lex/ModuleLoader.h" // TODO: Sema shouldn't depend on Lex
 #include "clang/Lex/Preprocessor.h" // Included for isCodeCompletionEnabled()
+#include "clang/Sema/AnalysisBasedWarnings.h"
 #include "clang/Sema/CXXFieldCollector.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/DelayedDiagnostic.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
Sema::ActOnCXXForRangeIdentifier(Scope *S, SourceLocation IdentLoc,
   FinalizeDeclaration(Var);
   return ActOnDeclStmt(FinalizeDeclaratorGroup(S, DS, Var), IdentLoc,
                        AttrEnd.isValid() ? AttrEnd : IdentLoc);
 }
 
 void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
   if (var->isInvalidDecl()) return;
 
+  AnalysisWarnings.IssueWarningsForRegisteredVarDecl(var);
+
   if (getLangOpts().OpenCL) {
     // OpenCL v2.0 s6.12.5 - Every block variable declaration must have an
     // initialiser
     if (var->getTypeSourceInfo()->getType()->isBlockPointerType() &&
         !var->hasInit()) {
       Diag(var->getLocation(), diag::err_opencl_invalid_block_declaration)
           << 1 /*Init*/;
       var->setInvalidDecl();

clang/lib/Sema/SemaDeclCXX.cpp

Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
   if (InstPos != UnparsedDefaultArgInstantiations.end()) {
     for (unsigned I = 0, N = InstPos->second.size(); I != N; ++I)
       InstPos->second[I]->setUninstantiatedDefaultArg(Arg);
 
     // We're done tracking this parameter's instantiations.
     UnparsedDefaultArgInstantiations.erase(InstPos);
   }
 
+  // Check for delayed warnings
+  AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param);
+
   return false;
 }
 
 /// ActOnParamDefaultArgument - Check whether the default argument
 /// provided for a function parameter is well-formed. If so, attach it
 /// to the parameter declaration.
 void
 Sema::ActOnParamDefaultArgument(Decl *param, SourceLocation EqualLoc,
Sema::ActOnParamDefaultArgument(Decl *param, SourceLocation EqualLoc,
 
   // Check that the default argument is well-formed
   CheckDefaultArgumentVisitor DefaultArgChecker(DefaultArg, this);
   if (DefaultArgChecker.Visit(DefaultArg)) {
     Param->setInvalidDecl();
     return;
   }
 
+  // Temporarily change the context and add param to it
+  // Allows DiagRuntimeBehavior to register this param with
+  // any possibly warnings
+  // Param gets added back to context when function is added
+  // to context
+  // FIXME: Params should probably be diagnosed after being
+  // actually added to context. Either params get added to
+  // context before the function scope or diagnostics are
+  // delayed until function scope is added
+  DeclContext *Cur = CurContext;
+  CurContext = Param->getDeclContext();
+  CurContext->addDecl(Param);
   SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
+  CurContext->removeDecl(Param);
+  CurContext = Cur;
 }
 
 /// ActOnParamUnparsedDefaultArgument - We've seen a default
 /// argument for a function parameter, but we can't parse it yet
 /// because we're inside a class definition. Note that this default
 /// argument will be parsed later.
 void Sema::ActOnParamUnparsedDefaultArgument(Decl *param,
                                              SourceLocation EqualLoc,

clang/lib/Sema/SemaExpr.cpp

if (auto Init = dyn_cast<ExprWithCleanups>(Param->getInit())) {
 
     // Append all the objects to the cleanup list.  Right now, this
     // should always be a no-op, because blocks in default argument
     // expressions should never be able to capture anything.
     assert(!Init->getNumObjects() &&
            "default argument expression has capturing blocks?");
   }
 
+  AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param);
+
   // We already type-checked the argument, so we know it works.
   // Just mark all of the declarations in this potentially-evaluated expression
   // as being "referenced".
   EnterExpressionEvaluationContext EvalContext(
       *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
   MarkDeclarationsReferencedInExpr(Param->getDefaultArg(),
                                    /*SkipLocalVariables=*/true);
   return false;
case ExpressionEvaluationContext::DiscardedStatement:
     break;
 
   case ExpressionEvaluationContext::ConstantEvaluated:
     // Relevant diagnostics should be produced by constant evaluation.
     break;
 
   case ExpressionEvaluationContext::PotentiallyEvaluated:
   case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
     if (!Stmts.empty() && getCurFunctionOrMethodDecl()) {
       FunctionScopes.back()->PossiblyUnreachableDiags.
-        push_back(sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
+        push_back(clang::sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
       return true;
     }
 
     // The initializer of a constexpr variable or of the first declaration of a
     // static data member is not syntactically a constant evaluated constant,
     // but nonetheless is always required to be a constant expression, so we
     // can skip diagnosing.
     // FIXME: Using the mangling context here is a hack.
+    //
+    // Mangling context seems to only be defined on constexpr vardecl that
+    // displayed errors
+    // This skips warnings that were already emitted as notes on errors.
     if (auto *VD = dyn_cast_or_null<VarDecl>(
             ExprEvalContexts.back().ManglingContextDecl)) {
       if (VD->isConstexpr() ||
           (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
         break;
+    }
+
+    // For any other kind of variable, we should build a CFG for its
+    // initializer and check whether the context in question is reachable.
+    if(auto *VD = dyn_cast_or_null<VarDecl>(
+        CurContext->getLastDecl())) {
+      if(VD->getDefinition()) {
+        VD = VD->getDefinition();
+      }
       // FIXME: For any other kind of variable, we should build a CFG for its
       // initializer and check whether the context in question is reachable.
+      // Some cases aren't picked up by path analysis currently
+      if(!Stmts.empty() && VD->isFileVarDecl()) {
+        AnalysisWarnings.RegisterVarDeclWarning(VD, clang::sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
+        return true;
+      }
     }
 
     Diag(Loc, PD);
     return true;
+
   }
 
   return false;
 }
 
 bool Sema::DiagRuntimeBehavior(SourceLocation Loc, const Stmt *Statement,
                                const PartialDiagnostic &PD) {
   return DiagRuntimeBehavior(
       Loc, Statement ? llvm::makeArrayRef(Statement) : llvm::None, PD);

clang/test/Sema/warn-unreachable-warning-var-decl.cpp

+// RUN: %clang_cc1 -verify %s
+int e = 1 ? 0 : 1/0;
+int g = 1 ? 1/0 : 0;// expected-warning{{division by zero is undefined}}
+
+#define SHIFT(n)        (((n) == 32) ? 0 : ((1<<(n))-1))
+
+int x = SHIFT(32);
+int y = SHIFT(0);
+
+// FIXME: Expressions in lambdas aren't ignored
+int z = [](){
+  return 1 ? 0 : 1/0; // expected-warning{{division by zero is undefined}}
+}();
+
+int f(void)
+{
+    int x = 1 ? 0 : 1/0;
+    return x;
+}
+
+template<typename T>
+struct X0 {
+    static T value;
+};
+
+template<typename T>
+struct X1 {
+    static T value;
+};
+
+template<typename T>
+T X0<T>::value = 3.14; // expected-warning{{implicit conversion from 'double' to 'int' changes value from 3.14 to 3}}
+
+template<typename T>
+T X1<T>::value = 1 ? 0 : 1/0;
+
+template struct X0<int>; // expected-note{{in instantiation of static data member 'X0<int>::value' requested here}}
+
+constexpr signed char c1 = 100 * 2; // ok expected-warning{{changes value}}
+constexpr signed char c2 = '\x64' * '\2'; // also ok  expected-warning{{changes value}}
+constexpr int shr_32 = 0 >> 32; // expected-error {{constant expression}} expected-note {{shift count 32 >= width of type}}