From 61767d49322f418b0e6fba299c81274f9bcda5b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 6 Jan 2018 16:08:12 +0100 Subject: [PATCH] Fixed #8125 (incorrect error iterators) --- lib/checkstl.cpp | 46 +++++++++++++++++++++++++++++++--------------- test/teststl.cpp | 9 +++++++++ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index b6b6cb2d5..5a0efb27b 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -106,6 +106,19 @@ static bool isIterator(const Variable *var) 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() { const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); @@ -121,7 +134,7 @@ void CheckStl::iterators() const Scope* invalidationScope = nullptr; // The container this iterator can be used with - const Variable* container = nullptr; + const Token* containerToken = nullptr; const Scope* containerAssignScope = nullptr; // When "validatingToken" is reached the validIterator is set to true @@ -135,7 +148,7 @@ void CheckStl::iterators() if (invalidationScope && tok2 == invalidationScope->classEnd) validIterator = true; // Assume that the iterator becomes valid again 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) { validIterator = true; @@ -144,14 +157,14 @@ void CheckStl::iterators() } // 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; if (tok2->astOperand1()->varId() == iteratorId) other = tok2->astOperand2()->tokAt(-3); else if (tok2->astOperand2()->varId() == iteratorId) other = tok2->astOperand1()->tokAt(-3); - if (Token::Match(other, "%name% . end|rend|cend|crend ( )") && other->varId() != container->declarationId()) - iteratorsError(tok2, container->name(), other->str()); + if (Token::Match(other, "%name% . end|rend|cend|crend ( )") && other->varId() != containerToken->varId()) + iteratorsError(tok2, getContainerName(containerToken), getContainerName(other)); } // 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 // report an error - if (container && tok2->varId() != container->declarationId()) { + if (containerToken && tok2->varId() != containerToken->varId()) { // skip error message if container is a set.. const Variable *variableInfo = tok2->variable(); const Token *decltok = variableInfo ? variableInfo->typeStartToken() : nullptr; @@ -187,7 +200,7 @@ void CheckStl::iterators() if (!par2 || par2->nextArgument()) continue; while (par2->str() != ")") { - if (par2->varId() == container->declarationId()) + if (par2->varId() == containerToken->varId()) break; if (isIterator(par2->variable())) break; // TODO: check if iterator points at same container @@ -200,11 +213,11 @@ void CheckStl::iterators() } // 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 - 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; eraseToken = tok2; invalidationScope = tok2->scope(); @@ -219,18 +232,21 @@ void CheckStl::iterators() else if (Token::Match(tok2, "%varid% = %name% .", iteratorId) && Token::simpleMatch(skipMembers(tok2->tokAt(2)), "erase (")) { // the returned iterator is valid - validatingToken = tok2->linkAt(5); - tok2 = tok2->tokAt(5); + validatingToken = skipMembers(tok2->tokAt(2))->linkAt(1); + tok2 = validatingToken->link(); } // Reassign the iterator - else if (Token::Match(tok2, "%varid% = %name% . begin|rbegin|cbegin|crbegin|find (", iteratorId)) { - validatingToken = tok2->linkAt(5); - container = tok2->tokAt(2)->variable(); + else if (Token::Match(tok2, "%varid% = %name% .", iteratorId) && + Token::Match(skipMembers(tok2->tokAt(2)), "begin|rbegin|cbegin|crbegin|find (")) { + validatingToken = skipMembers(tok2->tokAt(2))->linkAt(1); + containerToken = skipMembers(tok2->tokAt(2))->tokAt(-2); + if (containerToken->varId() == 0) + containerToken = nullptr; containerAssignScope = tok2->scope(); // skip ahead - tok2 = tok2->tokAt(5); + tok2 = validatingToken->link(); } // Reassign the iterator diff --git a/test/teststl.cpp b/test/teststl.cpp index 6249160ac..d5d396e15 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -182,6 +182,15 @@ private: "}"); ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str()); + check("struct C { std::list l1; void func(); };\n" + "void C::func() {\n" + " std::list::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 check("void f()\n" "{\n"