walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fixed #8125 (incorrect error iterators)

This commit is contained in:
Daniel Marjamäki 2018-01-06 16:08:12 +01:00
parent 98b45ffbc0
commit 61767d4932
2 changed files with 40 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
lib/checkstl.cpp
View File

@ -106,6 +106,19 @@ static bool isIterator(const Variable *var)
return true; return true;
} }
static std::string getContainerName(const Token *containerToken)
{
if (!containerToken)
return std::string();
std::string ret(containerToken->str());
for (const Token *nametok = containerToken; nametok; nametok = nametok->tokAt(-2)) {
if (!Token::Match(nametok->tokAt(-2), "%name% ."))
break;
ret = nametok->strAt(-2) + '.' + ret;
}
return ret;
}
void CheckStl::iterators() void CheckStl::iterators()
{ {
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
@ -121,7 +134,7 @@ void CheckStl::iterators()
const Scope* invalidationScope = nullptr; const Scope* invalidationScope = nullptr;
// The container this iterator can be used with // The container this iterator can be used with
const Variable* container = nullptr; const Token* containerToken = nullptr;
const Scope* containerAssignScope = nullptr; const Scope* containerAssignScope = nullptr;
// When "validatingToken" is reached the validIterator is set to true // When "validatingToken" is reached the validIterator is set to true
@ -135,7 +148,7 @@ void CheckStl::iterators()
if (invalidationScope && tok2 == invalidationScope->classEnd) if (invalidationScope && tok2 == invalidationScope->classEnd)
validIterator = true; // Assume that the iterator becomes valid again validIterator = true; // Assume that the iterator becomes valid again
if (containerAssignScope && tok2 == containerAssignScope->classEnd) if (containerAssignScope && tok2 == containerAssignScope->classEnd)
container = nullptr; // We don't know which containers might be used with the iterator containerToken = nullptr; // We don't know which containers might be used with the iterator
if (tok2 == validatingToken) { if (tok2 == validatingToken) {
validIterator = true; validIterator = true;
@ -144,14 +157,14 @@ void CheckStl::iterators()
} }
// Is iterator compared against different container? // Is iterator compared against different container?
if (tok2->isComparisonOp() && container && tok2->astOperand1() && tok2->astOperand2()) { if (tok2->isComparisonOp() && containerToken && tok2->astOperand1() && tok2->astOperand2()) {
const Token *other = nullptr; const Token *other = nullptr;
if (tok2->astOperand1()->varId() == iteratorId) if (tok2->astOperand1()->varId() == iteratorId)
other = tok2->astOperand2()->tokAt(-3); other = tok2->astOperand2()->tokAt(-3);
else if (tok2->astOperand2()->varId() == iteratorId) else if (tok2->astOperand2()->varId() == iteratorId)
other = tok2->astOperand1()->tokAt(-3); other = tok2->astOperand1()->tokAt(-3);
if (Token::Match(other, "%name% . end|rend|cend|crend ( )") && other->varId() != container->declarationId()) if (Token::Match(other, "%name% . end|rend|cend|crend ( )") && other->varId() != containerToken->varId())
iteratorsError(tok2, container->name(), other->str()); iteratorsError(tok2, getContainerName(containerToken), getContainerName(other));
} }
// Is the iterator used in a insert/erase operation? // Is the iterator used in a insert/erase operation?
@ -169,7 +182,7 @@ void CheckStl::iterators()
// If insert/erase is used on different container then // If insert/erase is used on different container then
// report an error // report an error
if (container && tok2->varId() != container->declarationId()) { if (containerToken && tok2->varId() != containerToken->varId()) {
// skip error message if container is a set.. // skip error message if container is a set..
const Variable *variableInfo = tok2->variable(); const Variable *variableInfo = tok2->variable();
const Token *decltok = variableInfo ? variableInfo->typeStartToken() : nullptr; const Token *decltok = variableInfo ? variableInfo->typeStartToken() : nullptr;
@ -187,7 +200,7 @@ void CheckStl::iterators()
if (!par2 || par2->nextArgument()) if (!par2 || par2->nextArgument())
continue; continue;
while (par2->str() != ")") { while (par2->str() != ")") {
if (par2->varId() == container->declarationId()) if (par2->varId() == containerToken->varId())
break; break;
if (isIterator(par2->variable())) if (isIterator(par2->variable()))
break; // TODO: check if iterator points at same container break; // TODO: check if iterator points at same container
@ -200,11 +213,11 @@ void CheckStl::iterators()
} }
// Show error message, mismatching iterator is used. // Show error message, mismatching iterator is used.
iteratorsError(tok2, container->name(), tok2->str()); iteratorsError(tok2, getContainerName(containerToken), getContainerName(tok2));
} }
// invalidate the iterator if it is erased // invalidate the iterator if it is erased
else if (tok2->strAt(2) == "erase" && (tok2->strAt(4) != "*" || (container && tok2->varId() == container->declarationId()))) { else if (tok2->strAt(2) == "erase" && (tok2->strAt(4) != "*" || (containerToken && tok2->varId() == containerToken->varId()))) {
validIterator = false; validIterator = false;
eraseToken = tok2; eraseToken = tok2;
invalidationScope = tok2->scope(); invalidationScope = tok2->scope();
@ -219,18 +232,21 @@ void CheckStl::iterators()
else if (Token::Match(tok2, "%varid% = %name% .", iteratorId) && else if (Token::Match(tok2, "%varid% = %name% .", iteratorId) &&
Token::simpleMatch(skipMembers(tok2->tokAt(2)), "erase (")) { Token::simpleMatch(skipMembers(tok2->tokAt(2)), "erase (")) {
// the returned iterator is valid // the returned iterator is valid
validatingToken = tok2->linkAt(5); validatingToken = skipMembers(tok2->tokAt(2))->linkAt(1);
tok2 = tok2->tokAt(5); tok2 = validatingToken->link();
} }
// Reassign the iterator // Reassign the iterator
else if (Token::Match(tok2, "%varid% = %name% . begin|rbegin|cbegin|crbegin|find (", iteratorId)) { else if (Token::Match(tok2, "%varid% = %name% .", iteratorId) &&
validatingToken = tok2->linkAt(5); Token::Match(skipMembers(tok2->tokAt(2)), "begin|rbegin|cbegin|crbegin|find (")) {
container = tok2->tokAt(2)->variable(); validatingToken = skipMembers(tok2->tokAt(2))->linkAt(1);
containerToken = skipMembers(tok2->tokAt(2))->tokAt(-2);
if (containerToken->varId() == 0)
containerToken = nullptr;
containerAssignScope = tok2->scope(); containerAssignScope = tok2->scope();
// skip ahead // skip ahead
tok2 = tok2->tokAt(5); tok2 = validatingToken->link();
} }
// Reassign the iterator // Reassign the iterator

9
test/teststl.cpp
View File

@ -182,6 +182,15 @@ private:
"}"); "}");
ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
check("struct C { std::list<int> l1; void func(); };\n"
"void C::func() {\n"
" std::list<int>::iterator it;\n"
" for (it = l1.begin(); it != l1.end(); ++it) { }\n"
" C c;\n"
" for (it = c.l1.begin(); it != c.l1.end(); ++it) { }\n"
"}");
ASSERT_EQUALS("", errout.str());
// Same check with reverse iterator // Same check with reverse iterator
check("void f()\n" check("void f()\n"
"{\n" "{\n"