Fix #11720 FN functionConst when using base class members

lib/checkclass.cpp

@@ -2316,20 +2316,32 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
                 return false;
             memberAccessed = true;
         }
-        bool mayModifyArgs = true;
-        if (const Function* f = funcTok->function()) { // TODO: improve (we bail out if there is any possible modification of any argument)
+
+        if (const Function* f = funcTok->function()) { // check known function
             const std::vector<const Token*> args = getArguments(funcTok);
             const auto argMax = std::min<nonneg int>(args.size(), f->argCount());
-            mayModifyArgs = false;
+
             for (nonneg int argIndex = 0; argIndex < argMax; ++argIndex) {
                 const Variable* const argVar = f->getArgumentVar(argIndex);
-                if (!argVar || ((argVar->isArrayOrPointer() || argVar->isReference()) && !argVar->isConst())) {
-                    mayModifyArgs = true;
-                    break;
+                if (!argVar || ((argVar->isArrayOrPointer() || argVar->isReference()) &&
+                                !(argVar->valueType() && argVar->valueType()->isConst(argVar->valueType()->pointer)))) { // argument might be modified
+                    const Token* arg = args[argIndex];
+                    // Member variable given as parameter
+                    const Token* varTok = previousBeforeAstLeftmostLeaf(arg);
+                    if (!varTok)
+                        return false;
+                    varTok = varTok->next();
+                    if ((varTok->isName() && isMemberVar(scope, varTok)) || (varTok->isUnaryOp("&") && (varTok = varTok->astOperand1()) && isMemberVar(scope, varTok))) {
+                        const Variable* var = varTok->variable();
+                        if (!var || (!var->isMutable() && !var->isConst()))
+                            return false;
+                    }
                 }
             }
+            return true;
         }
-        // Member variable given as parameter
+
+        // Member variable given as parameter to unknown function
         const Token *lpar = funcTok->next();
         if (Token::simpleMatch(lpar, "( ) ("))
             lpar = lpar->tokAt(2);
@@ -2338,8 +2350,8 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
                 tok = tok->link();
             else if ((tok->isName() && isMemberVar(scope, tok)) || (tok->isUnaryOp("&") && (tok = tok->astOperand1()) && isMemberVar(scope, tok))) {
                 const Variable* var = tok->variable();
-                if ((!var || (!var->isMutable() && !var->isConst())) && mayModifyArgs)
-                    return false; // TODO: Only bailout if function takes argument as non-const reference
+                if (!var || (!var->isMutable() && !var->isConst()))
+                    return false;
             }
         }
         return true;

lib/checkmemoryleak.cpp

@@ -707,7 +707,7 @@ void CheckMemoryLeakInClass::publicAllocationError(const Token *tok, const std::
 }
 
 
-void CheckMemoryLeakStructMember::check()
+void CheckMemoryLeakStructMember::check() const
 {
     if (mSettings->clang)
         return;
@@ -740,7 +740,7 @@ bool CheckMemoryLeakStructMember::isMalloc(const Variable *variable)
     return alloc;
 }
 
-void CheckMemoryLeakStructMember::checkStructVariable(const Variable * const variable)
+void CheckMemoryLeakStructMember::checkStructVariable(const Variable* const variable) const
 {
     if (!variable)
         return;

lib/checkmemoryleak.h

@@ -277,14 +277,14 @@ class CPPCHECKLIB CheckMemoryLeakStructMember : private Check, private CheckMemo
         checkMemoryLeak.check();
     }
 
-    void check();
+    void check() const;
 
 private:
 
     /** Is local variable allocated with malloc? */
     static bool isMalloc(const Variable *variable);
 
-    void checkStructVariable(const Variable * const variable);
+    void checkStructVariable(const Variable* const variable) const;
 
     void getErrorMessages(ErrorLogger * /*errorLogger*/, const Settings * /*settings*/) const override {}
 

lib/reverseanalyzer.cpp

@@ -120,7 +120,7 @@ struct ReverseTraversal {
         return continueB;
     }
 
-    Analyzer::Action analyzeRecursive(const Token* start) {
+    Analyzer::Action analyzeRecursive(const Token* start) const {
         Analyzer::Action result = Analyzer::Action::None;
         visitAstNodes(start, [&](const Token* tok) {
             result |= analyzer->analyze(tok, Analyzer::Direction::Reverse);

lib/symboldatabase.h

@@ -1479,7 +1479,7 @@ class CPPCHECKLIB SymbolDatabase {
     void createSymbolDatabaseSetTypePointers();
     void createSymbolDatabaseSetSmartPointerType();
     void createSymbolDatabaseEnums(); // cppcheck-suppress functionConst // has side effects
-    void createSymbolDatabaseEscapeFunctions();
+    void createSymbolDatabaseEscapeFunctions(); // cppcheck-suppress functionConst // has side effects
     // cppcheck-suppress functionConst
     void createSymbolDatabaseIncompleteVars();
 

test/testclass.cpp

@@ -198,6 +198,7 @@ class TestClass : public TestFixture {
         TEST_CASE(const85);
         TEST_CASE(const86);
         TEST_CASE(const87);
+        TEST_CASE(const88);
         TEST_CASE(const89);
 
         TEST_CASE(const_handleDefaultParameters);
@@ -6026,7 +6027,7 @@ class TestClass : public TestFixture {
                    "    int i{};\n"
                    "    S f() { return S(&i); }\n"
                    "};\n");
-        TODO_ASSERT_EQUALS("[test.cpp:7]: (style, inconclusive) Technically the member function 'C::f' can be const.\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:7]: (style, inconclusive) Technically the member function 'C::f' can be const.\n", errout.str());
 
         checkConst("struct S {\n"
                    "    const int* mp{};\n"
@@ -6317,6 +6318,16 @@ class TestClass : public TestFixture {
                    "    void g() { p->f(i); }\n"
                    "};\n");
         ASSERT_EQUALS("", errout.str());
+
+        checkConst("struct A {\n" // #11501
+                   "    enum E { E1 };\n"
+                   "    virtual void f(E) const = 0;\n"
+                   "};\n"
+                   "struct F {\n"
+                   "    A* a;\n"
+                   "    void g() { a->f(A::E1); }\n"
+                   "};\n");
+        ASSERT_EQUALS("[test.cpp:7]: (style, inconclusive) Technically the member function 'F::g' can be const.\n", errout.str());
     }
 
     void const82() { // #11513
@@ -6333,9 +6344,8 @@ class TestClass : public TestFixture {
                    "    void h(int, int*) const;\n"
                    "    void g() { int a; h(i, &a); }\n"
                    "};\n");
-        TODO_ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Technically the member function 'S::g' can be const.\n",
-                           "",
-                           errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Technically the member function 'S::g' can be const.\n",
+                      errout.str());
     }
 
     void const83() {
@@ -6400,7 +6410,113 @@ class TestClass : public TestFixture {
         ASSERT_EQUALS("", errout.str());
     }
 
-    void const87() { // #11626
+    void const87() {
+        checkConst("struct Tokenizer {\n" // #11720
+                   "        bool isCPP() const {\n"
+                   "        return cpp;\n"
+                   "    }\n"
+                   "    bool cpp;\n"
+                   "};\n"
+                   "struct Check {\n"
+                   "    const Tokenizer* const mTokenizer;\n"
+                   "    const int* const mSettings;\n"
+                   "};\n"
+                   "struct CheckA : Check {\n"
+                   "    static bool test(const std::string& funcname, const int* settings, bool cpp);\n"
+                   "};\n"
+                   "struct CheckB : Check {\n"
+                   "    bool f(const std::string& s);\n"
+                   "};\n"
+                   "bool CheckA::test(const std::string& funcname, const int* settings, bool cpp) {\n"
+                   "    return !funcname.empty() && settings && cpp;\n"
+                   "}\n"
+                   "bool CheckB::f(const std::string& s) {\n"
+                   "    return CheckA::test(s, mSettings, mTokenizer->isCPP());\n"
+                   "}\n");
+        ASSERT_EQUALS("[test.cpp:20] -> [test.cpp:15]: (style, inconclusive) Technically the member function 'CheckB::f' can be const.\n", errout.str());
+
+        checkConst("void g(int&);\n"
+                   "struct S {\n"
+                   "    struct { int i; } a[1];\n"
+                   "    void f() { g(a[0].i); }\n"
+                   "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkConst("struct S {\n"
+                   "    const int& g() const { return i; }\n"
+                   "    int i;\n"
+                   "};\n"
+                   "void h(int, const int&);\n"
+                   "struct T {\n"
+                   "    S s;\n"
+                   "    int j;\n"
+                   "    void f() { h(j, s.g()); }\n"
+                   "};\n");
+        ASSERT_EQUALS("[test.cpp:9]: (style, inconclusive) Technically the member function 'T::f' can be const.\n", errout.str());
+
+        checkConst("struct S {\n"
+                   "    int& g() { return i; }\n"
+                   "    int i;\n"
+                   "};\n"
+                   "void h(int, int&);\n"
+                   "struct T {\n"
+                   "    S s;\n"
+                   "    int j;\n"
+                   "    void f() { h(j, s.g()); }\n"
+                   "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkConst("struct S {\n"
+                   "    const int& g() const { return i; }\n"
+                   "    int i;\n"
+                   "};\n"
+                   "void h(int, const int*);\n"
+                   "struct T {\n"
+                   "    S s;\n"
+                   "    int j;\n"
+                   "    void f() { h(j, &s.g()); }\n"
+                   "};\n");
+        ASSERT_EQUALS("[test.cpp:9]: (style, inconclusive) Technically the member function 'T::f' can be const.\n", errout.str());
+
+        checkConst("struct S {\n"
+                   "    int& g() { return i; }\n"
+                   "    int i;\n"
+                   "};\n"
+                   "void h(int, int*);\n"
+                   "struct T {\n"
+                   "    S s;\n"
+                   "    int j;\n"
+                   "    void f() { h(j, &s.g()); }\n"
+                   "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkConst("void j(int** x);\n"
+                   "void k(int* const* y);\n"
+                   "struct S {\n"
+                   "    int* p;\n"
+                   "    int** q;\n"
+                   "    int* const* r;\n"
+                   "    void f1() { j(&p); }\n"
+                   "    void f2() { j(q); }\n"
+                   "    void g1() { k(&p); }\n"
+                   "    void g2() { k(q); }\n"
+                   "    void g3() { k(r); }\n"
+                   "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkConst("void m(int*& r);\n"
+                   "void n(int* const& s);\n"
+                   "struct T {\n"
+                   "    int i;\n"
+                   "    int* p;\n"
+                   "    void f1() { m(p); }\n"
+                   "    void f2() { n(&i); }\n"
+                   "    void f3() { n(p); }\n"
+                   "};\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void const88() { // #11626
         checkConst("struct S {\n"
                    "    bool f() { return static_cast<bool>(p); }\n"
                    "    const int* g() { return const_cast<const int*>(p); }\n"

test/testother.cpp

@@ -3633,6 +3633,20 @@ class TestOther : public TestFixture {
                       "[test.cpp:19]: (style) Parameter 's' can be declared as pointer to const\n",
                       errout.str());
 
+        check("struct S {\n" // #11573
+              "    const char* g() const {\n"
+              "        return m;\n"
+              "    }\n"
+              "    const char* m;\n"
+              "};\n"
+              "struct T { std::vector<S*> v; };\n"
+              "void f(T* t, const char* n) {\n"
+              "    for (const auto* p : t->v)\n"
+              "        if (strcmp(p->g(), n) == 0) {}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:8]: (style) Parameter 't' can be declared as pointer to const\n",
+                      errout.str());
+
         check("void f(int*& p, int* q) {\n"
               "    p = q;\n"
               "}\n");