[PATCH] D62525: [Analyzer] Add new visitor to the iterator checkers

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 2 inline comments as done.
baloghadamsoftware added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:281-282
+
+  // `FoundChange` becomes true when we find the statement the results in the
+  // current state of the iterator.
+  // `FoundEmptyness` becomes true when we find the block edge assuming
----------------
Szelethus wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > I don't think we should stop here. I think it's worth it to highlight *all* increments and decrements of the interesting iterator, so that it was clear how come that it has the given value.
> > > 
> > > Additionally, because iterators are copied around, it makes sense to "recursively" apply the visitor to the original object when the iterator is obtained as a copy (similarly to how `trackExpressionValue` works).
> > This can be done but an iterator can reach the past-the-end or the first position by changing the container's size as well, not only by incrementing or decrementing the iterator itself.
> Maybe in a followup patch, but I'd love to see a `trackIterator` function!
Hey, but it is done now, in the updated patch! See lines 1782-1803.


================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1495-1496
 
   // For deque-like containers invalidate all iterator positions. For
   // vector-like containers invalidate iterator positions after the insertion.
   const auto *Cont = Pos->getContainer();
----------------
Szelethus wrote:
> >>! In D62525#1523026, @baloghadamsoftware wrote:
> > For example, an insertion happens into a vector that invalidates all the iterators at and after the position where it is inserted.
> 
> Is this actually correct?
> 
> https://en.cppreference.com/w/cpp/container/deque
> > std::deque (double-ended queue) is an indexed sequence container that allows fast insertion and deletion at both its beginning and its end. In addition, insertion and deletion at either end of a deque never invalidates pointers or references to the rest of the elements.
> 
> https://en.cppreference.com/w/cpp/container/vector
> 
> > `insert`, `emplace`, `resize`: If the vector changed capacity, all of the iterators are invalidated. If not, only those after the insertion point.
[[ https://en.cppreference.com/w/cpp/container/deque/insert | https://en.cppreference.com/w/cpp/container/deque/insert ]]

//All iterators, including the past-the-end iterator, are invalidated.//

I think I speak here about `insert()`, not about `push_back()` and `push_front()`.

Since we use a conservative approach we always assume that vectors and double-end queues do not change capacities.


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

https://reviews.llvm.org/D62525