[PATCH] D77229: [Analyzer][NFC] Avoid handling of LazyCompundVals in IteratorModeling

Balogh, Ádám via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 24 23:43:09 PDT 2020


baloghadamsoftware added a comment.

Oh, I think now what do you mean: iterators stored in containers, thus iterators iterating over iterators. Yes, the current working of the checker does not support it because it stores iterator positions for both the prvalues and the glvalues. This is so from the beginning and this patch does not change anything about this behavior. Of course, this design concept is questionable, we can change it in the future. But not in this patch, this one is purely an NFC: it has exactly the same functionality as the previous versions, the only difference is that it does not hamper anymore with `LazyCompoundVal`s but reaches the real region of the objects (and only in case of objects by value, I check the AST type for paramters), exactly as you requested.

Anyway, to me storing iterators in containers sounds dangerous. Iterators are like lighted matches which you use and then extinguish. You do not put them into the drawer on into a paper box. If you store iterators in a container, how can you track which one of them is invalidated? This sounds a very bad practice to me.

Anyway, we can change the whole iterator modeling and checking in the future to support even this bad practice, but in their current state it is much more beneficial for the users to reduce the huge number of false positives and add note tags for the reported bugs, I think. Maybe we could better explain the current working in the comments at the beginning, and put FIXME-s there. Eventually we can also add such test cases, but with the current functionality even that cannot be done, because we currently do not keep track the content of the containers.


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

https://reviews.llvm.org/D77229



More information about the cfe-commits mailing list