Partial fix for #13301 false negative: functionConst with const/non-c…

…onst overloads (danmar#7003)
swasti16 · Dec 6, 2024 · aad6e09 · aad6e09
1 parent d6e7f40
commit aad6e09
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 8 deletions.
diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
@@ -2537,18 +2537,35 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, Member
                 return func && func->isConst();
             };
 
+            auto isConstContainerUsage = [&](const ValueType* vt) -> bool {
+                if (!vt || !vt->container)
+                    return false;
+                const auto yield = vt->container->getYield(end->str());
+                if (yield == Library::Container::Yield::START_ITERATOR || yield == Library::Container::Yield::END_ITERATOR) {
+                    const Token* parent = tok1->astParent();
+                    while (Token::Match(parent, "(|.|::"))
+                        parent = parent->astParent();
+                    if (parent && parent->isComparisonOp())
+                        return true;
+                    // TODO: use AST
+                    if (parent && parent->isAssignmentOp() && tok1->tokAt(-2)->variable() && Token::Match(tok1->tokAt(-2)->variable()->typeEndToken(), "const_iterator|const_reverse_iterator"))
+                        return true;
+                }
+                if ((yield == Library::Container::Yield::ITEM || yield == Library::Container::Yield::AT_INDEX) &&
+                    (lhs->isComparisonOp() || lhs->isAssignmentOp() || (lhs->str() == "(" && Token::Match(lhs->astParent(), "%cop%"))))
+                    return true; // assume that these functions have const overloads
+                return false;
+            };
+
             if (end->strAt(1) == "(") {
                 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"))) ||
-
-                    (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() && tok1->tokAt(-2)->variable() && Token::Match(tok1->tokAt(-2)->variable()->typeEndToken(), "const_iterator|const_reverse_iterator"))))) {
+                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"))) {
+                    // empty body
+                }
+                else if (isConstContainerUsage(lastVarTok->valueType())) {
                     // empty body
                 }
                 else if (var->smartPointerType() && var->smartPointerType()->classScope && isConstMemberFunc(var->smartPointerType()->classScope, end)) {

diff --git a/test/testclass.cpp b/test/testclass.cpp
@@ -188,6 +188,7 @@ class TestClass : public TestFixture {
         TEST_CASE(const94);
         TEST_CASE(const95); // #13320 - do not warn about r-value ref method
         TEST_CASE(const96);
+        TEST_CASE(const97);
 
         TEST_CASE(const_handleDefaultParameters);
         TEST_CASE(const_passThisToMemberOfOtherClass);
@@ -6723,6 +6724,45 @@ class TestClass : public TestFixture {
         ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Either there is a missing 'override', or the member function 'S::f' can be const.\n", errout_str());
     }
 
+    void const97() { // #13301
+        checkConst("struct S {\n"
+                   "    std::vector<int> v;\n"
+                   "    int f() {\n"
+                   "        const int& r = v.front();\n"
+                   "        return r;\n"
+                   "    }\n"
+                   "    int g() {\n"
+                   "        const int& r = v.at(0);\n"
+                   "        return r;\n"
+                   "    }\n"
+                   "    void h() {\n"
+                   "        if (v.front() == 0) {}\n"
+                   "        if (1 == v.front()) {}\n"
+                   "    }\n"
+                   "    void i() {\n"
+                   "        v.at(0) = 0;\n"
+                   "    }\n"
+                   "    void j() {\n"
+                   "        dostuff(1, v.at(0));\n"
+                   "    }\n"
+                   "    void k() {\n"
+                   "        int& r = v.front();\n"
+                   "        r = 0;\n"
+                   "    }\n"
+                   "};\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Technically the member function 'S::f' can be const.\n"
+                      "[test.cpp:7]: (style, inconclusive) Technically the member function 'S::g' can be const.\n"
+                      "[test.cpp:11]: (style, inconclusive) Technically the member function 'S::h' can be const.\n",
+                      errout_str());
+
+        checkConst("struct B { std::string s; };\n"
+                   "struct D : B {\n"
+                   "    bool f(std::string::iterator it) { return it == B::s.begin(); }\n"
+                   "};\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Technically the member function 'D::f' can be const.\n",
+                      errout_str());
+    }
+
     void const_handleDefaultParameters() {
         checkConst("struct Foo {\n"
                    "    void foo1(int i, int j = 0) {\n"