kuhar added a comment. The non-static-analyzer bits look good to me, I added a few nits.
================ Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:230 + + virtual void releaseMemory() { + PostDomTree.releaseMemory(); ---------------- If the virtual function is introduced by ManagesAnalysis, isn't it better to use override here? ================ Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:235 + // Lazily retrieves the set of control dependencies to \p A. + const CFGBlockVector &getControlDependencies(CFGBlock *A) { + auto It = ControlDepenencyMap.find(A); ---------------- Can it be const? ================ Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:252 + /// Whether \p A is control dependent on \p B. + bool isControlDependent(CFGBlock *A, CFGBlock *B) { + return llvm::is_contained(getControlDependencies(A), B); ---------------- Can it be const? ================ Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:257 + // Dumps immediate control dependencies for each block. + void dump() { + CFG *cfg = PostDomTree.getCFG(); ---------------- Can `dump` be const? ================ Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:257 + // Dumps immediate control dependencies for each block. + void dump() { + CFG *cfg = PostDomTree.getCFG(); ---------------- kuhar wrote: > Can `dump` be const? In LLVM, `dump`s are usually annotated with the `LLVM_DUMP_METHOD` attribute and not compiled in release builds. Is the convention different in the static analyzer? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62619/new/ https://reviews.llvm.org/D62619 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits