Szelethus added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542 + BR.markInteresting(It1); + if (const auto &LCV1 = It1.getAs<nonloc::LazyCompoundVal>()) { + BR.markInteresting(LCV1->getRegion()); + } ---------------- NoQ wrote: > baloghadamsoftware wrote: > > NoQ wrote: > > > I'm opposed to this code for the same reason that i'm opposed to it in > > > the debug checker. Parent region is an undocumented implementation detail > > > of `RegionStore`. It is supposed to be immaterial to the user. You should > > > not rely on its exact value. > > > > > > @baloghadamsoftware Can we eliminate all such code from iterator checkers > > > and instead identify all iterators by regions in which they're stored? > > > Does my improved C++ support help with this anyhow whenever it kicks in? > > How to find the region where it is stored? I am open to find better > > solutions, but it was the only one I could find so far. If we ignore > > `LazyCompoundVal` then everything falls apart, we can remove all the > > iterator-related checkers. > It's the region from which you loaded it. If you obtained it as > `Call.getArgSVal()` then it's the parameter region for the call. If you > obtained it as `Call.getReturnValue()` then it's the target region can be > obtained by looking at the //construction context// for the call. `LazyCompoundVal` and friends seem to be a constantly emerging headache for almost everyone. For how long I've spent in the analyzer, and have religiously studied conversations and your workbook about it, I still feel anxious whenever it comes up. It would be great to have this documented in the code sometime. ================ Comment at: clang/test/Analysis/iterator-modelling.cpp:169 clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}} // expected-note@-1{{TRUE}} ---------------- baloghadamsoftware wrote: > Szelethus wrote: > > Interestingness won't be propagated here because > > `clang_analyzer_iterator_container(i2) == &v` is interesting, not > > `clang_analyzer_iterator_container(i2)`, correct? > Currently only `clang_analyzer_express()` marks its argument as interesting. > This could be extended in the future, however the argument of > `clang_analyzer_eval()` is usually the result of the comparison, not the > symbolic comparison itself so the sides of the comparison are not reachable > at that point. Fair enough. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75677/new/ https://reviews.llvm.org/D75677 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits