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

117

Fix indentation.

clang/lib/Sema/AnalysisBasedWarnings.cpp

2002

Should PossiblyUnreachableDiags be const?

Also, fix indentation.

2010

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

2026

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

2046

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

104

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

The formal parameter should be abbreviated, maybe PUD?

116

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

2007

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.

2257

PUD

2272

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

16679

Punctuation.

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

40–41

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

95

Methods should be UpperCamelCased.

112

UpperCamelCase

clang/lib/Sema/SemaExpr.cpp

16672

does this need the clang:: qualifier?

16701

does this need clang::?

[Nathan-Huckleberry]

• Stylistic fixes of function names and removal of namespace prefixes

[rsmith]

clang/lib/Sema/SemaDeclCXX.cpp

340

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

16694

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

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

[nickdesaulniers]

clang/include/clang/Sema/Sema.h

1698

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

clang/lib/Sema/AnalysisBasedWarnings.cpp

2036

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

2009

is clang namespace required here?

[Nathan-Huckleberry]

clang/lib/Sema/SemaDeclCXX.cpp

340

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

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

2015

Please capitalize the names of variables.

clang/lib/Sema/SemaExpr.cpp

16678–16695

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

These new warnings seem valid.

[rsmith]

clang/include/clang/Parse/Parser.h

1873

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

1880

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

clang/lib/Sema/AnalysisBasedWarnings.cpp

2263

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?)

16670

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]

16680–16682

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?

16690

What does this fixme mean?

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

9

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

clang/include/clang/Parse/Parser.h

private:
 
   //===--------------------------------------------------------------------===//
   // C99 6.7.8: Initialization.
 
   /// ParseInitializer
   ///       initializer: [C99 6.7.8]
   ///         assignment-expression
   ///         '{' ...
-  ExprResult ParseInitializer() {
-    if (Tok.isNot(tok::l_brace))
-      return ParseAssignmentExpression();
-    return ParseBraceInitializer();
+  ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr) {
+    Actions.ExprEvalContexts.back().DeclForInitializer = DeclForInitializer;
+    ExprResult init;
+    if (Tok.isNot(tok::l_brace)) {
+      init = ParseAssignmentExpression();
+    } else {
+      init = ParseBraceInitializer();
+    }
+    Actions.ExprEvalContexts.back().DeclForInitializer = nullptr;
+    return init;
   }
+
   bool MayBeDesignationStart();
   ExprResult ParseBraceInitializer();
   ExprResult ParseInitializerWithPotentialDesignator();
 
   //===--------------------------------------------------------------------===//
   // clang Expressions
 
   ExprResult ParseBlockLiteralExpression();  // ^{...}

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;
 
   /// @}
 
+  void FlushDiagnostics(SmallVector<clang::sema::PossiblyUnreachableDiag, 4>);
+
 public:
   AnalysisBasedWarnings(Sema &s);
 
   void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
                      const Decl *D, QualType BlockType);
 
+  void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD);
+
+  void IssueWarningsForRegisteredVarDecl(VarDecl *VD);
+
   Policy getDefaultPolicy() { return DefaultPolicy; }
 
   void PrintStats() const;
+
+  void
+  EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC,
+                               SmallVector<PossiblyUnreachableDiag, 4> PUDs);
 };
 
 }} // end namespace clang::sema
 
 #endif

clang/include/clang/Sema/Sema.h

struct ExpressionEvaluationContextRecord {
     /// is indeed an unevaluated context.
     SmallVector<LambdaExpr *, 2> Lambdas;
 
     /// The declaration that provides context for lambda expressions
     /// and block literals if the normal declaration context does not
     /// suffice, e.g., in a default function argument.
     Decl *ManglingContextDecl;
 
+    /// Declaration for initializer if one is currently being
+    /// parsed. Used when an expression has a possibly unreachable
+    /// diagnostic to reference the declaration as a whole.
+    Decl *DeclForInitializer = nullptr;
+
     /// The context information used to mangle lambda expressions
     /// and block literals within this context.
     ///
     /// This mangling information is allocated lazily, since most contexts
     /// do not have lambda expressions or block literals.
     std::unique_ptr<MangleNumberingContext> MangleNumbering;
 
