[PATCH] D77229: [Analyzer][NFC] Avoid handling of LazyCompundVals in IteratorModeling
baloghadamsoftware added a comment. Now I completely know what the source of our misunderstanding is. You thought that this patch will **fix** an issue, namely that we store iterator positions for both the iterator values and the locations of the iterator variables. While this is definitely a wrong approach, it could not be fixed until we got rid from the hack with `LazyCompoundVal`s. These functions **keep this existing issue** unfixed. However, I could not understand you because I was completely focusing on how these functions could **introduce a new issue** and I could not find it. No wonder. Thus all my efforts were focused on finding a test cases which passed in the earlier versions but fail in this one because of these functions. Of course I could not find any and I was continuously proving that my patch is correct in the sense that it does not make things worse than they were. That is why I implemented the pointer-based iterators (it would have been better after this patch and a subsequent one that fixes this issue) where I was facing problems because of this wrong approach. In the same time you continuously came with new examples which tried to prove the issue, but I could not understand it because all these new test cases failed in the master version as well. We talked about two very different things because we had very different perceptions about the goal of this patch. That was all. 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
[PATCH] D77229: [Analyzer][NFC] Avoid handling of LazyCompundVals in IteratorModeling
baloghadamsoftware added a comment. @gamesh411, @whisperity, @martong and others, please suggest me new test cases if you think the current ones are not enough. 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
[PATCH] D77229: [Analyzer][NFC] Avoid handling of LazyCompundVals in IteratorModeling
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
