[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 19 11:15:11 PDT 2017


NoQ added a comment.

I'm sorry, i'd try to get back to this and unblock your progress as soon as possible.

One thing i notice is that you manipulate symbolic expressions manually, however many of the things that you need, eg stuff in your `compose()` method, seem to be already available in `SValBuilder::evalBinOp()`. I think you could simplify the code significantly if you rely on it.

One of the downsides of `SValBuilder::evalBinOp()` is that it sometimes simplifies some operations to `UnknownVal` when it knows that the rest of the analyzer would be unable to handle the results anyway. I'm not sure if your approach is more clever than the rest of the analyzer when it comes to handling such values. However, in any case, we're thinking about lifting these limitations in https://reviews.llvm.org/D28953, so it might be a good idea to wait for that to land as well.



================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:51
+// past-end iterator for all containers whenever their `.begin()` and `.end()`
+// are called. Since the Constraint Manager cannot handle SVals we need to take
+// over its role. We post-check equality and non-equality comparisons and
----------------
Just noticed: Constraint Manager can handle these SVals sometimes, suggesting `such SVals`.


================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:125
 private:
-  SymbolRef End;
+  SymbolRef Begin, End;
 
----------------
Just noticed: can we mark these as `const`, because there are no methods to modify them? I guess you intended to keep objects of this class immutable.


https://reviews.llvm.org/D32642





More information about the cfe-commits mailing list