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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to