diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 59eee855f..4ca021439 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1706,126 +1706,6 @@ void CheckStl::dereferenceInvalidIteratorError(const Token* deref, const std::st } - -void CheckStl::readingEmptyStlContainer_parseUsage(const Token* tok, const Library::Container* container, std::map& empty, bool noerror) -{ - // Check for various conditions for the way stl containers and variables can be used - if (tok->strAt(1) == "=" || (tok->strAt(1) == "[" && Token::simpleMatch(tok->linkAt(1), "] ="))) { - // Assignment (LHS) - empty.erase(tok->varId()); - } else if (Token::Match(tok, "%name% [")) { - // Access through operator[] - if (!container->arrayLike_indexOp) { // operator[] inserts an element if used on a std::map - if (!noerror && tok->strAt(-1) == "=") - readingEmptyStlContainerError(tok); - empty.erase(tok->varId()); - } else if (!noerror) - readingEmptyStlContainerError(tok); - } else if (Token::Match(tok, "%name% . %type% (")) { - // Member function call - const Library::Container::Action action = container->getAction(tok->strAt(2)); - if ((action == Library::Container::FIND || action == Library::Container::ERASE || action == Library::Container::POP || action == Library::Container::CLEAR) && !noerror) { - readingEmptyStlContainerError(tok); - return; - } - - const Token* parent = tok->tokAt(3)->astParent(); - const Library::Container::Yield yield = container->getYield(tok->strAt(2)); - const bool yieldsIterator = (yield == Library::Container::ITERATOR || yield == Library::Container::START_ITERATOR || yield == Library::Container::END_ITERATOR); - if (yield != Library::Container::NO_YIELD && - (!parent || Token::Match(parent, "%cop%|*") || parent->isAssignmentOp() || !yieldsIterator)) { // These functions read from the container - if (!noerror && (!yieldsIterator || !parent || !parent->isAssignmentOp())) - readingEmptyStlContainerError(tok); - } else - empty.erase(tok->varId()); - } else if (tok->strAt(-1) == "=") { - // Assignment (RHS) - if (!noerror) - readingEmptyStlContainerError(tok); - } else { - // Unknown usage. Assume it is initialized. - empty.erase(tok->varId()); - } -} - -void CheckStl::readingEmptyStlContainer() -{ - if (!mSettings->isEnabled(Settings::STYLE)) - return; - - if (!mSettings->inconclusive) - return; - - std::map emptyContainer; - - for (const Scope &scope : mTokenizer->getSymbolDatabase()->scopeList) { - if (scope.type != Scope::eFunction) - continue; - - for (const Token *tok = scope.bodyStart->next(); tok != scope.bodyEnd; tok = tok->next()) { - if (Token::Match(tok, "for|while")) { // Loops and end of scope clear the sets. - const Token* tok2 = tok->linkAt(1); - if (!tok2) - continue; - tok2 = tok2->next(); - for (const Token* end2 = tok2->link(); tok2 && tok2 != end2; tok2 = tok2->next()) { - if (!tok2->varId()) - continue; - - const std::map::const_iterator container = emptyContainer.find(tok2->varId()); - if (container == emptyContainer.end()) - continue; - - readingEmptyStlContainer_parseUsage(tok2, container->second, emptyContainer, true); - } - } else if (Token::Match(tok, "do|}|break|case")) { - emptyContainer.clear(); - } else if (tok->str() == "{" && tok->next()->scope()->type == Scope::eLambda) - tok = tok->link(); - - // function call - if (Token::Match(tok, "!!. %name% (") && !Token::simpleMatch(tok->linkAt(2), ") {")) { - for (std::map::iterator it = emptyContainer.begin(); it != emptyContainer.end();) { - const Variable *var = mTokenizer->getSymbolDatabase()->getVariableFromVarId(it->first); - if (var && (var->isLocal() || var->isArgument())) - ++it; - else - emptyContainer.erase(it++); - } - } - - if (!tok->varId()) - continue; - - // Check whether a variable should be marked as "empty" - const Variable* var = tok->variable(); - if (var && !var->isArrayOrPointer() && !var->typeStartToken()->isStandardType()) { - bool insert = false; - if (var->nameToken() == tok && var->isLocal() && !var->isStatic()) { // Local variable declared - insert = !Token::Match(tok->next(), "[(=:]"); // Only if not initialized - } else if (Token::Match(tok, "%var% . clear ( ) ;")) { - insert = true; - } - - if (insert) { - const Library::Container* container = mSettings->library.detectContainer(var->typeStartToken()); - if (container) - emptyContainer[var->declarationId()] = container; - continue; - } - } - - const std::map::const_iterator container = emptyContainer.find(tok->varId()); - if (container == emptyContainer.end()) - continue; - - readingEmptyStlContainer_parseUsage(tok, container->second, emptyContainer, false); - } - emptyContainer.clear(); - } -} - - void CheckStl::readingEmptyStlContainer2() { for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) { diff --git a/lib/checkstl.h b/lib/checkstl.h index 0a39bfdae..ac33bca7f 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -88,7 +88,6 @@ public: checkStl.size(); checkStl.redundantCondition(); checkStl.missingComparison(); - checkStl.readingEmptyStlContainer(); } /** Accessing container out of bounds using ValueFlow */ @@ -178,15 +177,9 @@ public: */ void dereferenceErasedError(const Token* erased, const Token* deref, const std::string& itername, bool inconclusive); - /** @brief Reading from empty stl container */ - void readingEmptyStlContainer(); - - /** @brief Reading from empty stl container (using valueflow) */ void readingEmptyStlContainer2(); private: - void readingEmptyStlContainer_parseUsage(const Token* tok, const Library::Container* container, std::map& empty, bool noerror); - void missingComparisonError(const Token* incrementToken1, const Token* incrementToken2); void string_c_strThrowError(const Token* tok); void string_c_strError(const Token* tok); diff --git a/test/teststl.cpp b/test/teststl.cpp index ac704e4b6..688503bfc 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -144,8 +144,6 @@ private: TEST_CASE(dereferenceInvalidIterator); TEST_CASE(dereferenceInvalidIterator2); // #6572 TEST_CASE(dereference_auto); - - TEST_CASE(readingEmptyStlContainer); } void check(const char code[], const bool inconclusive=false, const Standards::cppstd_t cppstandard=Standards::CPP11) { @@ -3151,226 +3149,6 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:18]: (error, inconclusive) Invalid iterator 'it' used.\n", errout.str()); } - - void readingEmptyStlContainer() { - check("void f() {\n" - " std::map CMap;\n" - " std::string strValue = CMap[1]; \n" - " std::cout << strValue << CMap.size() << std::endl;\n" - "}\n",true); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container 'CMap'\n", errout.str()); - - check("void f() {\n" - " std::map CMap;\n" - " std::string strValue = CMap[1];" - "}\n",true); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container 'CMap'\n", errout.str()); - - check("void f() {\n" - " std::map CMap;\n" - " CMap[1] = \"123\";\n" - " std::string strValue = CMap[1];" - "}\n",true); - ASSERT_EQUALS("", errout.str()); - - check("std::vector f() {\n" - " try {\n" - " std::vector Vector;\n" - " std::vector v2 = Vector;\n" - " std::string strValue = v2[1]; \n" // Do not complain here - this is a consecutive fault of the line above. - " }\n" - " return Vector;\n" - "}\n",true); - ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Reading from empty STL container 'Vector'\n", errout.str()); - - check("Vector f() {\n" - " try {\n" - " std::vector Vector;\n" - " Vector.push_back(\"123\");\n" - " std::vector v2 = Vector;\n" - " std::string strValue = v2[0]; \n" - " }\n" - " return Vector;\n" - "}\n", true); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " std::map mymap;\n" - " mymap[\"Bakery\"] = \"Barbara\";\n" - " std:string bakery_name = mymap[\"Bakery\"];\n" - "}\n", true); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " std::vector v;\n" - " v.insert(1);\n" - " int i = v[0];\n" - "}\n", true); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " std::vector v;\n" - " initialize(v);\n" - " int i = v[0];\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - - check("char f() {\n" - " std::string s(foo);\n" - " return s[0];\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " std::vector v = foo();\n" - " if(bar) v.clear();\n" - " int i = v.find(foobar);\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - - check("void f(std::set v) {\n" - " v.clear();\n" - " int i = v.find(foobar);\n" - "}", true); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container 'v'\n", errout.str()); - - check("void f(std::set v) {\n" - " v.clear();\n" - " v.begin();\n" - "}", true); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container 'v'\n", errout.str()); - - check("void f(std::set v) {\n" - " v.clear();\n" - " *v.begin();\n" - "}", true); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container 'v'\n", errout.str()); - - check("void f(std::set v) {\n" - " v.clear();\n" - " for(auto i = v.cbegin();\n" - " i != v.cend(); ++i) {}\n" - "}", true); - ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Reading from empty STL container 'v'\n", errout.str()); - - check("void f(std::set v) {\n" - " v.clear();\n" - " foo(v.begin());\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " std::map CMap;\n" - " std::string strValue = CMap[1];\n" - " std::string strValue2 = CMap[1];\n" - "}\n", true); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container 'CMap'\n", errout.str()); - - // #4306 - check("void f(std::vector v) {\n" - " v.clear();\n" - " for(int i = 0; i < v.size(); i++) { cout << v[i]; }\n" - "}", true); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container 'v'\n", errout.str()); - - // #7449 - nonlocal vector - check("std::vector v;\n" - "void f() {\n" - " v.clear();\n" - " dostuff()\n" - " if (v.empty()) { }\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - - check("std::vector v;\n" - "void f() {\n" - " v.clear();\n" - " if (v.empty()) { }\n" - "}", true); - ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Reading from empty STL container 'v'\n", errout.str()); - - // #6663 - check("void foo() {\n" - " std::set container;\n" - " while (container.size() < 5)\n" - " container.insert(22);\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - - // #6679 - check("class C {\n" - " C() {\n" - " switch (ret) {\n" - " case 1:\n" - " vec.clear();\n" - " break;\n" - " case 2:\n" - " if (vec.empty())\n" - " ;\n" - " break;\n" - " }\n" - " }\n" - " std::vector vec;\n" - "};", true); - ASSERT_EQUALS("", errout.str()); - - check("class C {\n" - " C() {\n" - " switch (ret) {\n" - " case 1:\n" - " vec.clear();\n" - " case 2:\n" - " if (vec.empty())\n" - " ;\n" - " break;\n" - " }\n" - " }\n" - " std::vector vec;\n" - "};", true); - ASSERT_EQUALS("", errout.str()); - - check("class C {\n" - " C() {\n" - " switch (ret) {\n" - " case 1:\n" - " vec.clear();\n" - " if (vec.empty())\n" - " ;\n" - " break;\n" - " }\n" - " }\n" - " std::vector vec;\n" - "};", true); - ASSERT_EQUALS("[test.cpp:6]: (style, inconclusive) Reading from empty STL container 'vec'\n", errout.str()); - - // #7560 - check("std::vector test;\n" - "std::vector::iterator it;\n" - "void Reset() {\n" - " test.clear();\n" - " it = test.end();\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - // #8055 - check("int main() {\n" - " std::string str;\n" - " auto l = [&]() {\n" - " if (str[0] == 'A')\n" - " std::cout << \"!\";\n" - " }\n" - " str = \"A\";\n" - " l();\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f(const std::vector &v) {\n" - " for (const std::string& s : v) {\n" - " if (s.find(x) != string::npos) {}\n" - " }\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - } }; REGISTER_TEST(TestStl)