From 2cacb13f857d4b35619e559f0996ff63996483a3 Mon Sep 17 00:00:00 2001
From: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Date: Wed, 2 Feb 2022 19:37:06 +0100
Subject: [PATCH] Fix #10671: functionConst FN with begin/end and
 const_iterator (#3749)

Check if the iterator is assigned to a const_iterator or
const_revese_iterator, in which case it is possible the function can be
const. Unfortunately, it is not possible to remove the hard coding of
cbegin, cend, crbegin and crend due to the need to handle auto, as in
the following code snippet:

    void cbegin_auto(void)
    {
        for (auto it = m_str.cbegin(); it != m_str.cend(); ++it) {;}
    }
---
 lib/checkclass.cpp |  9 +++++++--
 test/cfg/std.cpp   | 43 +++++++++++++++++++++++++++++++++++++++++++
 test/testclass.cpp | 19 +++++++++++++++++++
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
index 3c404b5d2..942f6c020 100644
--- a/lib/checkclass.cpp
+++ b/lib/checkclass.cpp
@@ -2205,8 +2205,13 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
                 const Variable *var = lastVarTok->variable();
                 if (!var)
                     return false;
-                if (var->isStlType() // assume all std::*::size() and std::*::empty() are const
-                    && (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy")))
+                if ((var->isStlType() // assume all std::*::size() and std::*::empty() are const
+                     && (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy"))) ||
+
+                    (lastVarTok->valueType() && lastVarTok->valueType()->container &&
+                     ((lastVarTok->valueType()->container->getYield(end->str()) == Library::Container::Yield::START_ITERATOR) ||
+                      (lastVarTok->valueType()->container->getYield(end->str()) == Library::Container::Yield::END_ITERATOR))
+                     && (tok1->previous()->isComparisonOp() || (tok1->previous()->isAssignmentOp() && Token::Match(tok1->tokAt(-2)->variable()->typeEndToken(), "const_iterator|const_reverse_iterator")))))
                     ;
                 else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end))
                     return false;
diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp
index 3e06bd92a..fc15910ea 100644
--- a/test/cfg/std.cpp
+++ b/test/cfg/std.cpp
@@ -3636,3 +3636,46 @@ void stdbind()
     // cppcheck-suppress unreadVariable
     auto f2 = std::bind(stdbind_helper, 10);
 }
+
+class A
+{
+    std::vector<std::string> m_str;
+
+public:
+
+    A() {}
+
+    // cppcheck-suppress functionConst
+    void begin_const_iterator(void)
+    {
+        for (std::vector<std::string>::const_iterator it = m_str.begin(); it != m_str.end(); ++it) {;}
+    }
+    // cppcheck-suppress functionConst
+    void cbegin_const_iterator(void)
+    {
+        for (std::vector<std::string>::const_iterator it = m_str.cbegin(); it != m_str.cend(); ++it) {;}
+    }
+    // cppcheck-suppress functionConst
+    void crbegin_const_iterator(void)
+    {
+        for (std::vector<std::string>::const_reverse_iterator it = m_str.crbegin(); it != m_str.crend(); ++it) {;}
+    }
+    // cppcheck-suppress functionConst
+    void rbegin_const_iterator(void)
+    {
+        for (std::vector<std::string>::const_reverse_iterator it = m_str.rbegin(); it != m_str.rend(); ++it) {;}
+    }
+    // cppcheck-suppress functionConst
+    void cbegin_auto(void)
+    {
+        for (auto it = m_str.cbegin(); it != m_str.cend(); ++it) {;}
+    }
+    void baz_begin_no_const_iterator(void)
+    {
+        for (std::vector<std::string>::iterator it = m_str.begin(); it != m_str.end(); ++it) {;}
+    }
+    void rbegin_no_const_iterator(void)
+    {
+        for (std::vector<std::string>::reverse_iterator it = m_str.rbegin(); it != m_str.rend(); ++it) {;}
+    }
+};
diff --git a/test/testclass.cpp b/test/testclass.cpp
index 66b0ed4b5..3f0aef3d0 100644
--- a/test/testclass.cpp
+++ b/test/testclass.cpp
@@ -54,6 +54,12 @@ private:
                                    "    <dealloc>free</dealloc>\n"
                                    "  </memory>\n"
                                    "  <smart-pointer class-name=\"std::shared_ptr\"/>\n"
+                                   "  <container id=\"stdVector\" startPattern=\"std :: vector &lt;\" itEndPattern=\"&gt; :: const_iterator\">\n"
+                                   "    <access>\n"
+                                   "      <function name=\"begin\" yields=\"start-iterator\"/>\n"
+                                   "      <function name=\"end\" yields=\"end-iterator\"/>\n"
+                                   "    </access>\n"
+                                   "  </container>\n"
                                    "</def>";
             tinyxml2::XMLDocument doc;
             doc.Parse(xmldata, sizeof(xmldata));
@@ -184,6 +190,7 @@ private:
         TEST_CASE(const71); // ticket #10146
         TEST_CASE(const72); // ticket #10520
         TEST_CASE(const73); // ticket #10735
+        TEST_CASE(const74); // ticket #10671
         TEST_CASE(const_handleDefaultParameters);
         TEST_CASE(const_passThisToMemberOfOtherClass);
         TEST_CASE(assigningPointerToPointerIsNotAConstOperation);
@@ -5919,6 +5926,18 @@ private:
         ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (style, inconclusive) Technically the member function 'S::f' can be const.\n", errout.str());
     }
 
+    void const74() { // #10671
+        checkConst("class A {\n"
+                   "    std::vector<std::string> m_str;\n"
+                   "public:\n"
+                   "    A() {}\n"
+                   "    void bar(void) {\n"
+                   "        for(std::vector<std::string>::const_iterator it = m_str.begin(); it != m_str.end(); ++it) {;}\n"
+                   "    }\n"
+                   "};");
+        ASSERT_EQUALS("[test.cpp:5]: (style, inconclusive) Technically the member function 'A::bar' can be const.\n", errout.str());
+    }
+
     void const_handleDefaultParameters() {
         checkConst("struct Foo {\n"
                    "    void foo1(int i, int j = 0) {\n"