     /// If we are processing a decltype type, a set of call expressions
private:
   /// Get the module whose scope we are currently within.
   Module *getCurrentModule() const {
     return ModuleScopes.empty() ? nullptr : ModuleScopes.back().Module;
   }
 
   VisibleModuleSet VisibleModules;
 
 public:
   /// Get the module owning an entity.
   Module *getOwningModule(Decl *Entity) { return Entity->getOwningModule(); }
 
   /// Make a merged definition of an existing hidden definition \p ND
   /// visible at the specified location.
   void makeMergedDefinitionVisible(NamedDecl *ND);
 
   bool isModuleVisible(const Module *M, bool ModulePrivate = false);
 
   /// Determine whether a declaration is visible to name lookup.
   bool isVisible(const NamedDecl *D) {
     return !D->isHidden() || isVisibleSlow(D);
   }
 
   /// Determine whether any declaration of an entity is visible.
   bool
   hasVisibleDeclaration(const NamedDecl *D,
                         llvm::SmallVectorImpl<Module *> *Modules = nullptr) {
     return isVisible(D) || hasVisibleDeclarationSlow(D, Modules);
   }
   bool hasVisibleDeclarationSlow(const NamedDecl *D,
                                  llvm::SmallVectorImpl<Module *> *Modules);
 

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/Parse/ParseDecl.cpp

if (Tok.is(tok::kw_delete)) {
       if (Tok.is(tok::code_completion)) {
         Actions.CodeCompleteInitializer(getCurScope(), ThisDecl);
         Actions.FinalizeDeclaration(ThisDecl);
         cutOffParsing();
         return nullptr;
       }
 
       PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
-      ExprResult Init = ParseInitializer();
+      ExprResult Init = ParseInitializer(ThisDecl);
 
       // If this is the only decl in (possibly) range based for statement,
       // our best guess is that the user meant ':' instead of '='.
       if (Tok.is(tok::r_paren) && FRI && D.isFirstDeclarator()) {
         Diag(EqualLoc, diag::err_single_decl_assign_in_for_range)
             << FixItHint::CreateReplacement(EqualLoc, ":");
         // We are trying to stop parser from looking for ';' in this for
         // statement, therefore preventing spurious errors to be issued.

clang/lib/Parse/ParseDeclCXX.cpp

if (Tok.is(tok::kw_delete)) {
         Diag(ConsumeToken(), diag::err_default_special_members);
       return ExprError();
     }
   }
   if (const auto *PD = dyn_cast_or_null<MSPropertyDecl>(D)) {
     Diag(Tok, diag::err_ms_property_initializer) << PD;
     return ExprError();
   }
-  return ParseInitializer();
+  return ParseInitializer(D);
 }
 
 void Parser::SkipCXXMemberSpecification(SourceLocation RecordLoc,
                                         SourceLocation AttrFixitLoc,
                                         unsigned TagType, Decl *TagDecl) {
   // Skip the optional 'final' keyword.
   if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
     assert(isCXX11FinalKeyword() && "not a class definition");

clang/lib/Parse/ParseOpenMP.cpp

if (isTokenEqualOrEqualTypo()) {
 
     if (Tok.is(tok::code_completion)) {
       Actions.CodeCompleteInitializer(getCurScope(), OmpPrivParm);
       Actions.FinalizeDeclaration(OmpPrivParm);
       cutOffParsing();
       return;
     }
 
-    ExprResult Init(ParseInitializer());
+    ExprResult Init(ParseInitializer(OmpPrivParm));
 
     if (Init.isInvalid()) {
       SkipUntil(tok::r_paren, tok::annot_pragma_openmp_end, StopBeforeMatch);
       Actions.ActOnInitializerError(OmpPrivParm);
     } else {
       Actions.AddInitializerToDecl(OmpPrivParm, Init.get(),
                                    /*DirectInit=*/false);
     }

clang/lib/Sema/AnalysisBasedWarnings.cpp

 } // namespace consumed
 } // namespace clang
 
