[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

[Jakub Kuderski via Phabricator via cfe-commits]

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