[PATCH] D130726: [clang][dataflow] Handle multiple context-sensitive calls to the same function

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Looks great!



================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:227
+
+    const VarDecl *Param = *ParamIt;
+    auto &Loc = Env.createStorageLocation(*Param);
----------------
maybe mention in the patch description that it also improves the modeling of parameter initialization?


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:236
+      Env.setValue(Loc, *ArgVal);
+    }
   }
----------------
Add an `else` that calls `createValue` on the decl's type, and then sets it, like `VisitDeclStmt`?


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:246
+  // analyze the same callee in a different context, and `setStorageLocation`
+  // requires there to not already be a storage location assigned.
+  this->LocToVal = std::move(CalleeEnv.LocToVal);
----------------
Maybe add something like "Conceptually, these maps capture information from the local scope, so when popping that scope, we do not propagate the maps".


================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3963
 
+TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
+  std::string Code = R"(
----------------
Are there more scenarios testable at this point? e.g
1. 2 layers of callees
2. more than one line of code inside the body?
3. one than one CFG block in the body?

If so, please add tests for those that are supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130726