[PATCH] D63889: Check possible warnings on global initializers for reachability

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/include/clang/Sema/Sema.h:1684-1686
+  void pushDeclForInitializer(Decl *D) { DeclForInitializer.push_back(D); }
+
+  void popDeclForInitializer() { DeclForInitializer.pop_back(); }
----------------
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.)


================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2015
+
+  bool analyzed = false;
+
----------------
Please capitalize the names of variables.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:16685-16690
     if (auto *VD = dyn_cast_or_null<VarDecl>(
             ExprEvalContexts.back().ManglingContextDecl)) {
       if (VD->isConstexpr() ||
           (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
         break;
+    }
----------------
Please get rid of the pre-existing hacky use of `ManglingContextDecl` here and move this into the `getDeclForInitializer` block below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63889/new/

https://reviews.llvm.org/D63889