From 9ed21fb917910f9ee8839527c5227f9a9c8a0986 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sun, 12 Mar 2023 11:39:18 +0100 Subject: [PATCH] Fix #11513 FN functionConst with comparison as argument (#4738) --- lib/checkclass.cpp | 17 +++++++++++++++-- lib/cppcheck.h | 2 +- lib/symboldatabase.h | 4 ++-- lib/tokenize.h | 4 ++-- test/testclass.cpp | 25 +++++++++++++++++++++++-- test/testsuppressions.cpp | 8 ++++---- test/testsymboldatabase.cpp | 2 +- 7 files changed, 48 insertions(+), 14 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 082a36b56..53e6cb2fd 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2306,6 +2306,19 @@ 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) + 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->valueType() || ((argVar->valueType()->pointer || argVar->valueType()->reference != Reference::None) && !argVar->isConst())) { + mayModifyArgs = true; + break; + } + } + } // Member variable given as parameter const Token *lpar = funcTok->next(); if (Token::simpleMatch(lpar, "( ) (")) @@ -2313,9 +2326,9 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& 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()))) { + else if ((tok->isName() && isMemberVar(scope, tok)) || (tok->isUnaryOp("&") && (tok = tok->astOperand1()) && isMemberVar(scope, tok))) { const Variable* var = tok->variable(); - if (!var || !var->isMutable()) + if ((!var || !var->isMutable()) && mayModifyArgs) return false; // TODO: Only bailout if function takes argument as non-const reference } } diff --git a/lib/cppcheck.h b/lib/cppcheck.h index e161f94ae..07469b8ae 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -145,7 +145,7 @@ public: bool isUnusedFunctionCheckEnabled() const; /** Remove *.ctu-info files */ - void removeCtuInfoFiles(const std::map& files); + void removeCtuInfoFiles(const std::map& files); // cppcheck-suppress functionConst // has side effects private: /** Are there "simple" rules */ diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index a28a44efc..c6d678092 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -1474,12 +1474,12 @@ private: void createSymbolDatabaseNeedInitialization(); void createSymbolDatabaseVariableSymbolTable(); void createSymbolDatabaseSetScopePointers(); - void createSymbolDatabaseSetFunctionPointers(bool firstPass); + void createSymbolDatabaseSetFunctionPointers(bool firstPass); // cppcheck-suppress functionConst // has side effects void createSymbolDatabaseSetVariablePointers(); // cppcheck-suppress functionConst void createSymbolDatabaseSetTypePointers(); void createSymbolDatabaseSetSmartPointerType(); - void createSymbolDatabaseEnums(); + void createSymbolDatabaseEnums(); // cppcheck-suppress functionConst // has side effects void createSymbolDatabaseEscapeFunctions(); // cppcheck-suppress functionConst void createSymbolDatabaseIncompleteVars(); diff --git a/lib/tokenize.h b/lib/tokenize.h index 0ba1f9705..3edd17f17 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -586,7 +586,7 @@ private: void unsupportedTypedef(const Token *tok) const; - void setVarIdClassDeclaration(const Token * const startToken, + void setVarIdClassDeclaration(const Token * const startToken, // cppcheck-suppress functionConst // has side effects VariableMap &variableMap, const nonneg int scopeStartVarId, std::map>& structMembers); @@ -595,7 +595,7 @@ private: std::map>& structMembers, nonneg int &varId) const; - void setVarIdClassFunction(const std::string &classname, + void setVarIdClassFunction(const std::string &classname, // cppcheck-suppress functionConst // has side effects Token * const startToken, const Token * const endToken, const std::map &varlist, diff --git a/test/testclass.cpp b/test/testclass.cpp index cc784f958..7d6ce802c 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -195,7 +195,9 @@ private: TEST_CASE(const79); // ticket #9861 TEST_CASE(const80); // ticket #11328 TEST_CASE(const81); // ticket #11330 - TEST_CASE(const82); + TEST_CASE(const82); // ticket #11513 + TEST_CASE(const83); + TEST_CASE(const_handleDefaultParameters); TEST_CASE(const_passThisToMemberOfOtherClass); TEST_CASE(assigningPointerToPointerIsNotAConstOperation); @@ -6322,7 +6324,26 @@ private: ASSERT_EQUALS("", errout.str()); } - void const82() { + void const82() { // #11513 + checkConst("struct S {\n" + " int i;\n" + " void h(bool) const;\n" + " void g() { h(i == 1); }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Technically the member function 'S::g' can be const.\n", + errout.str()); + + checkConst("struct S {\n" + " int i;\n" + " 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()); + } + + void const83() { checkConst("struct S {\n" " int i1, i2;\n" " void f(bool b);\n" diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 416108fdd..4c77752e4 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -561,7 +561,7 @@ private: ASSERT_EQUALS(true, suppressions6.isSuppressed(errorMessage("abc", "test.cpp", 123))); } - void inlinesuppress() { + void inlinesuppress() const { Suppressions::Suppression s; std::string msg; ASSERT_EQUALS(false, s.parseComment("/* some text */", &msg)); @@ -593,7 +593,7 @@ private: ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: a\n", errout.str()); } - void inlinesuppress_comment() { + void inlinesuppress_comment() const { Suppressions::Suppression s; std::string errMsg; ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc ; some comment", &errMsg)); @@ -604,7 +604,7 @@ private: ASSERT_EQUALS("", errMsg); } - void multi_inlinesuppress() { + void multi_inlinesuppress() const { std::vector suppressions; std::string errMsg; @@ -669,7 +669,7 @@ private: ASSERT_EQUALS(false, errMsg.empty()); } - void multi_inlinesuppress_comment() { + void multi_inlinesuppress_comment() const { std::vector suppressions; std::string errMsg; diff --git a/test/testsymboldatabase.cpp b/test/testsymboldatabase.cpp index 0d4990bf3..959977bef 100644 --- a/test/testsymboldatabase.cpp +++ b/test/testsymboldatabase.cpp @@ -7171,7 +7171,7 @@ private: ASSERT(tok->tokAt(2)->function()); } - void valueTypeMatchParameter() { + void valueTypeMatchParameter() const { ValueType vt_int(ValueType::Sign::SIGNED, ValueType::Type::INT, 0); ValueType vt_const_int(ValueType::Sign::SIGNED, ValueType::Type::INT, 0, 1); ASSERT_EQUALS((int)ValueType::MatchResult::SAME, (int)ValueType::matchParameter(&vt_int, &vt_int));