martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
LGTM!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:221
+ if (Handler) {
+ if (!C.wasInlined) {
+ if (Call.getNumArgs() > 1) {
----------------
baloghadamsoftware wrote:
> martong wrote:
> > Perhaps putting this hunk into a separate function or lambda could decrease
> > the nesting level, b/c you could have an early return if there is no
> > `Handler`.
> Early return is not possible because the execution must continue if the is no
> handler. However, I refactored `checkPostCall` now, the handling of both
> overloaded operators and advance-like functions are moved to separate
> functions.
Thanks, that's much better structured this way.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:559
+bool IteratorModeling::noChangeInPosition(CheckerContext &C, SVal Iter,
+ const Expr *CE) const {
----------------
baloghadamsoftware wrote:
> martong wrote:
> > Some comments would be helpful here. Also, should we use this function only
> > with `advance()` or it could be useful perhaps in other context?
> I renamed the function and added a short comment. Is it OK?
Yeah, looks ok.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:571
+ const ExplodedNode *N = C.getPredecessor();
+ while (N) {
+ ProgramPoint PP = N->getLocation();
----------------
baloghadamsoftware wrote:
> martong wrote:
> > I have a rough presumption that this hunk is a general pattern to get the
> > previous node for the previous iterator position. So perhaps it would be
> > useful as a standalone free/static member function too?
> I moved this to a standalone function and named it accordingly.
Thanks, it's easier to read the code this way.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76361/new/
https://reviews.llvm.org/D76361
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
