From 2e830f278c285ec45805c73166bdaf7260f3d7bb Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 28 Oct 2026 13:23:58 -0500 Subject: [PATCH 1/4] Fix 15116: False positive: stlFindInsert (modifying existing element) --- Makefile | 2 +- lib/checkstl.cpp | 73 ++++++++++++++++++++++++++++++++++---------- lib/checkstl.h | 3 +- oss-fuzz/Makefile | 2 +- test/teststl.cpp | 78 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index daed0997479..b4b35a08200 100644 --- a/Makefile +++ b/Makefile @@ -568,7 +568,7 @@ $(libcppdir)/checks.o: lib/checks.cpp lib/check.h lib/check64bit.h lib/checkasse $(libcppdir)/checksizeof.o: lib/checksizeof.cpp lib/check.h lib/checkers.h lib/checkimpl.h lib/checksizeof.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checksizeof.cpp -$(libcppdir)/checkstl.o: lib/checkstl.cpp lib/astutils.h lib/check.h lib/checkers.h lib/checkimpl.h lib/checknullpointer.h lib/checkstl.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/pathanalysis.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h +$(libcppdir)/checkstl.o: lib/checkstl.cpp lib/astutils.h lib/calculate.h lib/check.h lib/checkers.h lib/checkimpl.h lib/checknullpointer.h lib/checkstl.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/pathanalysis.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkstl.cpp $(libcppdir)/checkstring.o: lib/checkstring.cpp lib/astutils.h lib/check.h lib/checkers.h lib/checkimpl.h lib/checkstring.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index a4e8a92c188..f5ccead8600 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -19,6 +19,7 @@ #include "checkstl.h" #include "astutils.h" +#include "calculate.h" #include "errortypes.h" #include "library.h" #include "mathlib.h" @@ -1734,30 +1735,70 @@ void CheckStlImpl::checkFindInsert() if (!valueTok2) continue; if (isSameExpression(true, valueTok, valueTok2, mSettings, true, true)) { - checkFindInsertError(valueTok); + // Both branches set the same value; the replacement depends on both branches, so no suggestion + checkFindInsertError(valueTok, nullptr); } } else { - checkFindInsertError(valueTok); + checkFindInsertError(valueTok, condTok); } } } } -void CheckStlImpl::checkFindInsertError(const Token *tok) +/** When is the condition of the 'if' statement checked by checkFindInsert() true? */ +enum class FindInsertCond : std::uint8_t { UNKNOWN, FOUND, NOT_FOUND }; + +static FindInsertCond getFindInsertCond(const Token *condTok) +{ + const Token *rhs = condTok->astOperand2(); + if (!rhs) + return FindInsertCond::UNKNOWN; + if (Token::Match(condTok->astOperand1()->astOperand1(), ". find (")) { + // c.find(k) == c.end() / c.find(k) != c.end() + if (!Token::simpleMatch(rhs, "(") || !Token::Match(rhs->astOperand1(), ". end|cend (")) + return FindInsertCond::UNKNOWN; + if (condTok->str() == "==") + return FindInsertCond::NOT_FOUND; + if (condTok->str() == "!=") + return FindInsertCond::FOUND; + return FindInsertCond::UNKNOWN; + } + // c.count(k) == 0, c.count(k) > 0, ...: count() yields 0 or 1 + if (!rhs->hasKnownIntValue()) + return FindInsertCond::UNKNOWN; + const MathLib::bigint n = rhs->getKnownIntValue(); + const bool found = calculate(condTok->str(), 1, n) != 0; + const bool notFound = calculate(condTok->str(), 0, n) != 0; + if (found == notFound) + return FindInsertCond::UNKNOWN; + return found ? FindInsertCond::FOUND : FindInsertCond::NOT_FOUND; +} + +void CheckStlImpl::checkFindInsertError(const Token *tok, const Token *condTok) { std::string replaceExpr; - if (tok && Token::simpleMatch(tok->astParent(), "=") && tok == tok->astParent()->astOperand2() && Token::simpleMatch(tok->astParent()->astOperand1(), "[")) { - if (mSettings.standards.cpp < Standards::CPP11) - // We will recommend using emplace/try_emplace instead - return; - const std::string f = (mSettings.standards.cpp < Standards::CPP17) ? "emplace" : "try_emplace"; - replaceExpr = " Instead of '" + tok->astParent()->expressionString() + "' consider using '" + - tok->astParent()->astOperand1()->astOperand1()->expressionString() + - "." + f + "(" + - tok->astParent()->astOperand1()->astOperand2()->expressionString() + - ", " + - tok->expressionString() + - ");'."; + if (tok && condTok && Token::simpleMatch(tok->astParent(), "=") && tok == tok->astParent()->astOperand2() && Token::simpleMatch(tok->astParent()->astOperand1(), "[")) { + const std::string containerExpr = tok->astParent()->astOperand1()->astOperand1()->expressionString(); + const std::string keyExpr = tok->astParent()->astOperand1()->astOperand2()->expressionString(); + switch (getFindInsertCond(condTok)) { + case FindInsertCond::NOT_FOUND: { + if (mSettings.standards.cpp < Standards::CPP11) + // We will recommend using emplace/try_emplace instead + return; + const std::string f = (mSettings.standards.cpp < Standards::CPP17) ? "emplace" : "try_emplace"; + replaceExpr = " Instead of '" + tok->astParent()->expressionString() + "' consider using '" + + containerExpr + "." + f + "(" + keyExpr + ", " + tok->expressionString() + ");'."; + break; + } + case FindInsertCond::FOUND: + // The key is present, so the value is assigned rather than inserted; try_emplace() would not do that + replaceExpr = " Instead of '" + tok->astParent()->expressionString() + + "' consider assigning the value through the iterator returned by '" + + containerExpr + ".find(" + keyExpr + ")'."; + break; + case FindInsertCond::UNKNOWN: + break; + } } reportError( @@ -3899,7 +3940,7 @@ void CheckStl::getErrorMessages(ErrorLogger& errorLogger, const Settings& settin c.stlBoundariesError(nullptr); c.if_findError(nullptr, false); c.if_findError(nullptr, true); - c.checkFindInsertError(nullptr); + c.checkFindInsertError(nullptr, nullptr); c.string_c_strError(nullptr); c.string_c_strReturn(nullptr); c.string_c_strParam(nullptr, 0); diff --git a/lib/checkstl.h b/lib/checkstl.h index 0f9a70a7a15..7df568ffc64 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -219,7 +219,8 @@ class CPPCHECKLIB CheckStlImpl : public CheckImpl { void sameIteratorExpressionError(const Token *tok); void stlBoundariesError(const Token* tok); void if_findError(const Token* tok, bool str); - void checkFindInsertError(const Token *tok); + /** @param condTok the comparison in the if condition, nullptr => no specific suggestion */ + void checkFindInsertError(const Token *tok, const Token *condTok); void sizeError(const Token* tok); void redundantIfRemoveError(const Token* tok); void invalidContainerLoopError(const Token* tok, const Token* loopTok, ErrorPath errorPath); diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index 5b8a1d159d4..081aa035cdc 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -237,7 +237,7 @@ $(libcppdir)/checks.o: ../lib/checks.cpp ../lib/check.h ../lib/check64bit.h ../l $(libcppdir)/checksizeof.o: ../lib/checksizeof.cpp ../lib/check.h ../lib/checkers.h ../lib/checkimpl.h ../lib/checksizeof.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/refthunk.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checksizeof.cpp -$(libcppdir)/checkstl.o: ../lib/checkstl.cpp ../lib/astutils.h ../lib/check.h ../lib/checkers.h ../lib/checkimpl.h ../lib/checknullpointer.h ../lib/checkstl.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/pathanalysis.h ../lib/platform.h ../lib/refthunk.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h +$(libcppdir)/checkstl.o: ../lib/checkstl.cpp ../lib/astutils.h ../lib/calculate.h ../lib/check.h ../lib/checkers.h ../lib/checkimpl.h ../lib/checknullpointer.h ../lib/checkstl.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/pathanalysis.h ../lib/platform.h ../lib/refthunk.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkstl.cpp $(libcppdir)/checkstring.o: ../lib/checkstring.cpp ../lib/astutils.h ../lib/check.h ../lib/checkers.h ../lib/checkimpl.h ../lib/checkstring.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/refthunk.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h diff --git a/test/teststl.cpp b/test/teststl.cpp index bb20d2d8182..646f631afcb 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -7480,6 +7480,84 @@ class TestStl : public TestFixture { "}\n"); // TODO: use s? ASSERT_EQUALS("[test.cpp:7:17]: (performance) Searching before insertion is not necessary. Instead of 'x[5]=data' consider using 'x.try_emplace(5, data);'. [stlFindInsert]\n", errout_str()); } + + // The key is found => the value is assigned, try_emplace() would not do that + check("void f() {\n" + " std::map m{{1, false}};\n" + " if (m.count(1) != 0)\n" + " m[1] = true;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4:16]: (performance) Searching before insertion is not necessary. Instead of 'm[1]=true' consider assigning the value through the iterator returned by 'm.find(1)'. [stlFindInsert]\n", errout_str()); + + check("void f(std::map& m, unsigned x) {\n" + " if (m.find(x) != m.end())\n" + " m[x] = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + + check("void f(std::map& m, unsigned x) {\n" + " if (m.count(x) > 0)\n" + " m[x] = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + + check("void f(std::map& m, unsigned x) {\n" + " if (m.count(x) == 1)\n" + " m[x] = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + + // The key is not found => try_emplace() + check("void f(std::map& m, unsigned x) {\n" + " if (m.count(x) == 0)\n" + " m[x] = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider using 'm.try_emplace(x, 1);'. [stlFindInsert]\n", errout_str()); + + check("void f(std::map& m, unsigned x) {\n" + " if (m.count(x) < 1)\n" + " m[x] = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider using 'm.try_emplace(x, 1);'. [stlFindInsert]\n", errout_str()); + + // Unknown condition => no suggestion + check("void f(std::map& m, unsigned x, unsigned y) {\n" + " if (m.count(x) == y)\n" + " m[x] = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", errout_str()); + + // Both branches set the value => no suggestion + check("void f(std::map& m, unsigned x) {\n" + " if (m.find(x) == m.end())\n" + " m[x] = 1;\n" + " else\n" + " m[x] = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", errout_str()); + + check("void f(std::map& m, unsigned x) {\n" + " if (m.find(x) != m.end())\n" + " m[x] = 1;\n" + " else\n" + " m.emplace(x, 1);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", errout_str()); + + // The suggestion for a found key does not depend on the C++ standard + { + Settings s = settings; + const char code[] = "void f(std::map& m, unsigned x) {\n" + " if (m.count(x) != 0)\n" + " m[x] = 1;\n" + "}\n"; + s.standards.cpp = Standards::CPP03; + check(code, s); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + s.standards.cpp = Standards::CPP11; + check(code, s); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + } } void checkKnownEmptyContainer() { From 9b6e9f8704db4877b7ea5d87c4071f54e00d4575 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 29 Oct 2026 16:43:59 -0500 Subject: [PATCH 2/4] Improve message --- lib/checkstl.cpp | 20 +++++++++++--------- test/teststl.cpp | 12 ++++++------ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index f5ccead8600..8e0f612426d 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1776,7 +1776,8 @@ static FindInsertCond getFindInsertCond(const Token *condTok) void CheckStlImpl::checkFindInsertError(const Token *tok, const Token *condTok) { - std::string replaceExpr; + std::string suggestion; + std::string method = "insertion"; if (tok && condTok && Token::simpleMatch(tok->astParent(), "=") && tok == tok->astParent()->astOperand2() && Token::simpleMatch(tok->astParent()->astOperand1(), "[")) { const std::string containerExpr = tok->astParent()->astOperand1()->astOperand1()->expressionString(); const std::string keyExpr = tok->astParent()->astOperand1()->astOperand2()->expressionString(); @@ -1786,23 +1787,24 @@ void CheckStlImpl::checkFindInsertError(const Token *tok, const Token *condTok) // We will recommend using emplace/try_emplace instead return; const std::string f = (mSettings.standards.cpp < Standards::CPP17) ? "emplace" : "try_emplace"; - replaceExpr = " Instead of '" + tok->astParent()->expressionString() + "' consider using '" + - containerExpr + "." + f + "(" + keyExpr + ", " + tok->expressionString() + ");'."; + suggestion = " Instead of '" + tok->astParent()->expressionString() + "' consider using '" + + containerExpr + "." + f + "(" + keyExpr + ", " + tok->expressionString() + ");'."; break; } case FindInsertCond::FOUND: - // The key is present, so the value is assigned rather than inserted; try_emplace() would not do that - replaceExpr = " Instead of '" + tok->astParent()->expressionString() + - "' consider assigning the value through the iterator returned by '" + - containerExpr + ".find(" + keyExpr + ")'."; + // The key is present, so the element is modified rather than inserted; try_emplace() would not do that + method = "modification"; + suggestion = " Instead of '" + tok->astParent()->expressionString() + + "' consider assigning the value through the iterator returned by '" + + containerExpr + ".find(" + keyExpr + ")'."; break; case FindInsertCond::UNKNOWN: break; } } + std::string msg = "Searching before " + method + " is not necessary." + suggestion; - reportError( - tok, Severity::performance, "stlFindInsert", "Searching before insertion is not necessary." + replaceExpr, CWE398, Certainty::normal); + reportError(tok, Severity::performance, "stlFindInsert", msg, CWE398, Certainty::normal); } /** diff --git a/test/teststl.cpp b/test/teststl.cpp index 646f631afcb..b2c5fc30e49 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -7487,25 +7487,25 @@ class TestStl : public TestFixture { " if (m.count(1) != 0)\n" " m[1] = true;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4:16]: (performance) Searching before insertion is not necessary. Instead of 'm[1]=true' consider assigning the value through the iterator returned by 'm.find(1)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (performance) Searching before modification is not necessary. Instead of 'm[1]=true' consider assigning the value through the iterator returned by 'm.find(1)'. [stlFindInsert]\n", errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.find(x) != m.end())\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.count(x) > 0)\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.count(x) == 1)\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); // The key is not found => try_emplace() check("void f(std::map& m, unsigned x) {\n" @@ -7553,10 +7553,10 @@ class TestStl : public TestFixture { "}\n"; s.standards.cpp = Standards::CPP03; check(code, s); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); s.standards.cpp = Standards::CPP11; check(code, s); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); } } From 26e0795522571a88cc8d55f60f69683ee0fd4a96 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 29 Oct 2026 16:44:27 -0500 Subject: [PATCH 3/4] Format --- lib/checkstl.cpp | 19 ++++++++++--------- lib/checkstl.h | 2 +- test/teststl.cpp | 41 ++++++++++++++++++++++++++++++----------- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 8e0f612426d..afb4e483359 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -23,8 +23,8 @@ #include "errortypes.h" #include "library.h" #include "mathlib.h" -#include "refthunk.h" #include "pathanalysis.h" +#include "refthunk.h" #include "settings.h" #include "standards.h" #include "symboldatabase.h" @@ -1748,9 +1748,9 @@ void CheckStlImpl::checkFindInsert() /** When is the condition of the 'if' statement checked by checkFindInsert() true? */ enum class FindInsertCond : std::uint8_t { UNKNOWN, FOUND, NOT_FOUND }; -static FindInsertCond getFindInsertCond(const Token *condTok) +static FindInsertCond getFindInsertCond(const Token* condTok) { - const Token *rhs = condTok->astOperand2(); + const Token* rhs = condTok->astOperand2(); if (!rhs) return FindInsertCond::UNKNOWN; if (Token::Match(condTok->astOperand1()->astOperand1(), ". find (")) { @@ -1774,11 +1774,12 @@ static FindInsertCond getFindInsertCond(const Token *condTok) return found ? FindInsertCond::FOUND : FindInsertCond::NOT_FOUND; } -void CheckStlImpl::checkFindInsertError(const Token *tok, const Token *condTok) +void CheckStlImpl::checkFindInsertError(const Token* tok, const Token* condTok) { std::string suggestion; std::string method = "insertion"; - if (tok && condTok && Token::simpleMatch(tok->astParent(), "=") && tok == tok->astParent()->astOperand2() && Token::simpleMatch(tok->astParent()->astOperand1(), "[")) { + if (tok && condTok && Token::simpleMatch(tok->astParent(), "=") && tok == tok->astParent()->astOperand2() && + Token::simpleMatch(tok->astParent()->astOperand1(), "[")) { const std::string containerExpr = tok->astParent()->astOperand1()->astOperand1()->expressionString(); const std::string keyExpr = tok->astParent()->astOperand1()->astOperand2()->expressionString(); switch (getFindInsertCond(condTok)) { @@ -1787,16 +1788,16 @@ void CheckStlImpl::checkFindInsertError(const Token *tok, const Token *condTok) // We will recommend using emplace/try_emplace instead return; const std::string f = (mSettings.standards.cpp < Standards::CPP17) ? "emplace" : "try_emplace"; - suggestion = " Instead of '" + tok->astParent()->expressionString() + "' consider using '" + - containerExpr + "." + f + "(" + keyExpr + ", " + tok->expressionString() + ");'."; + suggestion = " Instead of '" + tok->astParent()->expressionString() + "' consider using '" + containerExpr + + "." + f + "(" + keyExpr + ", " + tok->expressionString() + ");'."; break; } case FindInsertCond::FOUND: // The key is present, so the element is modified rather than inserted; try_emplace() would not do that method = "modification"; suggestion = " Instead of '" + tok->astParent()->expressionString() + - "' consider assigning the value through the iterator returned by '" + - containerExpr + ".find(" + keyExpr + ")'."; + "' consider assigning the value through the iterator returned by '" + containerExpr + + ".find(" + keyExpr + ")'."; break; case FindInsertCond::UNKNOWN: break; diff --git a/lib/checkstl.h b/lib/checkstl.h index 7df568ffc64..059ddb7466e 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -220,7 +220,7 @@ class CPPCHECKLIB CheckStlImpl : public CheckImpl { void stlBoundariesError(const Token* tok); void if_findError(const Token* tok, bool str); /** @param condTok the comparison in the if condition, nullptr => no specific suggestion */ - void checkFindInsertError(const Token *tok, const Token *condTok); + void checkFindInsertError(const Token* tok, const Token* condTok); void sizeError(const Token* tok); void redundantIfRemoveError(const Token* tok); void invalidContainerLoopError(const Token* tok, const Token* loopTok, ErrorPath errorPath); diff --git a/test/teststl.cpp b/test/teststl.cpp index b2c5fc30e49..1e0dd48d698 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -7487,45 +7487,58 @@ class TestStl : public TestFixture { " if (m.count(1) != 0)\n" " m[1] = true;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4:16]: (performance) Searching before modification is not necessary. Instead of 'm[1]=true' consider assigning the value through the iterator returned by 'm.find(1)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:4:16]: (performance) Searching before modification is not necessary. Instead of 'm[1]=true' consider assigning the value through the iterator returned by 'm.find(1)'. [stlFindInsert]\n", + errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.find(x) != m.end())\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", + errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.count(x) > 0)\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", + errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.count(x) == 1)\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", + errout_str()); // The key is not found => try_emplace() check("void f(std::map& m, unsigned x) {\n" " if (m.count(x) == 0)\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider using 'm.try_emplace(x, 1);'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider using 'm.try_emplace(x, 1);'. [stlFindInsert]\n", + errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.count(x) < 1)\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider using 'm.try_emplace(x, 1);'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:3:16]: (performance) Searching before insertion is not necessary. Instead of 'm[x]=1' consider using 'm.try_emplace(x, 1);'. [stlFindInsert]\n", + errout_str()); // Unknown condition => no suggestion check("void f(std::map& m, unsigned x, unsigned y) {\n" " if (m.count(x) == y)\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", + errout_str()); // Both branches set the value => no suggestion check("void f(std::map& m, unsigned x) {\n" @@ -7534,7 +7547,8 @@ class TestStl : public TestFixture { " else\n" " m[x] = 1;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", + errout_str()); check("void f(std::map& m, unsigned x) {\n" " if (m.find(x) != m.end())\n" @@ -7542,7 +7556,8 @@ class TestStl : public TestFixture { " else\n" " m.emplace(x, 1);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before insertion is not necessary. [stlFindInsert]\n", + errout_str()); // The suggestion for a found key does not depend on the C++ standard { @@ -7553,10 +7568,14 @@ class TestStl : public TestFixture { "}\n"; s.standards.cpp = Standards::CPP03; check(code, s); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", + errout_str()); s.standards.cpp = Standards::CPP11; check(code, s); - ASSERT_EQUALS("[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:3:16]: (performance) Searching before modification is not necessary. Instead of 'm[x]=1' consider assigning the value through the iterator returned by 'm.find(x)'. [stlFindInsert]\n", + errout_str()); } } From 10bd8fa7a2aae7ab84caab8eee65e5a63b600d70 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 30 Oct 2026 15:06:48 -0500 Subject: [PATCH 4/4] Fix ci --- lib/checkstl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index afb4e483359..f07e8aa7166 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1753,7 +1753,7 @@ static FindInsertCond getFindInsertCond(const Token* condTok) const Token* rhs = condTok->astOperand2(); if (!rhs) return FindInsertCond::UNKNOWN; - if (Token::Match(condTok->astOperand1()->astOperand1(), ". find (")) { + if (Token::simpleMatch(condTok->astOperand1()->astOperand1(), ". find (")) { // c.find(k) == c.end() / c.find(k) != c.end() if (!Token::simpleMatch(rhs, "(") || !Token::Match(rhs->astOperand1(), ". end|cend (")) return FindInsertCond::UNKNOWN;