baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:382
}
+ } else if (!isa<CXXConstructorCall>(&Call)) {
+ // The main purpose of iterators is to abstract away from different
----------------
a.sidorin wrote:
> The function becomes > 100 lines long. Should we refactor this check into a
> separate function to improve readability?
Yes, I think so this would be a good idea. Should I do it now?
================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:388
+ // template parameters for different containers. So we can safely
+ // assume that passing iterators of different containers as arguments
+ // whose type replaces the same template parameter is a bug.
----------------
a.sidorin wrote:
> While this assumption is sane and is true for <algorithm> functions, user
> code can have other design solutions. There is nothing that prevents users
> from writing a function looking like:
> ```
> template <typename IterTy>
> void f(IterTy FromBegin, IterTy FromEnd, IterTy ToBegin, IterTy ToEnd);
> ```
> and there is nothing wrong with it.
> One of possible solutions is to restrict checker to check only functions
> from std namespace. What do you think?
We can restrict, of course, but first we should measure how it performs on real
code. With the restriction, we can get rid of some false positives but we may
also loose some true positives.
https://reviews.llvm.org/D32845
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
