ymandel added inline comments.
================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:35 +using NoopDiags = std::tuple<>; + ---------------- I'm not sure that this is valid style. It's essentially a unit type, which I quite like, but I've never seen this done before. ================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:64 +/// +/// `DiagsT` must have a default constructor. +template <typename Derived, typename LatticeT, typename DiagsT = NoopDiags> ---------------- Please describe this parameter. ================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103 + virtual void check(const Stmt *Stmt, Diags &E, const Lattice &L, + const Environment &Env) {} ---------------- The virtual function seems out of place here, given that everything else is statically resolved. Either drop `virtual` or move to the dyn. typed super class. That said, I'm not clear on why this isn't a free function. I imagine a model of creating the analysis, running it and that results in blocks annotated with `Environment`s and custome lattices. Then, you run any function you want over that using some visitor template helper. To account for state, we either make it an explicit parameter or take a `std::function`. ================ Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:52 + /// Current set of diagnostics. + DiagsT &Diags; + const Environment &Env; ---------------- this implies that we want to support some form of accumulation. I'd argue that any accumulation should already occur in the lattice (or the environment) and this should be a straight read of the state and some output. So, the `TransferState` struct should be enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits