[PATCH] D48427: [Analyzer] Iterator Checker Hotfix: Defer deletion of container data until its last iterator is cleaned up

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Yep, that'll do, thanks! Sorry for not keeping up with all the incoming reviews.



================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:536
     if (!SR.isLiveRegion(Cont.first)) {
-      State = State->remove<ContainerMap>(Cont.first);
+      if (!hasLiveIterators(State, Cont.first)) {
+        State = State->remove<ContainerMap>(Cont.first);
----------------
Just let's explain that, because it's something very unobvious about the code. Like, anybody who reads that is allowed to ask "omg why are they doing it??", and it's a good indication that we need a comment. We keep container information around because the iterator's container identity is anyway essential to understanding the iterator, and that causes us to keep a dead container around in the iterator map values, and we might as well keep it in the container map keys to avoid changing how we represent the iterator for no good reason.


https://reviews.llvm.org/D48427