mboehme accepted this revision. mboehme added inline comments.
================ Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:113 + // Don't do anything special for CXXForRangeStmt, because the condition (being + // implicitly generated) isn't visible from the loop body. + ---------------- Making this purely a comment might risk getting it attached to the wrong method. Consider adding an empty `VisitCXXForRangeStmt()`, which would make the "do nothing" behavior explicit in code? ================ Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:1631 + const AnalysisOutputs &) { + EXPECT_THAT(Results.keys(), UnorderedElementsAre("after_loop")); + }); ---------------- Is this check (and the `(void)0` with the `[[after_loop]` annotation) actually needed? IIUC, the condition we want to test is that the analysis converges. The fact that it produces a `Results` entry for `[[after_loop]]` is really more a check of the testing infrastructure rather than the analysis itself? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158848/new/ https://reviews.llvm.org/D158848 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits