[PATCH] D32905: [Analyzer] Iterator Checker - Part 9: Evaluation of std::find-like calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.
Herald added a subscriber: donat.nagy.


================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1756-1760
+  auto stateFound = state->BindExpr(CE, LCtx, RetVal);
+  auto stateNotFound = state->BindExpr(CE, LCtx, SecondParam);
+
+  C.addTransition(stateFound);
+  C.addTransition(stateNotFound);
----------------
Right now i don't think we're ready for finding a good generalized way of describing C++ method summaries. Even the mechanism in the C library checker is veeeery limited. In C++ i'd put a good generalized model for dealing with opaque symbolic structures as an important pre-requisite. But if you have anything specific in mind, i'd be excited to hear what you propose.

Also i still believe that state split is incorrect and leads to inevitable false positives even when all the information that's necessary to prevent these false positives is in fact available to the Analyzer. And for that reason i hesitate to put this logic into a checker that most people will need to enable by default.

On the other hand, because you support `==` and `!=`, these false positives might be suppress-able with `assert`s. So if they're rare, they're probably not that bad. But i'd still put it under a flag.

Actually, yeah, emptiness of the container might be pretty easy to model. It'll let you return `.end()` when `.begin()` is called, etc. You can declare that the container is empty when it's freshly default-constructed or a fresh return value of `.empty()` is assumed to be true. That's not enough to fix `.find()` modeling, just random thoughts.


https://reviews.llvm.org/D32905