diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index f96bf2a63..207458b3f 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2119,6 +2119,16 @@ void CheckClass::checkConst() } } +// tok should point at "this" +static const Token* getFuncTokFromThis(const Token* tok) { + if (!Token::simpleMatch(tok->next(), ".")) + return nullptr; + tok = tok->tokAt(2); + while (Token::Match(tok, "%name% ::")) + tok = tok->tokAt(2); + return Token::Match(tok, "%name% (") ? tok : nullptr; +} + bool CheckClass::isMemberVar(const Scope *scope, const Token *tok) const { bool again = false; @@ -2128,7 +2138,7 @@ bool CheckClass::isMemberVar(const Scope *scope, const Token *tok) const again = false; if (tok->str() == "this") { - return true; + return !getFuncTokFromThis(tok); // function calls are handled elsewhere } else if (Token::simpleMatch(tok->tokAt(-3), "( * this )")) { return true; } else if (Token::Match(tok->tokAt(-3), "%name% ) . %name%")) { @@ -2264,6 +2274,14 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& if (mTokenizer->hasIfdef(func->functionScope->bodyStart, func->functionScope->bodyEnd)) return false; + auto getFuncTok = [](const Token* tok) -> const Token* { + if (Token::simpleMatch(tok, "this")) + tok = getFuncTokFromThis(tok); + if ((Token::Match(tok, "%name% (|{") || Token::simpleMatch(tok->astParent(), "return {")) && !tok->isStandardType() && !tok->isKeyword()) + return tok; + return nullptr; + }; + // if the function doesn't have any assignment nor function call, // it can be a const function.. for (const Token *tok1 = func->functionScope->bodyStart; tok1 && tok1 != func->functionScope->bodyEnd; tok1 = tok1->next()) { @@ -2400,18 +2418,17 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& } // function/constructor call, return init list - else if ((Token::Match(tok1, "%name% (|{") || Token::simpleMatch(tok1->astParent(), "return {")) && !tok1->isStandardType() && - !Token::Match(tok1, "return|if|string|switch|while|catch|for")) { - if (isMemberFunc(scope, tok1) && tok1->strAt(-1) != ".") { - if (!isConstMemberFunc(scope, tok1)) + else if (const Token* funcTok = getFuncTok(tok1)) { + if (isMemberFunc(scope, funcTok) && (funcTok->strAt(-1) != "." || Token::simpleMatch(funcTok->tokAt(-2), "this ."))) { + if (!isConstMemberFunc(scope, funcTok)) return false; memberAccessed = true; } // Member variable given as parameter - const Token *lpar = tok1->next(); + const Token *lpar = funcTok->next(); if (Token::simpleMatch(lpar, "( ) (")) lpar = lpar->tokAt(2); - for (const Token* tok2 = lpar->next(); tok2 && tok2 != tok1->next()->link(); tok2 = tok2->next()) { + for (const Token* tok2 = lpar->next(); tok2 && tok2 != funcTok->next()->link(); tok2 = tok2->next()) { if (tok2->str() == "(") tok2 = tok2->link(); else if ((tok2->isName() && isMemberVar(scope, tok2)) || (tok2->isUnaryOp("&") && (tok2 = tok2->astOperand1()))) { diff --git a/test/testclass.cpp b/test/testclass.cpp index 0f857e2ac..e0bf7916a 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -196,6 +196,7 @@ private: TEST_CASE(const77); // ticket #10307, #10311 TEST_CASE(const78); // ticket #10315 TEST_CASE(const79); // ticket #9861 + TEST_CASE(const80); // ticket #11328 TEST_CASE(const_handleDefaultParameters); TEST_CASE(const_passThisToMemberOfOtherClass); TEST_CASE(assigningPointerToPointerIsNotAConstOperation); @@ -6152,6 +6153,40 @@ private: errout.str()); } + void const80() { // #11328 + checkConst("struct B { static void b(); };\n" + "struct S : B {\n" + " static void f() {}\n" + " void g() const;\n" + " void h();\n" + " void k();\n" + " void m();\n" + " void n();\n" + " int i;\n" + "};\n" + "void S::g() const {\n" + " this->f();\n" + "}\n" + "void S::h() {\n" + " this->f();\n" + "}\n" + "void S::k() {\n" + " if (i)\n" + " this->f();\n" + "}\n" + "void S::m() {\n" + " this->B::b();\n" + "}\n" + "void S::n() {\n" + " this->h();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:11] -> [test.cpp:4]: (performance, inconclusive) Technically the member function 'S::g' can be static (but you may consider moving to unnamed namespace).\n" + "[test.cpp:14] -> [test.cpp:5]: (performance, inconclusive) Technically the member function 'S::h' can be static (but you may consider moving to unnamed namespace).\n" + "[test.cpp:17] -> [test.cpp:6]: (style, inconclusive) Technically the member function 'S::k' can be const.\n" + "[test.cpp:21] -> [test.cpp:7]: (performance, inconclusive) Technically the member function 'S::m' can be static (but you may consider moving to unnamed namespace).\n", + errout.str()); + } + void const_handleDefaultParameters() { checkConst("struct Foo {\n" " void foo1(int i, int j = 0) {\n"