Rewrote CheckStl::readingEmptyStlContainer(), resolving all its false positives shown on CppChecks own code

This commit is contained in:
PKEuS 2014-03-18 12:38:22 +01:00
parent 177bf6fcb3
commit af161fc361
2 changed files with 96 additions and 60 deletions

View File

@ -1563,81 +1563,93 @@ void CheckStl::readingEmptyStlContainer()
if (!_settings->isEnabled("style")) if (!_settings->isEnabled("style"))
return; return;
if (!_settings->inconclusive) // check only if inconclusive true; if (!_settings->inconclusive)
return; return;
std::map<unsigned int, bool> empty; //map to keep track of whether the StlContainer has been given value atleast once. std::set<unsigned int> empty_map; // empty std::map-like instances of STL containers
std::set<unsigned int> empty_nonmap; // empty non-std::map-like instances of STL containers
static const char *STL_CONTAINERS[] = {"map", "vector"}; static const char *MAP_STL_CONTAINERS[] = { "map", "multimap", "unordered_map", "unordered_multimap" };
static const char *NONMAP_STL_CONTAINERS[] = { "deque", "forward_list", "list", "multiset", "queue", "set", "stack", "string", "unordered_multiset", "unordered_set", "vector" };
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase(); const std::list<Scope>& scopeList = _tokenizer->getSymbolDatabase()->scopeList;
std::size_t varListSize = symbolDatabase->getVariableListSize();
for (std::size_t i = 1; i < varListSize; ++i) {
// to include stl containers declared in file in used map
const Variable* var = symbolDatabase->getVariableFromVarId(i);
if (var && var->isLocal() && var->isStlType(STL_CONTAINERS))
empty[var->declarationId()] = true;
}
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (std::list<Scope>::const_iterator i = scopeList.begin(); i != scopeList.end(); ++i) {
bool stl1_exist = false; if (i->type != Scope::eFunction)
if (tok->variable() && tok->variable()->isStlType(STL_CONTAINERS)) {
if (empty.find(tok->varId()) != empty.end())
stl1_exist = true;
}
// to check for various conditions for the way stl containers and variables can be used
if (Token::Match(tok, "%var% =|[")) {
const Token *tok2;
if (Token::simpleMatch(tok->next(), "="))
tok2 = tok->tokAt(2);
// to check cases like Cmap[1]; or i = Cmap[1] -- the right wld evaluate true when token reaches it.
else if (Token::Match(tok->next()," [") && Token::simpleMatch(tok->linkAt(1),"] ="))
tok2 = tok->next()->link()->tokAt(2);
else
continue; continue;
bool stl2_exist = false; for (const Token *tok = i->classStart->next(); tok != i->classEnd; tok = tok->next()) {
bool stl2_isEmpty = true; if (Token::Match(tok, "for|while|do|}")) { // Loops and end of scope clear the sets.
empty_map.clear();
empty_nonmap.clear();
}
// rhs variable if (!tok->varId())
if (tok2->variable() && tok2->variable()->isStlType(STL_CONTAINERS)) { continue;
if (empty.find(tok2->varId()) != empty.end()) {
stl2_exist = true; // Check whether a variable should be marked as "empty"
stl2_isEmpty = empty[tok2->varId()]; const Variable* var = tok->variable();
if (var) {
bool insert = false;
if (tok->variable()->nameToken() == tok && var->isLocal() && !var->isStatic()) { // Local variable declared
insert = !Token::Match(tok->tokAt(1), "[(=]"); // Only if not initialized
} else if (Token::Match(tok, "%var% . clear ( ) ;")) {
insert = true;
}
if (insert) {
if (var->isStlType(MAP_STL_CONTAINERS))
empty_map.insert(var->declarationId());
else if (var->isStlType(NONMAP_STL_CONTAINERS))
empty_nonmap.insert(var->declarationId());
continue;
} }
} }
// if both var are Stl Container... bool map = empty_map.find(tok->varId()) != empty_map.end();
if (stl1_exist && stl2_exist) { if (!map && empty_nonmap.find(tok->varId()) == empty_nonmap.end())
if (stl2_isEmpty) continue;
readingEmptyStlContainerError(tok2);
// 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)
if (map)
empty_map.erase(tok->varId());
else else
empty[tok->varId()] = false; empty_nonmap.erase(tok->varId());
} } else if (Token::Match(tok, "%var% [")) {
// Access through operator[]
// if only second var is Stl Container if (map) { // operator[] inserts an element, if used on a std::map
else if (stl2_exist && stl2_isEmpty) if (tok->strAt(-1) == "=")
readingEmptyStlContainerError(tok); readingEmptyStlContainerError(tok);
empty_map.erase(tok->varId());
// if only first var is stl container } else if ((map && empty_map.find(tok->varId()) != empty_map.end()) || (!map && empty_nonmap.find(tok->varId()) != empty_nonmap.end()))
else if (stl1_exist) readingEmptyStlContainerError(tok);
empty[tok->varId()] = false; } else if (Token::Match(tok, "%var% . %type% (")) {
} else if (stl1_exist && Token::Match(tok, "%var% [ %any% ] = %any%")) { // Member function call
empty[tok->varId()] = false; if (Token::Match(tok->tokAt(2), "find|at|data|c_str|back|front|empty|top|size|count")) // These functions read from the container
} else if (stl1_exist && Token::Match(tok, "%var% . %type% (")) { readingEmptyStlContainerError(tok);
//static const char STL_CONTAINER_INSERT_FUNCTIONS[] = "%var% . insert|push_back|push_front|assign|set|reset|emplace_after|insert_after|push ("; else if (map)
empty[tok->varId()] = false; empty_map.erase(tok->varId());
else
empty_nonmap.erase(tok->varId());
} else if (tok->strAt(-1) == "=") {
// Assignment (RHS)
readingEmptyStlContainerError(tok);
} else {
// Unknown usage. Assume it is initialized.
if (map)
empty_map.erase(tok->varId());
else
empty_nonmap.erase(tok->varId());
} }
} }
empty_map.clear();
empty_nonmap.clear();
}
} }
void CheckStl::readingEmptyStlContainerError(const Token *tok) void CheckStl::readingEmptyStlContainerError(const Token *tok)
{ {
reportError(tok, Severity::style, "reademptycontainer", "Reading from empty STL container", true); reportError(tok, Severity::style, "reademptycontainer", "Reading from empty STL container", true);
} }

