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

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 15 16:45:46 PDT 2019

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()))
+    }
Please get rid of the pre-existing hacky use of `ManglingContextDecl` here and move this into the `getDeclForInitializer` block below.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list