[PATCH] D97874: [analyzer] Improve SVal cast from integer to bool using known RangeSet

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Great, thanks!

I still have concerns about these functions - `isNull`, `isNonNull` etc. They do indeed provide a quick test for nullness that doesn't involve constructing a new program state. This test, however, will never be as //precise// as constructing a new state. It used to be when `RangeConstraintManager` boiled down to simple range operations but now that it grew much larger (eg., the newly added support for symbolic == and != that involves tracking equivalence classes), there's really no way to tell whether a state is going to be feasible without running the whole machine and observing its emergent behavior, which is what `assume()` does. On the other hand i still don't see any indication that `assume()` is noticeably expensive. So i'm really worried that this is a premature optimization that sacrifices correctness for nothing. So i'm in favor of phasing out these functions entirely and converting all code to always use `assume()`. This may occasionally involve untangling unwanted recursions but i hope that all recursive-ish operations can be isolated within the constraint manager itself (and possibly in checker's `evalAssume()` which we can hopefully guard against with runtime assertions).


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

https://reviews.llvm.org/D97874