[clang][dataflow][NFC] Share code between Environment ctor and pushCallInternal().

[mboehme]

Summary

The deduplicated code is moved into initVars().

As an added bonus, pushCallInternal() now also gets the "Add all fields
mentioned in default member initializers" behavior, which apparently had been
added to the Environment ctor but not pushCallInternal().

[xazax.hun]

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

465

I wonder if we should rename this to something like initFieldAndGlobals.

[ymandel]

Thank you!

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

465

+1

[mboehme]

Changes in response to review comments

[mboehme]

Rename function again to be consistent with getFieldsAndGlobalVars().

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

465

Done.

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

private:
   const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
 
   /// Shared implementation of `pushCall` overloads. Note that unlike
   /// `pushCall`, this member is invoked on the environment of the callee, not
   /// of the caller.
   void pushCallInternal(const FunctionDecl *FuncDecl,
                         ArrayRef<const Expr *> Args);
 
-  /// Assigns storage locations and values to all variables in `Vars`.
-  void initVars(llvm::DenseSet<const VarDecl *> Vars);
+  /// Assigns storage locations and values to all global variables and fields
+  /// referenced in `FuncDecl`. `FuncDecl` must have a body.
+  void initFieldsAndGlobals(const FunctionDecl *FuncDecl);
 
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
 
   // FIXME: move the fields `CallStack`, `ReturnLoc` and `ThisPointeeLoc` into a
   // separate call-context object, shared between environments in the same call.
   // https://github.com/llvm/llvm-project/issues/59005

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (auto *DS = dyn_cast<DeclStmt>(&S)) {
     insertIfGlobal(*VD, Fields, Vars);
     if (const auto *FD = dyn_cast<FieldDecl>(VD))
       Fields.insert(FD);
   }
 }
 
 // FIXME: Add support for resetting globals after function calls to enable
 // the implementation of sound analyses.
-void Environment::initVars(llvm::DenseSet<const VarDecl *> Vars) {
+void Environment::initFieldsAndGlobals(const FunctionDecl *FuncDecl) {
+  assert(FuncDecl->getBody() != nullptr);
+
+  llvm::DenseSet<const FieldDecl *> Fields;
+  llvm::DenseSet<const VarDecl *> Vars;
+
+  // Look for global variable and field references in the
+  // constructor-initializers.
+  if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FuncDecl)) {
+    for (const auto *Init : CtorDecl->inits()) {
+      if (const auto *M = Init->getAnyMember())
+          Fields.insert(M);
+      const Expr *E = Init->getInit();
+      assert(E != nullptr);
+      getFieldsAndGlobalVars(*E, Fields, Vars);
+    }
+    // Add all fields mentioned in default member initializers.
+    for (const FieldDecl *F : CtorDecl->getParent()->fields())
+      if (const auto *I = F->getInClassInitializer())
+          getFieldsAndGlobalVars(*I, Fields, Vars);
+  }
+  getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
+
+  // These have to be added before the lines that follow to ensure that
+  // `create*` work correctly for structs.
+  DACtx->addModeledFields(Fields);
+
   for (const VarDecl *D : Vars) {
     if (getStorageLocation(*D, SkipPast::None) != nullptr)
       continue;
     auto &Loc = createStorageLocation(*D);
     setStorageLocation(*D, Loc);
     if (auto *Val = createValue(D->getType()))
       setValue(Loc, *Val);
   }

 Environment::Environment(DataflowAnalysisContext &DACtx,
                          const DeclContext &DeclCtx)
     : Environment(DACtx) {
   CallStack.push_back(&DeclCtx);
 
   if (const auto *FuncDecl = dyn_cast<FunctionDecl>(&DeclCtx)) {
     assert(FuncDecl->getBody() != nullptr);
 
-    llvm::DenseSet<const FieldDecl *> Fields;
-    llvm::DenseSet<const VarDecl *> Vars;
-
-    // Look for global variable and field references in the
-    // constructor-initializers.
-    if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(&DeclCtx)) {
-      for (const auto *Init : CtorDecl->inits()) {
-        if (const auto *M = Init->getAnyMember())
-          Fields.insert(M);
-        const Expr *E = Init->getInit();
-        assert(E != nullptr);
-        getFieldsAndGlobalVars(*E, Fields, Vars);
-      }
-      // Add all fields mentioned in default member initializers.
-      for (const FieldDecl *F  : CtorDecl->getParent()->fields())
-        if (const auto *I = F->getInClassInitializer())
-          getFieldsAndGlobalVars(*I, Fields, Vars);
-    }
-    getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
-
-    // These have to be added before the lines that follow to ensure that
-    // `create*` work correctly for structs.
-    DACtx.addModeledFields(Fields);
-
-    initVars(Vars);
+    initFieldsAndGlobals(FuncDecl);
 
     for (const auto *ParamDecl : FuncDecl->parameters()) {
       assert(ParamDecl != nullptr);
       auto &ParamLoc = createStorageLocation(*ParamDecl);
       setStorageLocation(*ParamDecl, ParamLoc);
       if (Value *ParamVal = createValue(ParamDecl->getType()))
         setValue(ParamLoc, *ParamVal);
     }
Environment Environment::pushCall(const CXXConstructExpr *Call) const {
 
   return Env;
 }
 
 void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
                                    ArrayRef<const Expr *> Args) {
   CallStack.push_back(FuncDecl);
 
-  // FIXME: Share this code with the constructor, rather than duplicating it.
-  llvm::DenseSet<const FieldDecl *> Fields;
-  llvm::DenseSet<const VarDecl *> Vars;
-  // Look for global variable references in the constructor-initializers.
-  if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FuncDecl)) {
-    for (const auto *Init : CtorDecl->inits()) {
-      if (const auto *M = Init->getAnyMember())
-        Fields.insert(M);
-      const Expr *E = Init->getInit();
-      assert(E != nullptr);
-      getFieldsAndGlobalVars(*E, Fields, Vars);
-    }
-  }
-  getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
-
-  // These have to be added before the lines that follow to ensure that
-  // `create*` work correctly for structs.
-  DACtx->addModeledFields(Fields);
-
-  initVars(Vars);
+  initFieldsAndGlobals(FuncDecl);
 
   const auto *ParamIt = FuncDecl->param_begin();
 
   // FIXME: Parameters don't always map to arguments 1:1; examples include
   // overloaded operators implemented as member functions, and parameter packs.
   for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) {
     assert(ParamIt != FuncDecl->param_end());