baloghadamsoftware marked 3 inline comments as done. baloghadamsoftware added a comment.
In D62688#1549574 <https://reviews.llvm.org/D62688#1549574>, @Szelethus wrote: > Hmm, an idea just popped into my head. I'm not sure whether we have a single > checker that does so much complicated (and totally awesome) modeling as > `IteratorChecker`. What do you think about a debug checker similar to > `debug.ExprInspection`, like `debug.IteratorInspection`? Good idea! However, I would do it in a way that we can reuse our existing debug functions in the tests: template <typename Iter> long clang_iterator_position(const Iter&); template <typename Iter, typename Cont> const Cont& clang_iterator_container(const Iter&); template <typename Iter> long clang_container_begin(const Iter&); template <typename Iter> long clang_container_end(const Iter&); Then we can nest calls for these functions into call of `clang_analyzer_dump()`, `clang_analyzer_eval()`, `clang_analyzer_denote()` etc. ================ Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:714 if (const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call)) { if (isBeginCall(Func)) { ---------------- Szelethus wrote: > We seem to retrieve the `CXXInstanceCall` here too? Eventually this function needs refactoring. ================ Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1741-1763 +bool isContainer(const CXXRecordDecl *CRD) { + if (!CRD) + return false; + + for (const auto *Decl : CRD->decls()) { + const auto *TD = dyn_cast<TypeDecl>(Decl); + if (!TD) ---------------- Szelethus wrote: > But what if we have a container for which the iterator isn't an inline class, > or we don't even have an in-class alias for it? I think whether the record > has a `begin`/`end` method would be a better heuristic. Or maybe both, or maybe either. I am not sure here. ================ Comment at: test/Analysis/iterator-range.cpp:247 +void empty(const std::vector<int> &V) { + for (auto n: V) {} + *V.begin(); // expected-warning{{Past-the-end iterator dereferenced}} ---------------- Szelethus wrote: > Aha, the for loop is here to force a state split on whether `V` is empty or > not, correct? Yes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62688/new/ https://reviews.llvm.org/D62688 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