 //===----------------------------------------------------------------------===//
 // AnalysisBasedWarnings - Worker object used by Sema to execute analysis-based
 //  warnings on a function, method, or block.
 //===----------------------------------------------------------------------===//
 
-clang::sema::AnalysisBasedWarnings::Policy::Policy() {
+sema::AnalysisBasedWarnings::Policy::Policy() {
   enableCheckFallThrough = 1;
   enableCheckUnreachable = 0;
   enableThreadSafetyAnalysis = 0;
   enableConsumedAnalysis = 0;
 }
 
 static unsigned isEnabled(DiagnosticsEngine &D, unsigned diag) {
   return (unsigned)!D.isIgnored(diag, SourceLocation());
 }
 
-clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
-  : S(s),
-    NumFunctionsAnalyzed(0),
-    NumFunctionsWithBadCFGs(0),
-    NumCFGBlocks(0),
-    MaxCFGBlocksPerFunction(0),
-    NumUninitAnalysisFunctions(0),
-    NumUninitAnalysisVariables(0),
-    MaxUninitAnalysisVariablesPerFunction(0),
-    NumUninitAnalysisBlockVisits(0),
+sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
+    : S(s), NumFunctionsAnalyzed(0), NumFunctionsWithBadCFGs(0),
+      NumCFGBlocks(0), MaxCFGBlocksPerFunction(0),
+      NumUninitAnalysisFunctions(0), NumUninitAnalysisVariables(0),
+      MaxUninitAnalysisVariablesPerFunction(0), NumUninitAnalysisBlockVisits(0),
       MaxUninitAnalysisBlockVisitsPerFunction(0) {
 
   using namespace diag;
   DiagnosticsEngine &D = S.getDiagnostics();
 
   DefaultPolicy.enableCheckUnreachable =
     isEnabled(D, warn_unreachable) ||
     isEnabled(D, warn_unreachable_break) ||
     isEnabled(D, warn_unreachable_return) ||
     isEnabled(D, warn_unreachable_loop_increment);
 
   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)
+void sema::AnalysisBasedWarnings::FlushDiagnostics(
+    const SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) {
+  for (const auto &D : PUDs)
     S.Diag(D.Loc, D.PD);
 }
 
-void clang::sema::
-AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
-                                     sema::FunctionScopeInfo *fscope,
-                                     const Decl *D, QualType BlockType) {
+void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags(
+    AnalysisDeclContext &AC,
+    SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) {
+
+  if (PUDs.empty()) {
+    return;
+  }
+
+  bool Analyzed = false;
+
+  // Register the expressions with the CFGBuilder.
+  for (const auto &D : PUDs) {
+    for (const Stmt *S : D.Stmts)
+      AC.registerForcedBlockExpression(S);
+  }
+
+  if (AC.getCFG()) {
+    Analyzed = true;
+    for (const auto &D : PUDs) {
+      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(PUDs);
+}
+
+void sema::AnalysisBasedWarnings::IssueWarnings(
+    sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope,
 
   // 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(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);
void sema::AnalysisBasedWarnings::IssueWarnings(
   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(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::PrintStats() const {
+void sema::AnalysisBasedWarnings::RegisterVarDeclWarning(
+    VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) {
+  VarDeclPossiblyUnreachableDiags[VD].emplace_back(PUD);
+}
+
+void 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;
+
+  EmitPossiblyUnreachableDiags(AC, VarDeclPossiblyUnreachableDiags[VD]);
+}
+
+void 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"

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;
   }
 
   SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
 }
 
 /// 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,
                                              SourceLocation ArgLoc) {
   if (!param)
     return;
 
   ParmVarDecl *Param = cast<ParmVarDecl>(param);
   Param->setUnparsedDefaultArg();
   UnparsedDefaultArgLocs[Param] = ArgLoc;
 }

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));
+      FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
+          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.
+    // 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>(
-            ExprEvalContexts.back().ManglingContextDecl)) {
+            ExprEvalContexts.back().DeclForInitializer)) {
+      if (VD->getDefinition()) {
+        VD = VD->getDefinition();
+      }
+      // 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.
       if (VD->isConstexpr() ||
           (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
         break;
-      // FIXME: For any other kind of variable, we should build a CFG for its
-      // initializer and check whether the context in question is reachable.
+      // FIXME: Some cases aren't picked up by path analysis currently
+      if (!Stmts.empty() && VD->isFileVarDecl()) {
+        AnalysisWarnings.RegisterVarDeclWarning(
+            VD, 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;       // expected-warning{{changes value}}
+constexpr signed char c2 = '\x64' * '\2'; // expected-warning{{changes value}}
+constexpr int shr_32 = 0 >> 32;           // expected-error {{constant expression}} expected-note {{shift count 32 >= width of type}}

clang/test/SemaCXX/constant-expression-cxx11.cpp

 
 constexpr thread_local B b3 { { 1 }, { 2 } }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}}
 void foo() {
   constexpr static B b1 { { 1 }, { 2 } }; // ok
   constexpr thread_local B b2 { { 1 }, { 2 } }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}}
   constexpr B b3 { { 1 }, { 2 } }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}}
 }
 
