[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103
+  virtual void transferCFGElement(const CFGElement *Element, Lattice &L,
+                                  Environment &Env) {}
+
----------------
Instead of adding virtual function, please continue following the CRTP pattern. Add the expected function declarations in the comment above the class.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:108
     Lattice &L = llvm::any_cast<Lattice &>(E.Value);
-    static_cast<Derived *>(this)->transfer(Stmt, L, Env);
+    transferCFGElement(Element, L, Env);
+
----------------



================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:112
+    if (Element->getKind() == CFGElement::Statement) {
+      transfer(Element->getAs<CFGStmt>()->getStmt(), L, Env);
+    }
----------------
Ditto.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:134
 /// the dataflow analysis cannot be performed successfully. Otherwise, calls
-/// `PostVisitStmt` on each statement with the final analysis results at that
-/// program point.
+/// `PostVisit` on each element with the final analysis results at that program
+/// point.
----------------



================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:144
+                                               typename AnalysisT::Lattice> &)>
+        PostVisit = nullptr) {
+  std::function<void(const CFGElement &,
----------------
"PostVisitCFG"?


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:209
+          if (Element.getKind() == CFGElement::Statement) {
+            PostVisitStmt(*Element.getAs<CFGStmt>(), State);
+          }
----------------
PTAL at the intended usage of getAs() here: llvm-project/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp

That is, it should be used like dyn_cast in if statements.


================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:260
 
-/// Transfers `State` by evaluating `CfgStmt` in the context of `Analysis`.
-/// `HandleTransferredStmt` (if provided) will be applied to `CfgStmt`, after it
-/// is evaluated.
-static void transferCFGStmt(
-    const ControlFlowContext &CFCtx,
-    llvm::ArrayRef<llvm::Optional<TypeErasedDataflowAnalysisState>> BlockStates,
-    const CFGStmt &CfgStmt, TypeErasedDataflowAnalysis &Analysis,
-    TypeErasedDataflowAnalysisState &State,
-    std::function<void(const CFGStmt &,
-                       const TypeErasedDataflowAnalysisState &)>
-        HandleTransferredStmt) {
+/// Built-in transfer function for `CfgStmt`.
+void builtInTransfer(const CFGStmt &CfgStmt,
----------------



================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:261
+/// Built-in transfer function for `CfgStmt`.
+void builtInTransfer(const CFGStmt &CfgStmt,
+                     TypeErasedDataflowAnalysisState &InputState,
----------------
"CfgStmt" is violating the style guide casing rules.


================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:270-271
 
-/// Transfers `State` by evaluating `CfgInit`.
-static void transferCFGInitializer(const CFGInitializer &CfgInit,
-                                   TypeErasedDataflowAnalysisState &State) {
-  const auto &ThisLoc = *cast<AggregateStorageLocation>(
-      State.Env.getThisPointeeStorageLocation());
+/// Built-in transfer function for `CfgInit`.
+void builtInTransfer(const CFGInitializer &CfgInit,
+                     TypeErasedDataflowAnalysisState &InputState) {
----------------
Ditto.


================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:325
+      case CFGElement::Statement: {
+        builtInTransfer(*Element.getAs<CFGStmt>(), State, TC);
+        break;
----------------
Should be using castAs().


================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329
+      case CFGElement::Initializer: {
+        builtInTransfer(*Element.getAs<CFGInitializer>(), State);
+        break;
----------------
castAs()


================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:438
 } // namespace dataflow
-} // namespace clang
+} // namespace clang
----------------
Please add the newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614