sgatev added inline comments.
================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103 + /// code being analysed. + virtual void transferCFGElement(const CFGElement *Element, Lattice &L, + Environment &Env) {} ---------------- Any reason not to call this one `transfer` too? ================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:157 +// Holds data structures required for running dataflow analysis. +struct AnalysisContext { ---------------- Let's start struct and member comments with `///`. ================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:184 /// All predecessors of `Block` except those with loop back edges must have -/// already been transferred. States in `BlockStates` that are set to +/// already been transferred. States in `TC.BlockStates` that are set to /// `llvm::None` represent basic blocks that are not evaluated yet. ---------------- ================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:312 +/// Transfers `State` by evaluating each element in the `Block` based on the +/// `TC.Analysis` specified. +/// ---------------- ================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:314 +/// +/// Built in transfer functions (if the option for `ApplyBuiltinTransfer` is set +/// by the analysis) will be applied to the element before evaluation by the ---------------- ================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:328-340 + switch (Element.getKind()) { + case CFGElement::Statement: { + builtinTransfer(Element.castAs<CFGStmt>(), State, AC); + break; + } + case CFGElement::Initializer: { + builtinTransfer(Element.castAs<CFGInitializer>(), State); ---------------- The level of abstraction for built-in and user code is different in this function which makes it hard to follow. Let's move this in a `builtinTransfer(const CFGElement &, TypeErasedDataflowAnalysisState &)` function that dispatches to one of the other built-in transfer functions. This way `transferCFGBlock` won't mix implementation details with the high level algorithm. ================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:341-347 + } + // User-provided analysis + AC.Analysis.transferTypeErased(&Element, State.Lattice, State.Env); + // Post processing + if (PostVisitCFG) { + PostVisitCFG(Element, State); + } ---------------- ================ Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:164-170 + std::function<void(AnalysisData)> VerifyResults, ArrayRef<std::string> Args, + const tooling::FileContentMappings &VirtualMappedFiles = {}) { + + std::function<void(ASTContext & Context, const CFGElement &, + const TypeErasedDataflowAnalysisState &)> + PostVisitCFG = nullptr; + if (PostVisitStmt) { ---------------- Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131614/new/ https://reviews.llvm.org/D131614 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits