samestep marked 9 inline comments as done. samestep added a comment. Fixed all the minor nits and responded to some comments.
================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:401 + const TypeErasedDataflowAnalysisState &State) { + PostVisitStmt(Stmt.getStmt(), State); + }); ---------------- sgatev wrote: > Let's harmonize the arguments to `transferBlock` and > `runTypeErasedDataflowAnalysis`. I think in both cases we only need access to > the internal `Stmt` so there's no need to pass `CFGStmt` to `transferBlock`. > This way we can pass `PostVisitStmt` directly here without having to wrap it > in a lambda. Makes sense to me; should I do that in a followup? ================ Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:87-89 + std::function<std::function<void( + const Stmt *, const TypeErasedDataflowAnalysisState &)>(ASTContext &)> + MakePostVisitStmt, ---------------- sgatev wrote: > Why is this a function that returns a function? Couldn't it be simply a void > function, like `VerifyResults`? It needs to take in the `ASTContext`, and it needs to be able to close over state; see the one we pass from `UncheckedOptionalAccessModelTest.cpp`. ================ 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( ---------------- ymandel wrote: > any chance that this could be folded into `MakePostVisitStmt`? Looking into this now. ================ Comment at: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:2338 // if (opt1.has_value()) { // opt2.value(); // } ---------------- gribozavr2 wrote: > I don't think this is correct, is it? 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