Fix issue 4693: Diagnostic when using the same iterators to an algorithm (#1326)

* Fix issue 4693: Diagnostic when using the same iterators to an algorithm

* Update classinfo

lib/checkstl.cpp

@@ -314,6 +314,11 @@ void CheckStl::mismatchingContainerExpressionError(const Token *tok1, const Toke
                 expr1 + "' and '" + expr2 + "' are used together.", CWE664, false);
 }
 
+void CheckStl::sameIteratorExpressionError(const Token *tok)
+{
+    reportError(tok, Severity::style, "sameIteratorExpression", "Same iterators expression are used for algorithm.", CWE664, false);
+}
+
 static const std::set<std::string> algorithm2 = { // func(begin1, end1
     "binary_search", "copy", "copy_if", "equal_range"
     , "generate", "is_heap", "is_heap_until", "is_partitioned"
@@ -392,6 +397,12 @@ void CheckStl::mismatchingContainers()
                 if (!i)
                     continue;
                 const Token * const argTok = args[argnr - 1];
+                if (i->first) {
+                    firstArg = argTok;
+                } 
+                if(i->last && firstArg && argTok && isSameExpression(true, false, firstArg, argTok, mSettings->library, false)) {
+                     sameIteratorExpressionError(firstArg);
+                }
                 const Variable *c = getContainer(argTok);
                 if (c) {
                     std::map<const Variable *, unsigned int>::const_iterator it = containerNr.find(c);
@@ -407,9 +418,7 @@ void CheckStl::mismatchingContainers()
                         mismatchingContainersError(argTok);
                     }
                 } else {
-                    if (i->first) {
+                    if (i->last && firstArg && argTok) {
-                        firstArg = argTok;
-                    } else if (i->last && firstArg && argTok) {
                         const Token * iter1 = getIteratorExpression(firstArg);
                         const Token * iter2 = getIteratorExpression(argTok);
                         if (iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false)) {

lib/checkstl.h

@@ -184,6 +184,7 @@ private:
     void iteratorsError(const Token* tok, const std::string& container1, const std::string& container2);
     void mismatchingContainersError(const Token* tok);
     void mismatchingContainerExpressionError(const Token *tok1, const Token *tok2);
+    void sameIteratorExpressionError(const Token *tok);
     void invalidIteratorError(const Token* tok, const std::string& func, const std::string& iterator_name);
     void invalidPointerError(const Token* tok, const std::string& func, const std::string& pointer_name);
     void stlBoundariesError(const Token* tok);
@@ -212,6 +213,7 @@ private:
         c.iteratorsError(nullptr, "container1", "container2");
         c.mismatchingContainersError(nullptr);
         c.mismatchingContainerExpressionError(nullptr, nullptr);
+        c.sameIteratorExpressionError(nullptr);
         c.dereferenceErasedError(nullptr, nullptr, "iter", false);
         c.stlOutOfBoundsError(nullptr, "i", "foo", false);
         c.negativeIndexError(nullptr, ValueFlow::Value(-1));
@@ -249,6 +251,7 @@ private:
                "- out of bounds errors\n"
                "- misuse of iterators when iterating through a container\n"
                "- mismatching containers in calls\n"
+               "- same iterators in calls\n"
                "- dereferencing an erased iterator\n"
                "- for vectors: using iterator/pointer after push_back has been used\n"
                "- optimisation: use empty() instead of size() to guarantee fast code\n"

test/teststl.cpp

@@ -55,6 +55,7 @@ private:
         TEST_CASE(iterator13);
         TEST_CASE(iterator14); // #8191
         TEST_CASE(iteratorExpression);
+        TEST_CASE(iteratorSameExpression);
 
         TEST_CASE(dereference);
         TEST_CASE(dereference_break);  // #3644 - handle "break"
@@ -595,6 +596,41 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void iteratorSameExpression() {
+        check("void f(std::vector<int> v) {\n"
+              "    std::for_each(v.begin(), v.begin(), [](int){});\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Same iterators expression are used for algorithm.\n", errout.str());
+
+        check("std::vector<int>& g();\n"
+              "void f() {\n"
+              "    std::for_each(g().begin(), g().begin(), [](int){});\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Same iterators expression are used for algorithm.\n", errout.str());
+
+        check("void f(std::vector<int> v) {\n"
+              "    std::for_each(v.end(), v.end(), [](int){});\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Same iterators expression are used for algorithm.\n", errout.str());
+
+        check("std::vector<int>& g();\n"
+              "void f() {\n"
+              "    std::for_each(g().end(), g().end(), [](int){});\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Same iterators expression are used for algorithm.\n", errout.str());
+
+        check("std::vector<int>::iterator g();\n"
+              "void f(std::vector<int> v) {\n"
+              "    std::for_each(g(), g(), [](int){});\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Same iterators expression are used for algorithm.\n", errout.str());
+
+        check("void f(std::vector<int>::iterator it) {\n"
+              "    std::for_each(it, it, [](int){});\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Same iterators expression are used for algorithm.\n", errout.str());
+    }
+
     // Dereferencing invalid pointer
     void dereference() {
         check("void f()\n"