View File

@ -2421,12 +2421,11 @@ private:
" try {\n" " try {\n"
" std::vector<std::string> Vector;\n" " std::vector<std::string> Vector;\n"
" std::vector<std::string> v2 = Vector;\n" " std::vector<std::string> v2 = Vector;\n"
" std::string strValue = v2[1]; \n" " std::string strValue = v2[1]; \n" // Do not complain here - this is a consecutive fault of the line above.
" }\n" " }\n"
" return Vector;\n" " return Vector;\n"
"}\n",true); "}\n",true);
ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Reading from empty STL container\n" ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Reading from empty STL container\n", errout.str());
"[test.cpp:5]: (style, inconclusive) Reading from empty STL container\n", errout.str());
check("f() {\n" check("f() {\n"
" try {\n" " try {\n"
@ -2453,13 +2452,38 @@ private:
"}\n", true); "}\n", true);
ASSERT_EQUALS("", errout.str()); 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::vector<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\n", errout.str());
check("void f() {\n" check("void f() {\n"
" std::map<int, std::string> CMap;\n" " std::map<int, std::string> CMap;\n"
" std::string strValue = CMap[1];\n" " std::string strValue = CMap[1];\n"
" std::string strValue2 = CMap[1];\n" " std::string strValue2 = CMap[1];\n"
"}\n", true); "}\n", true);
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container\n" ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container\n", errout.str());
"[test.cpp:4]: (style, inconclusive) Reading from empty STL container\n", errout.str());
} }
}; };