baloghadamsoftware marked 2 inline comments as done. baloghadamsoftware added a comment.
In https://reviews.llvm.org/D25660#590778, @NoQ wrote: > - Agree on the `evalAssume()` implementation (i'm still not quite > understanding what the problem is here, see the new inline comments); I think it will be settled soon. > - We should probably not warn by default on unchecked std::find (see comments > following the `push_back(2016)` example), unless some strong arguments > against such code patterns are provided; It is automatic. The role of evalCall is only to reduce the exploded graph. If I remove it, we get the same result (that is why we have a nonStdFind there, to check this case). but with far more states. Especially in case of vector, where the GNU implementation is quite complicated because of optimizations. > - A BugReporterVisitor should be added to report iterator state changes to > the user across the diagnostic path; I also thought of this. The question is where to start the chain. > - Our code owners often have strong opinions regarding warning message > wording. I need suggestions here. > Then there are a few ideas on finding more bugs, which you shouldn't > necessarily implement, that came up during the review, eg.: > > - Alexey suspects that iterators implemented as plain pointers (commonly used > in LLVM itself) might be unsupported; I think it is supported now. > - Alexey points out that ++/-- may be handled in a less conservative manner; That is a future plan, but then it also results in a new name for the checker, e.g. IteratorRange. > - More checks could be implemented in this checker, eg. passing `end()` as > first argument of `std::find()` is an instant bug (somebody accidentally > swapped begin and end?). Good idea, but what if it is intentional? I do not mean that we pass end() directly, but if we do a loop of find() functions where the beginning of the next range is always the successor of the last found element, we may result in a range of [end(), end()[, which I think is a valid empty range: const auto start = v.begin(); while(true) { const auto item = find(start, v.end()); if(item==v.end()) break; doSomething(*item); start = ++item; } > A list of ideas on improving core experience, mostly for myself because i > seem to be the only one with strong opinions on this: > > - Provide a checker callback for structure copies, which would unify the > multitude of similar callbacks in this checker; A callback? Or just move the copy into a simple (or template?) function? > - Consider removing the conjured structure symbol hack. Which hack do you mean here? In evalCall() of the various std functions? As I mentioned, they can be removed, but then we will get more states in the exploded graph. > Did i forget anything? https://reviews.llvm.org/D25660 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
