[PATCH] D133931: [clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `clang/Analysis/FlowSensitive`.

[weiyi via Phabricator via cfe-commits]

wyt added inline comments.


================
Comment at: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:137-140
+    auto CS = E->getAs<CFGStmt>();
+    if (!CS)
+      return;
+    auto S = CS->getStmt();
----------------
martong wrote:
> This is exactly the same code that you have in `SingleVarConstantPropagationTest.cpp` and in the rest of the files. I think this could be hoisted into a common function declared in `FlowSensitive/DataflowAnalysis.h`.
> 
> By the way, wouldn't it be more convenient for the client to provide a `transferStmt` function if the they are interested only in `Stmt`s? (Defining both `transferStmt` and `transfer` should be a compile time error)
Thanks for the comment!

For now, I decided not to pull this out into a common function. This seems more like a utility function that does not quite belong DataflowAnalysis.h - it would be nice to keep the code there dedicated to analysis logic. Besides, I don't think we save much since the boilerplate is just a few lines.

Also, about having another `transferStmt` function, at this stage I feel that it's best if we keep things compact and have just one way of doing things - the framework is constantly evolving and having multiple implementations of the same concept can be distracting. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133931