CheckStl: Use AST to handle iterator comparisons better

Author: danmar

lib/checkstl.cpp

@@ -140,9 +140,15 @@ void CheckStl::iterators()
                 validIterator = true;
 
             // Is iterator compared against different container?
-            if (Token::Match(tok2, "%varid% !=|== %name% . end|rend|cend|crend ( )", iteratorId) && container && tok2->tokAt(2)->varId() != container->declarationId()) {
-                iteratorsError(tok2, container->name(), tok2->strAt(2));
-                tok2 = tok2->tokAt(6);
+            if (tok2->isComparisonOp() && container) {
+                const Token *other = nullptr;
+                if (tok2->astOperand1()->varId() == iteratorId)
+                    other = tok2->astOperand2()->tokAt(-3);
+                else if (tok2->astOperand2()->varId() == iteratorId)
+                    other = tok2->astOperand1()->tokAt(-3);
+
+                if (Token::Match(other, "%name% . end|rend|cend|crend ( )") && other->varId() != container->declarationId())
+                    iteratorsError(tok2, container->name(), other->str());
             }
 
             // Is the iterator used in a insert/erase operation?

test/teststl.cpp

@@ -172,6 +172,15 @@ class TestStl : public TestFixture {
               "}");
         ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
 
+        check("void f()\n"
+              "{\n"
+              "    list<int> l1;\n"
+              "    list<int> l2;\n"
+              "    for (list<int>::iterator it = l1.begin(); l2.end() != it; ++it)\n"
+              "    { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
+
         // Same check with reverse iterator
         check("void f()\n"
               "{\n"
@@ -442,6 +451,14 @@ class TestStl : public TestFixture {
               "}");
         ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'map1' and 'map2'.\n", errout.str());
 
+        check("void f() {\n"
+              "    std::map<int, int> map1;\n"
+              "    std::map<int, int> map2;\n"
+              "    std::map<int, int>::const_iterator it = map1.find(123);\n"
+              "    if (map2.end() == it) { }"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'map1' and 'map2'.\n", errout.str());
+
         check("void f(std::string &s) {\n"
               "    int pos = s.find(x);\n"
               "    s.erase(pos);\n"
@@ -457,7 +474,7 @@ class TestStl : public TestFixture {
               "    std::vector<int>::const_iterator it;\n"
               "    it = a.begin();\n"
               "    while (it!=a.end())\n"
-              "        v++it;\n"
+              "        ++it;\n"
               "    it = t.begin();\n"
               "    while (it!=a.end())\n"
               "        ++it;\n"