[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h:126
+    std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
+        PostVisitStmt = nullptr);
 
----------------
Please update comment to mention this new param (and that its optional)


================
Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:65
 
-// Runs dataflow on the body of the function that matches `func_matcher` in code
-// snippet `code`. Requires: `Analysis` contains a type `Lattice`.
+struct AnalysisResults {
+  AnalysisResults(
----------------
nit: since this includes both inputs and results, maybe `AnalysisData`?


================
Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:123
   auto Analysis = MakeAnalysis(Context, Env);
+  auto PostVisitStmt = MakePostVisitStmt ? MakePostVisitStmt(Context) : nullptr;
 
----------------
nit: i prefer explicit comparisons, so `MakePostVisitStmt != nullptr`.


================
Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:161
+  return checkDataflow(
+      Code, std::move(FuncMatcher), std::move(MakeAnalysis), nullptr,
+      [&Expectations](AnalysisResults AnalysisResults) {
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:172-191
+        std::vector<std::pair<std::string, StateT>> Results;
+        for (const CFGBlock *Block : AnalysisResults.CFCtx.getCFG()) {
+          // Skip blocks that were not evaluated.
+          if (!AnalysisResults.BlockStates[Block->getBlockID()])
+            continue;
+
+          transferBlock(
----------------
any chance that this could be folded into `MakePostVisitStmt`?


================
Comment at: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1248
+          UncheckedOptionalAccessDiagnosis Diagnosis(Ctx, Options);
+          return [&Diagnostics, Diagnosis = std::move(Diagnosis)](
+                     const Stmt *Stmt,
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127898