From 3836367d95fa86836ef512405e61090116ec5ed8 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Thu, 30 Mar 2023 07:24:23 +0200 Subject: [PATCH] Fix FN passedByValue with array access, range-based for (#4922) * Fix FN passedByValue with array access, range-based for * Format * Fix/suppress new warnings --- gui/checkthread.cpp | 2 +- gui/mainwindow.cpp | 2 +- gui/mainwindow.h | 2 +- lib/checkother.cpp | 21 ++++++++++++++++++--- lib/importproject.cpp | 2 +- lib/importproject.h | 2 +- test/testother.cpp | 32 +++++++++++++++++++++++++++++++- 7 files changed, 54 insertions(+), 9 deletions(-) diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp index ca95d8d62..5afa7f4a7 100644 --- a/gui/checkthread.cpp +++ b/gui/checkthread.cpp @@ -45,7 +45,7 @@ #include // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature -static bool executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output) +static bool executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output) // cppcheck-suppress passedByValue { output.clear(); diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index 076ef2a63..e3f3631f3 100644 --- a/gui/mainwindow.cpp +++ b/gui/mainwindow.cpp @@ -1158,7 +1158,7 @@ void MainWindow::checkConfiguration() analyzeProject(mProjectFile, false, true); } -void MainWindow::reAnalyzeSelected(QStringList files) +void MainWindow::reAnalyzeSelected(const QStringList& files) { if (files.empty()) return; diff --git a/gui/mainwindow.h b/gui/mainwindow.h index 540bdf869..3597f3ecd 100644 --- a/gui/mainwindow.h +++ b/gui/mainwindow.h @@ -251,7 +251,7 @@ private: * @brief Reanalyze selected files * @param files list of selected files */ - void reAnalyzeSelected(QStringList files); + void reAnalyzeSelected(const QStringList& files); /** * @brief Analyze the project. diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a90adbed5..07e3cf50b 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1176,6 +1176,14 @@ static int estimateSize(const Type* type, const Settings* settings, const Symbol }); } +static bool isConstRangeBasedFor(const Token* tok) { + if (astIsRangeBasedForDecl(tok)) { + const Variable* loopVar = tok->astParent()->astOperand1()->variable(); + return loopVar && (!loopVar->isReference() || loopVar->isConst()); + } + return false; +} + static bool canBeConst(const Variable *var, const Settings* settings) { if (!var->scope()) @@ -1201,6 +1209,8 @@ static bool canBeConst(const Variable *var, const Settings* settings) continue; const Token* parent = tok2->astParent(); + while (Token::simpleMatch(parent, "[")) + parent = parent->astParent(); if (!parent) continue; if (Token::simpleMatch(tok2->next(), ";") && tok2->next()->isSplittedVarDeclEq()) { @@ -1249,9 +1259,12 @@ static bool canBeConst(const Variable *var, const Settings* settings) (parent->astOperand2() && settings->library.isFunctionConst(parent->astOperand2()))) continue; else if (parent->isAssignmentOp()) { - if (parent->astOperand1() == tok2) + const Token* assignee = parent->astOperand1(); + while (Token::simpleMatch(assignee, "[")) + assignee = assignee->astOperand1(); + if (assignee == tok2) return false; - const Variable* assignedVar = parent->astOperand1() ? parent->astOperand1()->variable() : nullptr; + const Variable* assignedVar = assignee ? assignee->variable() : nullptr; if (assignedVar && !assignedVar->isConst() && assignedVar->isReference() && @@ -1263,7 +1276,9 @@ static bool canBeConst(const Variable *var, const Settings* settings) continue; else return false; - } else + } else if (isConstRangeBasedFor(tok2)) + continue; + else return false; } diff --git a/lib/importproject.cpp b/lib/importproject.cpp index e5af3ff51..714b7a5dc 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -274,7 +274,7 @@ static std::string unescape(const std::string &in) return out; } -void ImportProject::FileSettings::parseCommand(std::string command) +void ImportProject::FileSettings::parseCommand(const std::string& command) { std::string defs; diff --git a/lib/importproject.h b/lib/importproject.h index 94527f970..81077b77b 100644 --- a/lib/importproject.h +++ b/lib/importproject.h @@ -78,7 +78,7 @@ public: bool msc; bool useMfc; - void parseCommand(std::string command); + void parseCommand(const std::string& command); void setDefines(std::string defs); void setIncludePaths(const std::string &basepath, const std::list &in, std::map &variables); }; diff --git a/test/testother.cpp b/test/testother.cpp index e885945f0..d122c4c55 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -1993,6 +1993,36 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + check("int x(int);\n" + "void f(std::vector v, int& j) {\n" + " for (int i : v)\n" + " j = i;\n" + "}\n" + "void fn(std::vector v) {\n" + " for (int& i : v)\n" + " i = x(i);\n" + "}\n" + "void g(std::vector v, int& j) {\n" + " for (int i = 0; i < v.size(); ++i)\n" + " j = v[i];\n" + "}\n" + "void gn(std::vector v) {\n" + " for (int i = 0; i < v.size(); ++i)\n" + " v[i] = x(i);\n" + "}\n" + "void h(std::vector> v, int& j) {\n" + " for (int i = 0; i < v.size(); ++i)\n" + " j = v[i][0];\n" + "}\n" + "void hn(std::vector> v) {\n" + " for (int i = 0; i < v.size(); ++i)\n" + " v[i][0] = x(i);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (performance) Function parameter 'v' should be passed by const reference.\n" + "[test.cpp:10]: (performance) Function parameter 'v' should be passed by const reference.\n" + "[test.cpp:18]: (performance) Function parameter 'v' should be passed by const reference.\n", + errout.str()); + Settings settings1; PLATFORM(settings1.platform, cppcheck::Platform::Type::Win64); check("using ui64 = unsigned __int64;\n" @@ -2194,7 +2224,7 @@ private: " const int& i = x[0];\n" " return i;\n" "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (performance) Function parameter 'x' should be passed by const reference.\n", errout.str()); check("int f(std::vector x) {\n" " static int& i = x[0];\n"