[PATCH] D62551: [analyzer][Dominator] Add post dominator tree builder for the CFG + a debug checker

Jakub Kuderski via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 28 15:02:47 PDT 2019


kuhar added inline comments.


================
Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:111
+      assert(
+          (IDom && !IDom->getBlock() && *I == &(*I)->getParent()->getExit() &&
+           IsPostDom) ||
----------------
kuhar wrote:
> Szelethus wrote:
> > kuhar wrote:
> > > Assertions with multiple conditions conjoined are difficult to debug -- perhaps it'd be better to split them and assign to separate booleans for each condition, with descriptive names? This code can be hidden behind an ifdef for debug. 
> > Do you like this better? I prefer running static analysis on large codebases with a Release+Asserts build, and I fear that this check will be lost if I use `#ifdef DEBUG`.
> I think it's much more readable now, thanks! Release+Asserts defines NDEBUG and the assertion will work if you put it into an ifdef.
*does not define NDEBUG


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62551/new/

https://reviews.llvm.org/D62551





More information about the cfe-commits mailing list