[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.


================
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.
----------------
baloghadamsoftware wrote:
> 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.
One more thing:

The main purpose of iterators is to make algorithms independent of the data representation. So you can decide whether your algorithm works on a specific representation and create non-template function that takes reference for the specific container itself or you make it generic so you use template function which takes iterators. If you chose this latter one for an algorithm that works on two different container then there is no point to restrict the function to only work on two containers with exactly the same representation. Either specific or generic, but there is no point for something in between.


https://reviews.llvm.org/D32845