From 95b2eba388c2c19d587a86fd9f3f140b9e35b9c6 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 8 Mar 2023 08:12:06 -0600 Subject: [PATCH 1/6] Fix 11605: FN useStlAlgo with multiple conditions --- lib/checkstl.cpp | 132 +++++++++++++++++++++++++++++++++++++++++++++++ test/teststl.cpp | 111 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 239 insertions(+), 4 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 027f22f8c93..892c31336f4 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -2670,6 +2670,131 @@ static std::string minmaxCompare(const Token *condTok, nonneg int loopVar, nonne return algo; } +namespace { +struct LoopAnalyzer { + const Token* bodyTok = nullptr; + const Token* loopVar = nullptr; + const Settings* settings = nullptr; + std::set varsChanged = {}; + + explicit LoopAnalyzer(const Token* tok, const Settings* psettings) : bodyTok(tok->next()->link()->next()), settings(psettings) + { + const Token *splitTok = tok->next()->astOperand2(); + if (Token::simpleMatch(splitTok, ":") && splitTok->previous()->varId() != 0) { + loopVar = splitTok->previous(); + } + if (valid()) { + findChangedVariables(); + } + } + bool isLoopVarChanged() const { + return varsChanged.count(loopVar->varId()) > 0; + } + + bool isModified(const Token* tok) const { + if (tok->variable() && tok->variable()->isConst()) + return false; + int n = 1 + (astIsPointer(tok) ? 1 : 0); + for(int i=0;i + void findTokens(Predicate pred, F f) const { + for(const Token* tok=bodyTok;precedes(tok, bodyTok->link());tok = tok->next()) { + if (pred(tok)) + f(tok); + } + } + + template + const Token* findToken(Predicate pred) const { + for(const Token* tok=bodyTok;precedes(tok, bodyTok->link());tok = tok->next()) { + if (pred(tok)) + return tok; + } + return nullptr; + } + + bool hasGotoOrBreak() const { + return findToken([](const Token* tok) { + return Token::Match(tok, "goto|break"); + }); + } + + bool valid() const { + return bodyTok && loopVar; + } + + std::string findAlgo() const { + if(!valid()) + return ""; + bool loopVarChanged = isLoopVarChanged(); + if (!loopVarChanged && varsChanged.empty()) { + if (hasGotoOrBreak()) + return ""; + bool alwaysTrue = true; + bool alwaysFalse = true; + auto hasReturn = [](const Token* tok) { return Token::simpleMatch(tok, "return"); }; + findTokens(hasReturn, [&](const Token* tok) { + const Token* returnTok = tok->astOperand1(); + if (!returnTok || !returnTok->hasKnownIntValue() || !astIsBool(returnTok)) { + alwaysTrue = false; + alwaysFalse = false; + return; + } + (returnTok->values().front().intvalue ? alwaysTrue : alwaysFalse) &= true; + (returnTok->values().front().intvalue ? alwaysFalse : alwaysTrue) &= false; + }); + if (alwaysTrue == alwaysFalse) + return ""; + if (alwaysTrue) + return "std::any_of"; + else + return "std::all_of or std::none_of"; + } + return ""; + } + + bool isLocalVar(const Variable* var) const { + if (!var) + return false; + if (var->isPointer() || var->isReference()) + return false; + if (var->declarationId() == loopVar->varId()) + return false; + const Scope* scope = var->scope(); + return scope->isNestedIn(bodyTok->scope()); + } +private: + void findChangedVariables() { + std::set vars; + for(const Token* tok=bodyTok;precedes(tok, bodyTok->link());tok = tok->next()) { + if (tok->varId() == 0) + continue; + if (vars.count(tok->varId()) > 0) + continue; + if (isLocalVar(tok->variable())) { + vars.insert(tok->varId()); + continue; + } + if (!isModified(tok)) + continue; + varsChanged.insert(tok->varId()); + vars.insert(tok->varId()); + } + } +}; +} + void CheckStl::useStlAlgorithm() { if (!mSettings->severity.isEnabled(Severity::style)) @@ -2694,6 +2819,13 @@ void CheckStl::useStlAlgorithm() continue; if (!Token::simpleMatch(tok->next()->link(), ") {")) continue; + LoopAnalyzer a{tok, mSettings}; + std::string algoName = a.findAlgo(); + if (!algoName.empty()) { + useStlAlgorithmError(tok, algoName); + continue; + } + const Token *bodyTok = tok->next()->link()->next(); const Token *splitTok = tok->next()->astOperand2(); const Token* loopVar{}; diff --git a/test/teststl.cpp b/test/teststl.cpp index c519c184c13..6be0626bdc3 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -170,6 +170,7 @@ class TestStl : public TestFixture { TEST_CASE(loopAlgoIncrement); TEST_CASE(loopAlgoConditional); TEST_CASE(loopAlgoMinMax); + TEST_CASE(loopAlgoMultipleReturn); TEST_CASE(invalidContainer); TEST_CASE(invalidContainerLoop); @@ -574,7 +575,7 @@ class TestStl : public TestFixture { " return true;\n" " return false;\n" "}\n"); - ASSERT_EQUALS("test.cpp:6:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + ASSERT_EQUALS("test.cpp:5:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); checkNormal("std::vector range(int n);\n" "bool f(bool b) {\n" @@ -587,7 +588,7 @@ class TestStl : public TestFixture { " return true;\n" " return false;\n" "}\n"); - ASSERT_EQUALS("test.cpp:8:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + ASSERT_EQUALS("test.cpp:7:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); checkNormal("bool g();\n" "int f(int x) {\n" @@ -5120,7 +5121,7 @@ class TestStl : public TestFixture { " return true;\n" "}\n", true); - ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::all_of or std::none_of algorithm instead of a raw loop.\n", errout.str()); check("bool pred(int x);\n" "bool foo() {\n" @@ -5293,7 +5294,7 @@ class TestStl : public TestFixture { " }\n" " return false;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); } void loopAlgoMinMax() { @@ -5338,6 +5339,108 @@ class TestStl : public TestFixture { ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout.str()); } + void loopAlgoMultipleReturn() { + check("bool f(const std::vector& v) {\n" +" for (auto i : v) {\n" +" if (i < 0)\n" +" continue;\n" +" if (i)\n" +" return true;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + + check("bool g(const std::vector& v) {\n" +" for (auto i : v) {\n" +" if (i % 5 == 0)\n" +" return true;\n" +" if (i % 7 == 0)\n" +" return true;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + + +check("bool g(const std::vector& v) {\n" +" for (auto i : v) {\n" +" if (i % 5 == 0)\n" +" return false;\n" +" if (i % 7 == 0)\n" +" return true;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("bool g(std::vector& v) {\n" +" for (auto& i : v) {\n" +" if (i % 5 == 0)\n" +" return false;\n" +" if (i % 7 == 0)\n" +" i++;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("bool g(const std::vector& v, int& j) {\n" +" for (auto i : v) {\n" +" if (i % 5 == 0)\n" +" return false;\n" +" if (i % 7 == 0)\n" +" j++;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("bool g(const std::vector& v, int& j) {\n" +" for (auto i : v) {\n" +" int& k = j;\n" +" if (i % 5 == 0)\n" +" return false;\n" +" if (i % 7 == 0)\n" +" k++;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("bool g(const std::vector& v, int& j) {\n" +" for (auto i : v) {\n" +" int* k = &j;\n" +" if (i % 5 == 0)\n" +" return false;\n" +" if (i % 7 == 0)\n" +" (*k)++;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("bool g(const std::vector& v, int j) {\n" +" for (auto i : v) {\n" +" int k = j;\n" +" if (i % 5 == 0)\n" +" return false;\n" +" if (i % 7 == 0)\n" +" k++;\n" +" }\n" +" return false;\n" +"}\n", + true); + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::all_of or std::none_of algorithm instead of a raw loop.\n", errout.str()); + } + void invalidContainer() { check("void f(std::vector &v) {\n" " auto v0 = v.begin();\n" From 84866a9a339321b851f8b6977d27a5710132629d Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 8 Mar 2023 08:29:42 -0600 Subject: [PATCH 2/6] Use any_of --- lib/astutils.cpp | 8 +++++--- lib/checkother.cpp | 6 +++--- lib/checkstl.cpp | 11 +++++------ 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 9dcf6f4b2ad..6c03c59e887 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1981,9 +1981,9 @@ bool isUniqueExpression(const Token* tok) if (!scope) return true; const std::string returnType = fun->retType ? fun->retType->name() : fun->retDef->stringifyList(fun->tokenDef); - for (const Function& f:scope->functionList) { + if (not std::all_of(scope->functionList.begin(), scope->functionList.end(), [&](const Function& f) { if (f.type != Function::eFunction) - continue; + return true; const std::string freturnType = f.retType ? f.retType->name() : f.retDef->stringifyList(f.returnDefEnd()); if (f.argumentList.size() == fun->argumentList.size() && @@ -1991,7 +1991,9 @@ bool isUniqueExpression(const Token* tok) f.name() != fun->name()) { return false; } - } + return true; + })) + return false; } else if (tok->variable()) { const Variable * var = tok->variable(); const Scope * scope = var->scope(); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index d9453cc0a94..ae3d33a1561 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2780,7 +2780,7 @@ void CheckOther::pointerPositiveError(const Token *tok, const ValueFlow::Value * /* check if a constructor in given class scope takes a reference */ static bool constructorTakesReference(const Scope * const classScope) { - for (const Function &constructor : classScope->functionList) { + return std::any_of(classScope->functionList.begin(), classScope->functionList.end(), [&](const Function& constructor) { if (constructor.isConstructor()) { for (int argnr = 0U; argnr < constructor.argCount(); argnr++) { const Variable * const argVar = constructor.getArgumentVar(argnr); @@ -2789,8 +2789,8 @@ static bool constructorTakesReference(const Scope * const classScope) } } } - } - return false; + return false; + }); } //--------------------------------------------------------------------------- diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 892c31336f4..fc3e7cce664 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -3012,16 +3012,15 @@ static bool isKnownEmptyContainer(const Token* tok) { if (!tok) return false; - for (const ValueFlow::Value& v:tok->values()) { + return std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) { if (!v.isKnown()) - continue; + return false; if (!v.isContainerSizeValue()) - continue; + return false; if (v.intvalue != 0) - continue; + return false; return true; - } - return false; + }); } void CheckStl::knownEmptyContainer() From 6b4a344956e82e3ad5c6a12afcca75de3ac1a45d Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 8 Mar 2023 08:30:07 -0600 Subject: [PATCH 3/6] Format --- lib/astutils.cpp | 3 +- lib/checkstl.cpp | 221 +++++++++++++++++++++++++---------------------- test/teststl.cpp | 157 +++++++++++++++++---------------- 3 files changed, 198 insertions(+), 183 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 6c03c59e887..85d82f109f8 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1986,8 +1986,7 @@ bool isUniqueExpression(const Token* tok) return true; const std::string freturnType = f.retType ? f.retType->name() : f.retDef->stringifyList(f.returnDefEnd()); - if (f.argumentList.size() == fun->argumentList.size() && - returnType == freturnType && + if (f.argumentList.size() == fun->argumentList.size() && returnType == freturnType && f.name() != fun->name()) { return false; } diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index fc3e7cce664..ba12f466421 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -2671,129 +2671,140 @@ static std::string minmaxCompare(const Token *condTok, nonneg int loopVar, nonne } namespace { -struct LoopAnalyzer { - const Token* bodyTok = nullptr; - const Token* loopVar = nullptr; - const Settings* settings = nullptr; - std::set varsChanged = {}; - - explicit LoopAnalyzer(const Token* tok, const Settings* psettings) : bodyTok(tok->next()->link()->next()), settings(psettings) - { - const Token *splitTok = tok->next()->astOperand2(); - if (Token::simpleMatch(splitTok, ":") && splitTok->previous()->varId() != 0) { - loopVar = splitTok->previous(); + struct LoopAnalyzer { + const Token* bodyTok = nullptr; + const Token* loopVar = nullptr; + const Settings* settings = nullptr; + std::set varsChanged = {}; + + explicit LoopAnalyzer(const Token* tok, const Settings* psettings) + : bodyTok(tok->next()->link()->next()), settings(psettings) + { + const Token* splitTok = tok->next()->astOperand2(); + if (Token::simpleMatch(splitTok, ":") && splitTok->previous()->varId() != 0) { + loopVar = splitTok->previous(); + } + if (valid()) { + findChangedVariables(); + } } - if (valid()) { - findChangedVariables(); + bool isLoopVarChanged() const { + return varsChanged.count(loopVar->varId()) > 0; } - } - bool isLoopVarChanged() const { - return varsChanged.count(loopVar->varId()) > 0; - } - bool isModified(const Token* tok) const { - if (tok->variable() && tok->variable()->isConst()) + bool isModified(const Token* tok) const + { + if (tok->variable() && tok->variable()->isConst()) + return false; + int n = 1 + (astIsPointer(tok) ? 1 : 0); + for (int i = 0; i < n; i++) { + bool inconclusive = false; + if (isVariableChangedByFunctionCall(tok, i, settings, &inconclusive)) + return true; + if (inconclusive) + return true; + if (isVariableChanged(tok, i, settings, true)) + return true; + } return false; - int n = 1 + (astIsPointer(tok) ? 1 : 0); - for(int i=0;i - void findTokens(Predicate pred, F f) const { - for(const Token* tok=bodyTok;precedes(tok, bodyTok->link());tok = tok->next()) { - if (pred(tok)) - f(tok); + template + void findTokens(Predicate pred, F f) const + { + for (const Token* tok = bodyTok; precedes(tok, bodyTok->link()); tok = tok->next()) { + if (pred(tok)) + f(tok); + } } - } - template - const Token* findToken(Predicate pred) const { - for(const Token* tok=bodyTok;precedes(tok, bodyTok->link());tok = tok->next()) { - if (pred(tok)) - return tok; + template + const Token* findToken(Predicate pred) const + { + for (const Token* tok = bodyTok; precedes(tok, bodyTok->link()); tok = tok->next()) { + if (pred(tok)) + return tok; + } + return nullptr; } - return nullptr; - } - bool hasGotoOrBreak() const { - return findToken([](const Token* tok) { - return Token::Match(tok, "goto|break"); - }); - } + bool hasGotoOrBreak() const + { + return findToken([](const Token* tok) { + return Token::Match(tok, "goto|break"); + }); + } - bool valid() const { - return bodyTok && loopVar; - } + bool valid() const { + return bodyTok && loopVar; + } - std::string findAlgo() const { - if(!valid()) - return ""; - bool loopVarChanged = isLoopVarChanged(); - if (!loopVarChanged && varsChanged.empty()) { - if (hasGotoOrBreak()) + std::string findAlgo() const + { + if (!valid()) return ""; - bool alwaysTrue = true; - bool alwaysFalse = true; - auto hasReturn = [](const Token* tok) { return Token::simpleMatch(tok, "return"); }; - findTokens(hasReturn, [&](const Token* tok) { - const Token* returnTok = tok->astOperand1(); - if (!returnTok || !returnTok->hasKnownIntValue() || !astIsBool(returnTok)) { - alwaysTrue = false; - alwaysFalse = false; - return; - } - (returnTok->values().front().intvalue ? alwaysTrue : alwaysFalse) &= true; - (returnTok->values().front().intvalue ? alwaysFalse : alwaysTrue) &= false; - }); - if (alwaysTrue == alwaysFalse) - return ""; - if (alwaysTrue) - return "std::any_of"; - else - return "std::all_of or std::none_of"; + bool loopVarChanged = isLoopVarChanged(); + if (!loopVarChanged && varsChanged.empty()) { + if (hasGotoOrBreak()) + return ""; + bool alwaysTrue = true; + bool alwaysFalse = true; + auto hasReturn = [](const Token* tok) { + return Token::simpleMatch(tok, "return"); + }; + findTokens(hasReturn, [&](const Token* tok) { + const Token* returnTok = tok->astOperand1(); + if (!returnTok || !returnTok->hasKnownIntValue() || !astIsBool(returnTok)) { + alwaysTrue = false; + alwaysFalse = false; + return; + } + (returnTok->values().front().intvalue ? alwaysTrue : alwaysFalse) &= true; + (returnTok->values().front().intvalue ? alwaysFalse : alwaysTrue) &= false; + }); + if (alwaysTrue == alwaysFalse) + return ""; + if (alwaysTrue) + return "std::any_of"; + else + return "std::all_of or std::none_of"; + } + return ""; } - return ""; - } - bool isLocalVar(const Variable* var) const { - if (!var) - return false; - if (var->isPointer() || var->isReference()) - return false; - if (var->declarationId() == loopVar->varId()) - return false; - const Scope* scope = var->scope(); - return scope->isNestedIn(bodyTok->scope()); - } -private: - void findChangedVariables() { - std::set vars; - for(const Token* tok=bodyTok;precedes(tok, bodyTok->link());tok = tok->next()) { - if (tok->varId() == 0) - continue; - if (vars.count(tok->varId()) > 0) - continue; - if (isLocalVar(tok->variable())) { + bool isLocalVar(const Variable* var) const + { + if (!var) + return false; + if (var->isPointer() || var->isReference()) + return false; + if (var->declarationId() == loopVar->varId()) + return false; + const Scope* scope = var->scope(); + return scope->isNestedIn(bodyTok->scope()); + } + + private: + void findChangedVariables() + { + std::set vars; + for (const Token* tok = bodyTok; precedes(tok, bodyTok->link()); tok = tok->next()) { + if (tok->varId() == 0) + continue; + if (vars.count(tok->varId()) > 0) + continue; + if (isLocalVar(tok->variable())) { + vars.insert(tok->varId()); + continue; + } + if (!isModified(tok)) + continue; + varsChanged.insert(tok->varId()); vars.insert(tok->varId()); - continue; } - if (!isModified(tok)) - continue; - varsChanged.insert(tok->varId()); - vars.insert(tok->varId()); } - } -}; -} + }; +} // namespace void CheckStl::useStlAlgorithm() { diff --git a/test/teststl.cpp b/test/teststl.cpp index 6be0626bdc3..d4a9cce164e 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -5121,7 +5121,8 @@ class TestStl : public TestFixture { " return true;\n" "}\n", true); - ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::all_of or std::none_of algorithm instead of a raw loop.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::all_of or std::none_of algorithm instead of a raw loop.\n", + errout.str()); check("bool pred(int x);\n" "bool foo() {\n" @@ -5294,7 +5295,8 @@ class TestStl : public TestFixture { " }\n" " return false;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", + errout.str()); } void loopAlgoMinMax() { @@ -5339,106 +5341,109 @@ class TestStl : public TestFixture { ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout.str()); } - void loopAlgoMultipleReturn() { + void loopAlgoMultipleReturn() + { check("bool f(const std::vector& v) {\n" -" for (auto i : v) {\n" -" if (i < 0)\n" -" continue;\n" -" if (i)\n" -" return true;\n" -" }\n" -" return false;\n" -"}\n", + " for (auto i : v) {\n" + " if (i < 0)\n" + " continue;\n" + " if (i)\n" + " return true;\n" + " }\n" + " return false;\n" + "}\n", true); - ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", + errout.str()); check("bool g(const std::vector& v) {\n" -" for (auto i : v) {\n" -" if (i % 5 == 0)\n" -" return true;\n" -" if (i % 7 == 0)\n" -" return true;\n" -" }\n" -" return false;\n" -"}\n", + " for (auto i : v) {\n" + " if (i % 5 == 0)\n" + " return true;\n" + " if (i % 7 == 0)\n" + " return true;\n" + " }\n" + " return false;\n" + "}\n", true); - ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); - + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", + errout.str()); -check("bool g(const std::vector& v) {\n" -" for (auto i : v) {\n" -" if (i % 5 == 0)\n" -" return false;\n" -" if (i % 7 == 0)\n" -" return true;\n" -" }\n" -" return false;\n" -"}\n", + check("bool g(const std::vector& v) {\n" + " for (auto i : v) {\n" + " if (i % 5 == 0)\n" + " return false;\n" + " if (i % 7 == 0)\n" + " return true;\n" + " }\n" + " return false;\n" + "}\n", true); ASSERT_EQUALS("", errout.str()); check("bool g(std::vector& v) {\n" -" for (auto& i : v) {\n" -" if (i % 5 == 0)\n" -" return false;\n" -" if (i % 7 == 0)\n" -" i++;\n" -" }\n" -" return false;\n" -"}\n", + " for (auto& i : v) {\n" + " if (i % 5 == 0)\n" + " return false;\n" + " if (i % 7 == 0)\n" + " i++;\n" + " }\n" + " return false;\n" + "}\n", true); ASSERT_EQUALS("", errout.str()); check("bool g(const std::vector& v, int& j) {\n" -" for (auto i : v) {\n" -" if (i % 5 == 0)\n" -" return false;\n" -" if (i % 7 == 0)\n" -" j++;\n" -" }\n" -" return false;\n" -"}\n", + " for (auto i : v) {\n" + " if (i % 5 == 0)\n" + " return false;\n" + " if (i % 7 == 0)\n" + " j++;\n" + " }\n" + " return false;\n" + "}\n", true); ASSERT_EQUALS("", errout.str()); check("bool g(const std::vector& v, int& j) {\n" -" for (auto i : v) {\n" -" int& k = j;\n" -" if (i % 5 == 0)\n" -" return false;\n" -" if (i % 7 == 0)\n" -" k++;\n" -" }\n" -" return false;\n" -"}\n", + " for (auto i : v) {\n" + " int& k = j;\n" + " if (i % 5 == 0)\n" + " return false;\n" + " if (i % 7 == 0)\n" + " k++;\n" + " }\n" + " return false;\n" + "}\n", true); ASSERT_EQUALS("", errout.str()); check("bool g(const std::vector& v, int& j) {\n" -" for (auto i : v) {\n" -" int* k = &j;\n" -" if (i % 5 == 0)\n" -" return false;\n" -" if (i % 7 == 0)\n" -" (*k)++;\n" -" }\n" -" return false;\n" -"}\n", + " for (auto i : v) {\n" + " int* k = &j;\n" + " if (i % 5 == 0)\n" + " return false;\n" + " if (i % 7 == 0)\n" + " (*k)++;\n" + " }\n" + " return false;\n" + "}\n", true); ASSERT_EQUALS("", errout.str()); check("bool g(const std::vector& v, int j) {\n" -" for (auto i : v) {\n" -" int k = j;\n" -" if (i % 5 == 0)\n" -" return false;\n" -" if (i % 7 == 0)\n" -" k++;\n" -" }\n" -" return false;\n" -"}\n", - true); - ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::all_of or std::none_of algorithm instead of a raw loop.\n", errout.str()); + " for (auto i : v) {\n" + " int k = j;\n" + " if (i % 5 == 0)\n" + " return false;\n" + " if (i % 7 == 0)\n" + " k++;\n" + " }\n" + " return false;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:2]: (style) Consider using std::all_of or std::none_of algorithm instead of a raw loop.\n", + errout.str()); } void invalidContainer() { From 77f0c2ab481ec73d27b191960bbbe4ea23bdaa6b Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 8 Mar 2023 10:57:30 -0600 Subject: [PATCH 4/6] Use any_of --- test/testvalueflow.cpp | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index e1ff1a95ceb..82d1735d6bd 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -202,12 +202,14 @@ class TestValueFlow : public TestFixture { for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { if (tok->str() == "x" && tok->linenr() == linenr) { - for (const ValueFlow::Value& val:tok->values()) { + if (std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& val) { if (val.isSymbolicValue()) - continue; + return false; if (val.isKnown() && val.intvalue == value) return true; - } + return false; + })) + return true; } } @@ -222,12 +224,14 @@ class TestValueFlow : public TestFixture { for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) { if (tok->str() == "x" && tok->linenr() == linenr) { - for (const ValueFlow::Value& val : tok->values()) { + if (std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& val) { if (!val.isSymbolicValue()) - continue; + return false; if (val.isKnown() && val.intvalue == value && val.tokvalue->expressionString() == expr) return true; - } + return false; + })) + return true; } } @@ -243,12 +247,14 @@ class TestValueFlow : public TestFixture { for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { if (tok->str() == "x" && tok->linenr() == linenr) { - for (const ValueFlow::Value& val:tok->values()) { + if (std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& val) { if (val.isSymbolicValue()) - continue; + return false; if (val.isImpossible() && val.intvalue == value) return true; - } + return false; + })) + return true; } } @@ -264,12 +270,14 @@ class TestValueFlow : public TestFixture { for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) { if (tok->str() == "x" && tok->linenr() == linenr) { - for (const ValueFlow::Value& val : tok->values()) { + if (std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& val) { if (!val.isSymbolicValue()) - continue; + return false; if (val.isImpossible() && val.intvalue == value && val.tokvalue->expressionString() == expr) return true; - } + return false; + })) + return true; } } @@ -285,12 +293,14 @@ class TestValueFlow : public TestFixture { for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { if (tok->str() == "x" && tok->linenr() == linenr) { - for (const ValueFlow::Value& val:tok->values()) { + if (std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& val) { if (val.isSymbolicValue()) - continue; + return false; if (val.isInconclusive() && val.intvalue == value) return true; - } + return false; + })) + return true; } } From 18e136ebfaef368d757638cace1db3ca491db0a5 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 8 Mar 2023 10:58:18 -0600 Subject: [PATCH 5/6] Dont use keyword --- lib/astutils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 85d82f109f8..327ea5932bf 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1981,7 +1981,7 @@ bool isUniqueExpression(const Token* tok) if (!scope) return true; const std::string returnType = fun->retType ? fun->retType->name() : fun->retDef->stringifyList(fun->tokenDef); - if (not std::all_of(scope->functionList.begin(), scope->functionList.end(), [&](const Function& f) { + if (!std::all_of(scope->functionList.begin(), scope->functionList.end(), [&](const Function& f) { if (f.type != Function::eFunction) return true; From 6866e761c5d938f0edd03aecd380f80614cb4845 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 8 Mar 2023 14:06:46 -0600 Subject: [PATCH 6/6] Use any_of --- lib/symboldatabase.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 2dc8977042d..50c6b752145 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -5128,7 +5128,7 @@ const Type* SymbolDatabase::findVariableType(const Scope *start, const Token *ty bool Scope::hasInlineOrLambdaFunction() const { - for (const Scope *s : nestedList) { + return std::any_of(nestedList.begin(), nestedList.end(), [&](const Scope* s) { // Inline function if (s->type == Scope::eUnconditional && Token::simpleMatch(s->bodyStart->previous(), ") {")) return true; @@ -5137,8 +5137,8 @@ bool Scope::hasInlineOrLambdaFunction() const return true; if (s->hasInlineOrLambdaFunction()) return true; - } - return false; + return false; + }); } void Scope::findFunctionInBase(const std::string & name, nonneg int args, std::vector & matches) const