From 0ca369d05c18e1158049f62a11f0025aac010d27 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 1 Aug 2022 18:39:38 -0500 Subject: [PATCH 1/2] Fix 11147: FP invalidContainer with substr() --- lib/checkstl.cpp | 19 ++++++++++++++++--- test/teststl.cpp | 10 ++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index bb1be71566d..eda6f4aaddd 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1068,6 +1068,21 @@ static const ValueFlow::Value* getInnerLifetime(const Token* tok, return nullptr; } +static const Token* endOfExpression(const Token* tok) +{ + if (!tok) + return nullptr; + const Token* parent = tok->astParent(); + while(Token::simpleMatch(parent, ".")) + parent = parent->astParent(); + if (!parent) + return tok->next(); + const Token* endToken = nextAfterAstRightmostLeaf(parent); + if (!endToken) + return parent->next(); + return endToken; +} + void CheckStl::invalidContainer() { const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); @@ -1120,9 +1135,7 @@ void CheckStl::invalidContainer() } if (Token::Match(assignExpr, "%assign%") && Token::Match(assignExpr->astOperand1(), "%var%")) skipVarIds.insert(assignExpr->astOperand1()->varId()); - const Token* endToken = nextAfterAstRightmostLeaf(tok->next()->astParent()); - if (!endToken) - endToken = tok->next(); + const Token* endToken = endOfExpression(tok); const ValueFlow::Value* v = nullptr; ErrorPath errorPath; PathAnalysis::Info info = diff --git a/test/teststl.cpp b/test/teststl.cpp index d7ea7f62f82..834a4afb300 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -5465,6 +5465,16 @@ class TestStl : public TestFixture { "}\n", true); ASSERT_EQUALS("", errout.str()); + + // #11147 + check("void f(std::string& s) {\n" + " if (!s.empty()) {\n" + " std::string::iterator it = s.begin();\n" + " s = s.substr(it - s.begin());\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (performance) Ineffective call of function 'substr' because a prefix of the string is assigned to itself. Use resize() or pop_back() instead.\n", errout.str()); } void invalidContainerLoop() { From 00a9b0a70c51f796381d920ff5aa81e2ae8863fc Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 1 Aug 2022 18:40:16 -0500 Subject: [PATCH 2/2] Format --- lib/checkstl.cpp | 2 +- test/teststl.cpp | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index eda6f4aaddd..d54b7192865 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1073,7 +1073,7 @@ static const Token* endOfExpression(const Token* tok) if (!tok) return nullptr; const Token* parent = tok->astParent(); - while(Token::simpleMatch(parent, ".")) + while (Token::simpleMatch(parent, ".")) parent = parent->astParent(); if (!parent) return tok->next(); diff --git a/test/teststl.cpp b/test/teststl.cpp index 834a4afb300..e279d8e85ed 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -5468,13 +5468,15 @@ class TestStl : public TestFixture { // #11147 check("void f(std::string& s) {\n" - " if (!s.empty()) {\n" - " std::string::iterator it = s.begin();\n" - " s = s.substr(it - s.begin());\n" - " }\n" - "}\n", - true); - ASSERT_EQUALS("[test.cpp:4]: (performance) Ineffective call of function 'substr' because a prefix of the string is assigned to itself. Use resize() or pop_back() instead.\n", errout.str()); + " if (!s.empty()) {\n" + " std::string::iterator it = s.begin();\n" + " s = s.substr(it - s.begin());\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS( + "[test.cpp:4]: (performance) Ineffective call of function 'substr' because a prefix of the string is assigned to itself. Use resize() or pop_back() instead.\n", + errout.str()); } void invalidContainerLoop() {