From 215124461e7ac39d7d940376cb9fe6d73f2a4037 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Thu, 2 Mar 2023 21:51:58 +0100 Subject: [PATCH] Fix #11499 FN functionConst with operator usage (#4722) --- lib/checkclass.cpp | 67 ++++++++++++++++++++---------- lib/checkmemoryleak.cpp | 2 +- lib/checkmemoryleak.h | 2 +- lib/forwardanalyzer.cpp | 4 +- lib/reverseanalyzer.cpp | 4 +- test/testclass.cpp | 92 ++++++++++++++++++++++++++++++++++++++++- 6 files changed, 142 insertions(+), 29 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 115432515..80d9e110d 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2291,11 +2291,37 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& 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()) + bool isReturn = false; + if ((Token::Match(tok, "%name% (|{") || (isReturn = Token::simpleMatch(tok->astParent(), "return {"))) && !tok->isStandardType() && !tok->isKeyword()) { + if (isReturn) + tok = tok->astParent(); return tok; + } return nullptr; }; + auto checkFuncCall = [this, &memberAccessed](const Token* funcTok, const Scope* scope) { + 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 = funcTok->next(); + if (Token::simpleMatch(lpar, "( ) (")) + lpar = lpar->tokAt(2); + for (const Token* tok = lpar->next(); tok && tok != funcTok->next()->link(); tok = tok->next()) { + if (tok->str() == "(") + tok = tok->link(); + else if ((tok->isName() && isMemberVar(scope, tok)) || (tok->isUnaryOp("&") && (tok = tok->astOperand1()))) { + const Variable* var = tok->variable(); + if (!var || !var->isMutable()) + return false; // TODO: Only bailout if function takes argument as non-const reference + } + } + return true; + }; + // 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()) { @@ -2374,6 +2400,18 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& break; } + auto hasOverloadedMemberAccess = [](const Token* end, const Scope* scope) -> bool { + if (!end || !scope || !Token::simpleMatch(end->astParent(), ".")) + return false; + auto it = std::find_if(scope->functionList.begin(), scope->functionList.end(), [](const Function& f) { + return f.isConst() && f.name() == "operator."; + }); + if (it == scope->functionList.end() || !it->retType || !it->retType->classScope) + return false; + const Function* func = it->retType->classScope->findFunction(end, /*requireConst*/ true); + return func && func->isConst(); + }; + if (end->strAt(1) == "(") { const Variable *var = lastVarTok->variable(); if (!var) @@ -2389,6 +2427,9 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& ; else if (var->smartPointerType() && var->smartPointerType()->classScope && isConstMemberFunc(var->smartPointerType()->classScope, end)) { ; + } + else if (hasOverloadedMemberAccess(end, var->typeScope())) { + ; } else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end)) return false; } @@ -2416,8 +2457,8 @@ 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% ( !!)")) - tok1 = tok1->previous(); // check function call + if (tok1 == end && Token::Match(end->previous(), ". %name% ( !!)") && !checkFuncCall(tok1, scope)) // function call on member + return false; } // streaming: << @@ -2435,24 +2476,8 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& // function/constructor call, return init list 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 = funcTok->next(); - if (Token::simpleMatch(lpar, "( ) (")) - lpar = lpar->tokAt(2); - 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()))) { - const Variable* var = tok2->variable(); - if (!var || !var->isMutable()) - return false; // TODO: Only bailout if function takes argument as non-const reference - } - } + if (!checkFuncCall(funcTok, scope)) + 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/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index cb8560c50..f7cbece50 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -452,7 +452,7 @@ bool CheckMemoryLeakInFunction::test_white_list(const std::string &funcname, con // a = malloc(10); a = realloc(a, 100); //--------------------------------------------------------------------------- -void CheckMemoryLeakInFunction::checkReallocUsage() +void CheckMemoryLeakInFunction::checkReallocUsage() const { // only check functions const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index f2dc42ecc..de78f308c 100644 --- a/lib/checkmemoryleak.h +++ b/lib/checkmemoryleak.h @@ -183,7 +183,7 @@ public: /** * Checking for a memory leak caused by improper realloc usage. */ - void checkReallocUsage(); + void checkReallocUsage() const; private: /** Report all possible errors (for the --errorlist) */ diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 31771d206..3a2c32075 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -288,7 +288,7 @@ struct ForwardTraversal { return ft; } - std::vector tryForkScope(Token* endBlock, bool isModified = false) { + std::vector tryForkScope(Token* endBlock, bool isModified = false) const { if (analyzer->updateScope(endBlock, isModified)) { ForwardTraversal ft = fork(); return {std::move(ft)}; @@ -296,7 +296,7 @@ struct ForwardTraversal { return std::vector {}; } - std::vector tryForkUpdateScope(Token* endBlock, bool isModified = false) { + std::vector tryForkUpdateScope(Token* endBlock, bool isModified = false) const { std::vector result = tryForkScope(endBlock, isModified); for (ForwardTraversal& ft : result) ft.updateScope(endBlock); diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 31512c868..bb674e621 100644 --- a/lib/reverseanalyzer.cpp +++ b/lib/reverseanalyzer.cpp @@ -42,7 +42,7 @@ struct ReverseTraversal { ValuePtr analyzer; const Settings* settings; - std::pair evalCond(const Token* tok) { + std::pair evalCond(const Token* tok) const { std::vector result = analyzer->evaluate(tok); // TODO: We should convert to bool const bool checkThen = std::any_of(result.cbegin(), result.cend(), [](int x) { @@ -142,7 +142,7 @@ struct ReverseTraversal { return result; } - Token* isDeadCode(Token* tok, const Token* end = nullptr) { + Token* isDeadCode(Token* tok, const Token* end = nullptr) const { int opSide = 0; for (; tok && tok->astParent(); tok = tok->astParent()) { if (tok == end) diff --git a/test/testclass.cpp b/test/testclass.cpp index eccc6b252..373c85fbe 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -6221,8 +6221,8 @@ private: errout.str()); } - void const81() { // #11330 - checkConst("struct A {\n" + void const81() { + checkConst("struct A {\n" // #11330 " bool f() const;\n" "};\n" "struct S {\n" @@ -6233,6 +6233,94 @@ private: "};\n"); ASSERT_EQUALS("[test.cpp:6]: (style, inconclusive) Technically the member function 'S::g' can be const.\n", errout.str()); + + checkConst("struct A {\n" // #11499 + " void f() const;\n" + "};\n" + "template\n" + "struct P {\n" + " T* operator->();\n" + " const T* operator->() const;\n" + "};\n" + "struct S {\n" + " P p;\n" + " void g() { p->f(); }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:11]: (style, inconclusive) Technically the member function 'S::g' can be const.\n", + errout.str()); + + checkConst("struct A {\n" + " void f(int) const;\n" + "};\n" + "template\n" + "struct P {\n" + " T* operator->();\n" + " const T* operator->() const;\n" + "};\n" + "struct S {\n" + " P p;\n" + " void g() { p->f(1); }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:11]: (style, inconclusive) Technically the member function 'S::g' can be const.\n", errout.str()); + + checkConst("struct A {\n" + " void f(void*) const;\n" + "};\n" + "template\n" + "struct P {\n" + " T* operator->();\n" + " const T* operator->() const;\n" + " P& operator=(P) {\n" + " return *this;\n" + " }\n" + "};\n" + "struct S {\n" + " P p;\n" + " std::vector g() { p->f(nullptr); return {}; }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:14]: (style, inconclusive) Technically the member function 'S::g' can be const.\n", errout.str()); + + checkConst("struct A {\n" + " void f();\n" + "};\n" + "template\n" + "struct P {\n" + " T* operator->();\n" + " const T* operator->() const;\n" + "};\n" + "struct S {\n" + " P p;\n" + " void g() { p->f(); }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + checkConst("struct A {\n" + " void f() const;\n" + "};\n" + "template\n" + "struct P {\n" + " T* operator->();\n" + "};\n" + "struct S {\n" + " P p;\n" + " void g() { p->f(); }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + checkConst("struct A {\n" + " void f(int&) const;\n" + "};\n" + "template\n" + "struct P {\n" + " T* operator->();\n" + " const T* operator->() const;\n" + "};\n" + "struct S {\n" + " P p;\n" + " int i;\n" + " void g() { p->f(i); }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void const_handleDefaultParameters() {