diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index a3ff5dfe4aa..d8c9298afee 100644 --- a/lib/checkclass.cpp +++ b/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 args = getArguments(funcTok); const auto argMax = std::min(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; diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 71dacdf5e12..209f1bdd283 100644 --- a/lib/checkmemoryleak.cpp +++ b/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; diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index de78f308c3e..0cc9a151065 100644 --- a/lib/checkmemoryleak.h +++ b/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 {} diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 70311afbcd3..f3158478140 100644 --- a/lib/reverseanalyzer.cpp +++ b/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); diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 739d7d0f8aa..b170a4c6ef1 100644 --- a/lib/symboldatabase.h +++ b/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(); diff --git a/test/testclass.cpp b/test/testclass.cpp index 6fc1363995e..a9015dc84ce 100644 --- a/test/testclass.cpp +++ b/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(p); }\n" " const int* g() { return const_cast(p); }\n" diff --git a/test/testother.cpp b/test/testother.cpp index 4ba61960b74..5b23e951f89 100644 --- a/test/testother.cpp +++ b/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 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");