-constexpr B &&b4 = ((1, 2), 3, 4, B { {10}, {{20}} });
+constexpr B &&b4 = ((1, 2), 3, 4, B{{10}, {{20}}}); //expected-warning {{expression result unused}}
 static_assert(&b4 != &b2, "");
 
 // Proposed DR: copy-elision doesn't trigger lifetime extension.
 constexpr B b5 = B{ {0}, {0} }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}}
 
 namespace NestedNonStatic {
   // Proposed DR: for a reference constant expression to refer to a static
   // storage duration temporary, that temporary must itself be initialized

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

 
 enum my_byte : unsigned char {};
 
 struct pad {
   char a;
   int b;
 };
 
-constexpr int ok_byte = (__builtin_bit_cast(std::byte[8], pad{1, 2}), 0);
-constexpr int ok_uchar = (__builtin_bit_cast(unsigned char[8], pad{1, 2}), 0);
+constexpr int ok_byte = (__builtin_bit_cast(std::byte[8], pad{1, 2}), 0);      // expected-warning {{expression result unused}}
+constexpr int ok_uchar = (__builtin_bit_cast(unsigned char[8], pad{1, 2}), 0); // expected-warning {{expression result unused}}
 
 #ifdef __CHAR_UNSIGNED__
 // expected-note@+5 {{indeterminate value can only initialize an object of type 'unsigned char', 'char', or 'std::byte'; 'my_byte' is invalid}}}}
 #else
 // expected-note@+3 {{indeterminate value can only initialize an object of type 'unsigned char' or 'std::byte'; 'my_byte' is invalid}}
 #endif
 // expected-error@+1 {{constexpr variable 'bad_my_byte' must be initialized by a constant expression}}
-constexpr int bad_my_byte = (__builtin_bit_cast(my_byte[8], pad{1, 2}), 0);
+constexpr int bad_my_byte = (__builtin_bit_cast(my_byte[8], pad{1, 2}), 0); // expected-warning {{expression result unused}}
 #ifndef __CHAR_UNSIGNED__
 // expected-error@+3 {{constexpr variable 'bad_char' must be initialized by a constant expression}}
 // expected-note@+2 {{indeterminate value can only initialize an object of type 'unsigned char' or 'std::byte'; 'char' is invalid}}
 #endif
-constexpr int bad_char =  (__builtin_bit_cast(char[8], pad{1, 2}), 0);
+constexpr int bad_char = (__builtin_bit_cast(char[8], pad{1, 2}), 0); // expected-warning {{expression result unused}}
 
 struct pad_buffer { unsigned char data[sizeof(pad)]; };
 constexpr bool test_pad_buffer() {
   pad x = {1, 2};
   pad_buffer y = __builtin_bit_cast(pad_buffer, x);
   pad z = __builtin_bit_cast(pad, y);
   return x.a == z.a && x.b == z.b;
 }
 static_assert(test_pad_buffer());

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

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 template<typename T, T Divisor>
 class X {
 public:
   static const T value = 10 / Divisor; // expected-error{{in-class initializer for static data member is not a constant expression}}
+  //expected-warning@-1 {{division by zero is undefined}}
 };
 
 int array1[X<int, 2>::value == 5? 1 : -1];
 X<int, 0> xi0; // expected-note{{in instantiation of template class 'X<int, 0>' requested here}}
 
 
 template<typename T>
 class Y {