diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index ce07fb9c6..e1f706bfa 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1253,15 +1253,24 @@ void CheckStl::uselessCalls() for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { if (printWarning && Token::Match(tok, "%var% . compare|find|rfind|find_first_not_of|find_first_of|find_last_not_of|find_last_of ( %name% [,)]") && tok->varId() == tok->tokAt(4)->varId()) { + const Variable* var = tok->variable(); + if (!var || !var->isStlType()) + continue; uselessCallsReturnValueError(tok->tokAt(4), tok->str(), tok->strAt(2)); } else if (printPerformance && Token::Match(tok, "%var% . swap ( %name% )") && tok->varId() == tok->tokAt(4)->varId()) { + const Variable* var = tok->variable(); + if (!var || !var->isStlType()) + continue; uselessCallsSwapError(tok, tok->str()); } else if (printPerformance && Token::Match(tok, "%var% . substr (") && tok->variable() && tok->variable()->isStlStringType()) { - if (Token::Match(tok->tokAt(4), "0| )")) + if (Token::Match(tok->tokAt(4), "0| )")) { + const Variable* var = tok->variable(); + if (!var || !var->isStlType()) + continue; uselessCallsSubstrError(tok, false); - else if (tok->strAt(4) == "0" && tok->linkAt(3)->strAt(-1) == "npos") { + } else if (tok->strAt(4) == "0" && tok->linkAt(3)->strAt(-1) == "npos") { if (!tok->linkAt(3)->previous()->variable()) // Make sure that its no variable uselessCallsSubstrError(tok, false); } else if (Token::simpleMatch(tok->linkAt(3)->tokAt(-2), ", 0 )")) diff --git a/test/teststl.cpp b/test/teststl.cpp index ee2a0b10d..997711c3e 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2498,11 +2498,19 @@ private: " s1.swap(s2);\n" " s2.swap(s2);\n" "};"); + ASSERT_EQUALS("", errout.str()); + + check("void f()\n" + "{\n" + " std::string s1, s2;\n" + " s1.swap(s2);\n" + " s2.swap(s2);\n" + "};"); ASSERT_EQUALS("[test.cpp:5]: (performance) It is inefficient to swap a object with itself by calling 's2.swap(s2)'\n", errout.str()); check("void f()\n" "{\n" - " string s1, s2;\n" + " std::string s1, s2;\n" " s1.compare(s2);\n" " s2.compare(s2);\n" " s1.compare(s2.c_str());\n" @@ -2510,6 +2518,17 @@ private: "};"); ASSERT_EQUALS("[test.cpp:5]: (warning) It is inefficient to call 's2.compare(s2)' as it always returns 0.\n", errout.str()); + // #7370 False positive uselessCallsCompare on unknown type + check("class ReplayIteratorImpl{\n" + " int Compare(ReplayIteratorImpl* other) {\n" + " int cmp;\n" + " int ret = cursor_->compare(cursor_, other->cursor_, &cmp);\n" + " return (cmp);\n" + " }\n" + " WT_CURSOR *cursor_;\n" + "};"); + ASSERT_EQUALS("", errout.str()); + check("void f()\n" "{\n" " int x=1;\n" @@ -2566,6 +2585,7 @@ private: ASSERT_EQUALS("", errout.str()); check("void f() {\n" + " std::vector a;\n" " std::remove(a.begin(), a.end(), val);\n" " std::remove_if(a.begin(), a.end(), val);\n" " std::unique(a.begin(), a.end(), val);\n" @@ -2573,9 +2593,9 @@ private: " a.erase(std::remove(a.begin(), a.end(), val));\n" " std::remove(\"foo.txt\");\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning) Return value of std::remove() ignored. Elements remain in container.\n" - "[test.cpp:3]: (warning) Return value of std::remove_if() ignored. Elements remain in container.\n" - "[test.cpp:4]: (warning) Return value of std::unique() ignored. Elements remain in container.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (warning) Return value of std::remove() ignored. Elements remain in container.\n" + "[test.cpp:4]: (warning) Return value of std::remove_if() ignored. Elements remain in container.\n" + "[test.cpp:5]: (warning) Return value of std::unique() ignored. Elements remain in container.\n", errout.str()); // #4431 - fp check("bool f() {\n"