Remove inconclusive warnings about reading empty stl container. We have better ValueFlow-based checking.
This commit is contained in:
Daniel Marjamäki
parent
b3f12fcc7e
commit
772939476d
120
lib/checkstl.cpp
120
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<unsigned int, const Library::Container*>& 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<unsigned int, const Library::Container*> 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<unsigned int, const Library::Container*>::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<unsigned int, const Library::Container*>::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<unsigned int, const Library::Container*>::const_iterator container = emptyContainer.find(tok->varId());
|
|
||||||
if (container == emptyContainer.end())
|
|
||||||
continue;
|
|
||||||
|
|
||||||
readingEmptyStlContainer_parseUsage(tok, container->second, emptyContainer, false);
|
|
||||||
}
|
|
||||||
emptyContainer.clear();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
void CheckStl::readingEmptyStlContainer2()
|
void CheckStl::readingEmptyStlContainer2()
|
||||||
{
|
{
|
||||||
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
|
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
|
||||||
|
|
|
||||||
|
|
@ -88,7 +88,6 @@ public:
|
||||||
checkStl.size();
|
checkStl.size();
|
||||||
checkStl.redundantCondition();
|
checkStl.redundantCondition();
|
||||||
checkStl.missingComparison();
|
checkStl.missingComparison();
|
||||||
checkStl.readingEmptyStlContainer();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Accessing container out of bounds using ValueFlow */
|
/** 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);
|
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) */
|
/** @brief Reading from empty stl container (using valueflow) */
|
||||||
void readingEmptyStlContainer2();
|
void readingEmptyStlContainer2();
|
||||||
private:
|
private:
|
||||||
void readingEmptyStlContainer_parseUsage(const Token* tok, const Library::Container* container, std::map<unsigned int, const Library::Container*>& empty, bool noerror);
|
|
||||||
|
|
||||||
void missingComparisonError(const Token* incrementToken1, const Token* incrementToken2);
|
void missingComparisonError(const Token* incrementToken1, const Token* incrementToken2);
|
||||||
void string_c_strThrowError(const Token* tok);
|
void string_c_strThrowError(const Token* tok);
|
||||||
void string_c_strError(const Token* tok);
|
void string_c_strError(const Token* tok);
|
||||||
|
|
|
||||||
222
test/teststl.cpp
222
test/teststl.cpp
|
|
@ -144,8 +144,6 @@ private:
|
||||||
TEST_CASE(dereferenceInvalidIterator);
|
TEST_CASE(dereferenceInvalidIterator);
|
||||||
TEST_CASE(dereferenceInvalidIterator2); // #6572
|
TEST_CASE(dereferenceInvalidIterator2); // #6572
|
||||||
TEST_CASE(dereference_auto);
|
TEST_CASE(dereference_auto);
|
||||||
|
|
||||||
TEST_CASE(readingEmptyStlContainer);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void check(const char code[], const bool inconclusive=false, const Standards::cppstd_t cppstandard=Standards::CPP11) {
|
void check(const char code[], const bool inconclusive=false, const Standards::cppstd_t cppstandard=Standards::CPP11) {
|
||||||
|
|
@ -3151,226 +3149,6 @@ private:
|
||||||
"}\n");
|
"}\n");
|
||||||
ASSERT_EQUALS("[test.cpp:18]: (error, inconclusive) Invalid iterator 'it' used.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:18]: (error, inconclusive) Invalid iterator 'it' used.\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
void readingEmptyStlContainer() {
|
|
||||||
check("void f() {\n"
|
|
||||||
" std::map<int, std::string> 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<int,std::string> 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<int,std::string> CMap;\n"
|
|
||||||
" CMap[1] = \"123\";\n"
|
|
||||||
" std::string strValue = CMap[1];"
|
|
||||||
"}\n",true);
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("std::vector<std::string> f() {\n"
|
|
||||||
" try {\n"
|
|
||||||
" std::vector<std::string> Vector;\n"
|
|
||||||
" std::vector<std::string> 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<std::string> Vector;\n"
|
|
||||||
" Vector.push_back(\"123\");\n"
|
|
||||||
" std::vector<std::string> 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<std::string,std::string> 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<int> v;\n"
|
|
||||||
" v.insert(1);\n"
|
|
||||||
" int i = v[0];\n"
|
|
||||||
"}\n", true);
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("void f() {\n"
|
|
||||||
" std::vector<int> 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<int> v = foo();\n"
|
|
||||||
" if(bar) v.clear();\n"
|
|
||||||
" int i = v.find(foobar);\n"
|
|
||||||
"}", true);
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("void f(std::set<int> 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<int> 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<int> 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<int> 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<int> v) {\n"
|
|
||||||
" v.clear();\n"
|
|
||||||
" foo(v.begin());\n"
|
|
||||||
"}", true);
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("void f() {\n"
|
|
||||||
" std::map<int, std::string> 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<int> 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<int> v;\n"
|
|
||||||
"void f() {\n"
|
|
||||||
" v.clear();\n"
|
|
||||||
" dostuff()\n"
|
|
||||||
" if (v.empty()) { }\n"
|
|
||||||
"}", true);
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("std::vector<int> 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<int> 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<int> 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<int> 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<int> vec;\n"
|
|
||||||
"};", true);
|
|
||||||
ASSERT_EQUALS("[test.cpp:6]: (style, inconclusive) Reading from empty STL container 'vec'\n", errout.str());
|
|
||||||
|
|
||||||
// #7560
|
|
||||||
check("std::vector<int> test;\n"
|
|
||||||
"std::vector<int>::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<std::string> &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)
|
REGISTER_TEST(TestStl)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue