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

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

Reply via email to