From 10b55cc0cfff98badd22e35922262b518ffe0783 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Fri, 26 May 2023 17:24:13 +0200 Subject: [PATCH] Fix #11654 FN functionConst if only non-const member usage is call to itself (#5092) * Fix #11654 FN functionConst if only non-const member usage is call to itself * Format * Add const --- lib/checkclass.cpp | 10 +++++----- lib/checkclass.h | 2 +- lib/checkuninitvar.cpp | 2 +- lib/checkuninitvar.h | 2 +- lib/tokenize.h | 2 +- test/testclass.cpp | 24 ++++++++++++++++++++++++ 6 files changed, 33 insertions(+), 9 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 4192286da..a3ff5dfe4 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -741,7 +741,7 @@ bool CheckClass::isBaseClassMutableMemberFunc(const Token *tok, const Scope *sco return false; } -void CheckClass::initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage) +void CheckClass::initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage) const { if (!func.functionScope) return; @@ -2310,9 +2310,9 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& return nullptr; }; - auto checkFuncCall = [this, &memberAccessed](const Token* funcTok, const Scope* scope) { + auto checkFuncCall = [this, &memberAccessed](const Token* funcTok, const Scope* scope, const Function* func) { if (isMemberFunc(scope, funcTok) && (funcTok->strAt(-1) != "." || Token::simpleMatch(funcTok->tokAt(-2), "this ."))) { - if (!isConstMemberFunc(scope, funcTok)) + if (!isConstMemberFunc(scope, funcTok) && func != funcTok->function()) return false; memberAccessed = true; } @@ -2490,7 +2490,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& return false; tok1 = jumpBackToken?jumpBackToken:end; // Jump back to first [ to check inside, or jump to end of expression - if (tok1 == end && Token::Match(end->previous(), ". %name% ( !!)") && !checkFuncCall(tok1, scope)) // function call on member + if (tok1 == end && Token::Match(end->previous(), ". %name% ( !!)") && !checkFuncCall(tok1, scope, func)) // function call on member return false; } @@ -2509,7 +2509,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& // function/constructor call, return init list else if (const Token* funcTok = getFuncTok(tok1)) { - if (!checkFuncCall(funcTok, scope)) + if (!checkFuncCall(funcTok, scope, func)) return false; } else if (Token::simpleMatch(tok1, "> (") && (!tok1->link() || !Token::Match(tok1->link()->previous(), "static_cast|const_cast|dynamic_cast|reinterpret_cast"))) { return false; diff --git a/lib/checkclass.h b/lib/checkclass.h index 93c50aaf1..4a940857f 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -392,7 +392,7 @@ private: * @param scope pointer to variable Scope * @param usage reference to usage vector */ - void initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage); + void initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage) const; /** * @brief gives a list of tokens where virtual functions are called directly or indirectly diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 05c7cc79f..080e42bc6 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -827,7 +827,7 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var return false; } -const Token *CheckUninitVar::checkExpr(const Token *tok, const Variable& var, const Alloc alloc, bool known, bool *bailout) +const Token* CheckUninitVar::checkExpr(const Token* tok, const Variable& var, const Alloc alloc, bool known, bool* bailout) const { if (!tok) return nullptr; diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index 7eb5e554f..8c4ccbccb 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -82,7 +82,7 @@ public: void checkStruct(const Token *tok, const Variable &structvar); enum Alloc { NO_ALLOC, NO_CTOR_CALL, CTOR_CALL, ARRAY }; bool checkScopeForVariable(const Token *tok, const Variable& var, bool* const possibleInit, bool* const noreturn, Alloc* const alloc, const std::string &membervar, std::map variableValue); - const Token *checkExpr(const Token *tok, const Variable& var, const Alloc alloc, bool known, bool *bailout=nullptr); + const Token* checkExpr(const Token* tok, const Variable& var, const Alloc alloc, bool known, bool* bailout = nullptr) const; bool checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, Alloc alloc, const std::string &membervar); bool checkLoopBody(const Token *tok, const Variable& var, const Alloc alloc, const std::string &membervar, const bool suppressErrors); const Token* checkLoopBodyRecursive(const Token *start, const Variable& var, const Alloc alloc, const std::string &membervar, bool &bailout) const; diff --git a/lib/tokenize.h b/lib/tokenize.h index c6ab8dd03..f296f10ae 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -195,7 +195,7 @@ public: * \param only_k_r_fpar Only simplify K&R function parameters */ void simplifyVarDecl(const bool only_k_r_fpar); - void simplifyVarDecl(Token * tokBegin, const Token * const tokEnd, const bool only_k_r_fpar); + void simplifyVarDecl(Token * tokBegin, const Token * const tokEnd, const bool only_k_r_fpar); // cppcheck-suppress functionConst // has side effects /** * Simplify variable initialization diff --git a/test/testclass.cpp b/test/testclass.cpp index f5a323d9e..6fc136399 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -198,6 +198,7 @@ private: TEST_CASE(const85); TEST_CASE(const86); TEST_CASE(const87); + TEST_CASE(const89); TEST_CASE(const_handleDefaultParameters); TEST_CASE(const_passThisToMemberOfOtherClass); @@ -6414,6 +6415,29 @@ private: errout.str()); } + void const89() { // #11654 + checkConst("struct S {\n" + " void f(bool b);\n" + " int i;\n" + "};\n" + "void S::f(bool b) {\n" + " if (i && b)\n" + " f(false);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:2]: (style, inconclusive) Technically the member function 'S::f' can be const.\n", errout.str()); + + checkConst("struct S {\n" + " void f(int& r);\n" + " int i;\n" + "};\n" + "void S::f(int& r) {\n" + " r = 0;\n" + " if (i)\n" + " f(i);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void const_handleDefaultParameters() { checkConst("struct Foo {\n" " void foo1(int i, int j = 0) {\n"