From d839bc37ce0c43a7ad85a9f3cfeaa8f77b593ac1 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Fri, 21 Feb 2025 19:01:17 +0100 Subject: [PATCH 1/4] Update teststl.cpp --- test/teststl.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/teststl.cpp b/test/teststl.cpp index da6b1c1fd5e..084a7d99405 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -5859,7 +5859,7 @@ class TestStl : public TestFixture { dinit(CheckOptions, $.inconclusive = true)); ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout_str()); - check("void f(const std::vector& v) {\n" + check("void f(const std::vector& v) {\n" // #9091 " int maxY = 0;\n" " for (int y : v) {\n" " if (y > maxY)\n" @@ -5867,9 +5867,7 @@ class TestStl : public TestFixture { " }\n" "}\n", dinit(CheckOptions, $.inconclusive = true)); - TODO_ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::max_element algorithm instead of a raw loop.\n", - "", - errout_str()); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::max_element algorithm instead of a raw loop.\n", errout_str()); } void loopAlgoMultipleReturn() From f80fe0806b878e6c8b968d442a2488836136415e Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Fri, 21 Feb 2025 19:02:45 +0100 Subject: [PATCH 2/4] Update checkstl.cpp --- lib/checkstl.cpp | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 1b3a8edb8b8..1a44cb3a632 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -2899,10 +2899,21 @@ void CheckStl::useStlAlgorithm() return !astIsContainer(tok); // don't warn for containers, where overloaded operators can be costly }; - auto isConditionWithoutSideEffects = [this](const Token* tok) -> bool { + enum class ConditionOpType { OTHER, MIN, MAX }; + auto isConditionWithoutSideEffects = [this](const Token* tok, ConditionOpType& type) -> bool { if (!Token::simpleMatch(tok, "{") || !Token::simpleMatch(tok->previous(), ")")) return false; - return isConstExpression(tok->linkAt(-1)->astOperand2(), mSettings->library); + const Token* condTok = tok->linkAt(-1)->astOperand2(); + if (isConstExpression(condTok, mSettings->library)) { + if (condTok->str() == "<") + type = ConditionOpType::MIN; + else if (condTok->str() == ">") + type = ConditionOpType::MAX; + else + type = ConditionOpType::OTHER; + return true; + } + return false; }; auto isAccumulation = [](const Token* tok, int varId) { @@ -3035,14 +3046,27 @@ void CheckStl::useStlAlgorithm() else algo = "std::replace_if"; } else { + ConditionOpType type{}; if (addByOne(assignTok, assignVarId)) algo = "std::count_if"; else if (accumulateBoolLiteral(assignTok, assignVarId)) algo = "std::any_of, std::all_of, std::none_of, or std::accumulate"; else if (assignTok->str() != "=") algo = "std::accumulate"; - else if (hasBreak && isConditionWithoutSideEffects(condBodyTok)) - algo = "std::any_of, std::all_of, std::none_of"; + else if (isConditionWithoutSideEffects(condBodyTok, type)) { + if (hasBreak) + algo = "std::any_of, std::all_of, std::none_of"; + else if (assignTok->astOperand2()->varId() == loopVar->varId()) { + if (type == ConditionOpType::MIN) + algo = "std::min_element"; + else if (type == ConditionOpType::MAX) + algo = "std::max_element"; + else + continue; + } + else + continue; + } else continue; } From f9af4dd8357106584585c7acce3eca6b7f3ba9b4 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Fri, 21 Feb 2025 20:28:08 +0100 Subject: [PATCH 3/4] Update checkstl.cpp --- lib/checkstl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 1a44cb3a632..028a29f7ac0 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -2899,7 +2899,7 @@ void CheckStl::useStlAlgorithm() return !astIsContainer(tok); // don't warn for containers, where overloaded operators can be costly }; - enum class ConditionOpType { OTHER, MIN, MAX }; + enum class ConditionOpType : std::uint8_t { OTHER, MIN, MAX }; auto isConditionWithoutSideEffects = [this](const Token* tok, ConditionOpType& type) -> bool { if (!Token::simpleMatch(tok, "{") || !Token::simpleMatch(tok->previous(), ")")) return false; From 5281792b092c55d9bc63a1ea8352ceb33a4e4fcb Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sun, 23 Feb 2025 22:59:14 +0100 Subject: [PATCH 4/4] Add test --- test/teststl.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/teststl.cpp b/test/teststl.cpp index 084a7d99405..ee1224d4656 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -5868,6 +5868,17 @@ class TestStl : public TestFixture { "}\n", dinit(CheckOptions, $.inconclusive = true)); ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::max_element algorithm instead of a raw loop.\n", errout_str()); + + check("int f(const std::vector& v) {\n" + " int minY = 0;\n" + " for (int y : v) {\n" + " if (y < minY)\n" + " minY = y;\n" + " }\n" + " return minY;\n" + "}\n", + dinit(CheckOptions, $.inconclusive = true)); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::min_element algorithm instead of a raw loop.\n", errout_str()); } void loopAlgoMultipleReturn()