From 33981fe42c1f7524496883ec897b3834a7adba58 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sat, 25 Nov 2023 22:58:16 +0100 Subject: [PATCH] Fix #12214 FN constParameterReference / #12216 FP constParameterReference (#5691) --- lib/astutils.cpp | 4 ++++ lib/checkother.cpp | 21 ++------------------- lib/tokenize.cpp | 9 +++------ lib/valueflow.cpp | 4 ++-- test/cfg/std.cpp | 2 +- test/cfg/wxwidgets.cpp | 2 +- test/testother.cpp | 18 ++++++++++++++++-- 7 files changed, 29 insertions(+), 31 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index c1025da349e..2da5f3f219e 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2290,6 +2290,8 @@ static T* getTokenArgumentFunctionImpl(T* tok, int& argn) tok = tok->astOperand1(); while (tok && (tok->isUnaryOp("*") || tok->str() == "[")) tok = tok->astOperand1(); + if (Token::Match(tok, ". * %name%")) // bailout for pointer to member + return tok->tokAt(2); while (Token::simpleMatch(tok, ".")) tok = tok->astOperand2(); while (Token::simpleMatch(tok, "::")) { @@ -2630,6 +2632,8 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, if (!ftok->function() || !ftok->function()->isConst()) return true; } + if (Token::Match(tok2->astParent(), ". * %name%")) // bailout + return true; if (Token::simpleMatch(tok2, "[") && astIsContainer(tok) && vt && vt->container && vt->container->stdAssociativeLike) return true; diff --git a/lib/checkother.cpp b/lib/checkother.cpp index c55219abf9a..041b9d022cc 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1390,6 +1390,8 @@ void CheckOther::checkConstVariable() continue; if (var->isVolatile()) continue; + if (var->nameToken()->isExpandedMacro()) + continue; if (isStructuredBindingVariable(var)) // TODO: check all bound variables continue; if (isVariableChanged(var, mSettings, mTokenizer->isCPP())) @@ -1461,25 +1463,6 @@ void CheckOther::checkConstVariable() if (usedInAssignment) continue; } - // Skip if we ever cast this variable to a pointer/reference to a non-const type - { - bool castToNonConst = false; - for (const Token* tok = var->nameToken(); tok != scope->bodyEnd && tok != nullptr; tok = tok->next()) { - if (tok->isCast()) { - if (!tok->valueType()) { - castToNonConst = true; // safe guess - break; - } - const bool isConst = tok->valueType()->isConst(tok->valueType()->pointer); - if (!isConst) { - castToNonConst = true; - break; - } - } - } - if (castToNonConst) - continue; - } constVariableError(var, hasFunction ? function : nullptr); } diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 26255413810..3cfef404952 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -619,7 +619,7 @@ namespace { bool mUsed = false; public: - TypedefSimplifier(Token* typedefToken, int &num) : mTypedefToken(typedefToken) { + explicit TypedefSimplifier(Token* typedefToken) : mTypedefToken(typedefToken) { Token* start = typedefToken->next(); if (Token::simpleMatch(start, "typename")) start = start->next(); @@ -641,7 +641,6 @@ namespace { mRangeTypeQualifiers = rangeQualifiers; Token* typeName = rangeBefore.second->previous(); if (typeName->isKeyword()) { - (void)num; // TODO typeName->insertToken("T:" + std::to_string(num++)); typeName->insertToken(nameToken->str()); } @@ -1048,8 +1047,7 @@ void Tokenizer::simplifyTypedef() std::map numberOfTypedefs; for (Token* tok = list.front(); tok; tok = tok->next()) { if (tok->str() == "typedef") { - int dummy = 0; - TypedefSimplifier ts(tok, dummy); + TypedefSimplifier ts(tok); if (!ts.fail()) numberOfTypedefs[ts.name()]++; continue; @@ -1057,7 +1055,6 @@ void Tokenizer::simplifyTypedef() } int indentlevel = 0; - int typeNum = 1; std::map typedefs; for (Token* tok = list.front(); tok; tok = tok->next()) { if (!tok->isName()) { @@ -1069,7 +1066,7 @@ void Tokenizer::simplifyTypedef() } if (indentlevel == 0 && tok->str() == "typedef") { - TypedefSimplifier ts(tok, typeNum); + TypedefSimplifier ts(tok); if (!ts.fail() && numberOfTypedefs[ts.name()] == 1) { if (mSettings->severity.isEnabled(Severity::portability) && ts.isInvalidConstFunctionType(typedefs)) reportError(tok->next(), Severity::portability, "invalidConstFunctionType", diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 3a375e3eef6..16d7595a9dd 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5238,7 +5238,7 @@ static const Scope* getLoopScope(const Token* tok) } // -static void valueFlowConditionExpressions(TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings &settings) +static void valueFlowConditionExpressions(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings &settings) { for (const Scope * scope : symboldatabase.functionScopes) { if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) { @@ -7482,7 +7482,7 @@ static void valueFlowInjectParameter(const TokenList& tokenlist, settings); } -static void valueFlowSwitchVariable(TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowSwitchVariable(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings *settings) { for (const Scope &scope : symboldatabase.scopeList) { if (scope.type != Scope::ScopeType::eSwitch) diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index ecabb2f95c5..b0926a0eed5 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -4206,7 +4206,7 @@ void uninivar_istream_read(std::istream &f) f.read(buffer, size); } -void uninitvar_string_compare(std::string &teststr, std::wstring &testwstr) +void uninitvar_string_compare(const std::string &teststr, const std::wstring &testwstr) { const char *pStrUninit; // cppcheck-suppress uninitvar diff --git a/test/cfg/wxwidgets.cpp b/test/cfg/wxwidgets.cpp index 0b4f85c873c..7472fc54350 100644 --- a/test/cfg/wxwidgets.cpp +++ b/test/cfg/wxwidgets.cpp @@ -29,7 +29,7 @@ #include #include -void uninitvar_wxRegEx_GetMatch(wxRegEx &obj, size_t *start, size_t *len, size_t index) +void uninitvar_wxRegEx_GetMatch(const wxRegEx &obj, size_t *start, size_t *len, size_t index) { size_t s,l; size_t *sPtr,*lPtr; diff --git a/test/testother.cpp b/test/testother.cpp index 83fad7c3111..a97f52b8dad 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2873,7 +2873,7 @@ class TestOther : public TestFixture { " x.f();\n" " foo( static_cast(0) );\n" "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Parameter 'x' can be declared as reference to const\n", errout.str()); check("class a {\n" " void foo(const int& i) const;\n" @@ -3375,6 +3375,18 @@ class TestOther : public TestFixture { "[test.cpp:1]: (style) Parameter 's2' can be declared as reference to const\n", errout.str()); + check("void f(int& r) {\n" // #12214 + " (void)(true);\n" + " if (r) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'r' can be declared as reference to const\n", errout.str()); + + check("struct S { void f(int&); };\n" // #12216 + "void g(S& s, int& r, void (S::* p2m)(int&)) {\n" + " (s.*p2m)(r);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + check("struct S {\n" " void f(int& r) { p = &r; }\n" " int* p;\n" @@ -10861,7 +10873,9 @@ class TestOther : public TestFixture { " for (auto &j : g(std::move(l))) { (void)j; }\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (warning) Access of moved variable 'l'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) Variable 'j' can be declared as reference to const\n" + "[test.cpp:4]: (warning) Access of moved variable 'l'.\n", + errout.str()); } void moveCallback()