From acbe66c3ebcf329bad8120b44c43921ae9122656 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sun, 5 Nov 2023 20:25:43 +0100 Subject: [PATCH 1/4] Fix #12147 false negative: passedByValue --- lib/astutils.cpp | 4 ++-- test/testother.cpp | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 64c5a74a3c6..5b69cfcd7b9 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2579,8 +2579,8 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, if (indirect == 0 && astIsPointer(tok)) return false; - const Token *ftok = tok->tokAt(2); - if (astIsContainer(tok) && vt && vt->container) { + const Token *ftok = tok2->astParent()->next(); + if (astIsContainer(tok2->astParent()->previous()) && vt && vt->container) { const Library::Container* c = vt->container; const Library::Container::Action action = c->getAction(ftok->str()); if (contains({Library::Container::Action::INSERT, diff --git a/test/testother.cpp b/test/testother.cpp index 62173127cf3..a46c2138fad 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2172,6 +2172,18 @@ class TestOther : public TestFixture { "}\n"); ASSERT_EQUALS("[test.cpp:1]: (performance) Function parameter 't' should be passed by const reference.\n", errout.str()); + check("struct S { std::list l; };\n" // #12147 + "class C { public: std::list l; };\n" + "bool f(S s) {\n" + " return s.l.empty();\n" + "}\n" + "bool f(C c) {\n" + " return c.l.empty();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (performance) Function parameter 's' should be passed by const reference.\n" + "[test.cpp:6]: (performance) Function parameter 'c' should be passed by const reference.\n", + errout.str()); + Settings settings1 = settingsBuilder().platform(Platform::Type::Win64).build(); check("using ui64 = unsigned __int64;\n" "ui64 Test(ui64 one, ui64 two) { return one + two; }\n", From 32d4bd313cc5dad99e290fbca0a994d9f09d0f7f Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sun, 5 Nov 2023 21:16:35 +0100 Subject: [PATCH 2/4] Use STL algo --- lib/checkclass.cpp | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index b69010bd9b4..676d69733b4 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -659,12 +659,11 @@ std::vector CheckClass::createUsageList(const Scope *scope) void CheckClass::assignVar(std::vector &usageList, nonneg int varid) { - for (Usage& usage: usageList) { - if (usage.var->declarationId() == varid) { - usage.assign = true; - return; - } - } + auto it = std::find_if(usageList.begin(), usageList.end(), [varid](const Usage& usage) { + return usage.var->declarationId() == varid; + }); + if (it != usageList.end()) + it->assign = true; } void CheckClass::assignVar(std::vector &usageList, const Token* vartok) @@ -673,23 +672,21 @@ void CheckClass::assignVar(std::vector &usageList, const Token* vartok) assignVar(usageList, vartok->varId()); return; } - for (Usage& usage: usageList) { + auto it = std::find_if(usageList.begin(), usageList.end(), [vartok](const Usage& usage) { // FIXME: This is a workaround when varid is not set for a derived member - if (usage.var->name() == vartok->str()) { - usage.assign = true; - return; - } - } + return usage.var->name() == vartok->str(); + }); + if (it != usageList.end()) + it->assign = true; } void CheckClass::initVar(std::vector &usageList, nonneg int varid) { - for (Usage& usage: usageList) { - if (usage.var->declarationId() == varid) { - usage.init = true; - return; - } - } + auto it = std::find_if(usageList.begin(), usageList.end(), [varid](const Usage& usage) { + return usage.var->declarationId() == varid; + }); + if (it != usageList.end()) + it->init = true; } void CheckClass::assignAllVar(std::vector &usageList) From e0b99a3c4d2fc86e34b11483a6f8b13decf68053 Mon Sep 17 00:00:00 2001 From: chrchr Date: Mon, 6 Nov 2023 11:27:32 +0100 Subject: [PATCH 3/4] Use AST --- lib/astutils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 5b69cfcd7b9..e28993f76e7 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2579,8 +2579,8 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, if (indirect == 0 && astIsPointer(tok)) return false; - const Token *ftok = tok2->astParent()->next(); - if (astIsContainer(tok2->astParent()->previous()) && vt && vt->container) { + const Token *ftok = tok2->astParent()->astOperand2(); + if (astIsContainer(tok2->astParent()->astOperand1()) && vt && vt->container) { const Library::Container* c = vt->container; const Library::Container::Action action = c->getAction(ftok->str()); if (contains({Library::Container::Action::INSERT, From b450d82d824415f8fa277245ef35fe4e3edde698 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 6 Nov 2023 22:09:58 +0100 Subject: [PATCH 4/4] Add test --- test/testother.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/testother.cpp b/test/testother.cpp index 11b077686d2..0615492fbdb 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2203,6 +2203,13 @@ class TestOther : public TestFixture { "[test.cpp:6]: (performance) Function parameter 'c' should be passed by const reference.\n", errout.str()); + check("struct S { std::list a[1][1]; };\n" + "bool f(S s) {\n" + " return s.a[0][0].empty();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (performance) Function parameter 's' should be passed by const reference.\n", + errout.str()); + Settings settings1 = settingsBuilder().platform(Platform::Type::Win64).build(); check("using ui64 = unsigned __int64;\n" "ui64 Test(ui64 one, ui64 two) { return one + two; }\n",