diff --git a/cfg/std.cfg b/cfg/std.cfg index 63812285f..5d3109d7e 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -3959,7 +3959,7 @@ tmpfile fclose - + @@ -3978,7 +3978,7 @@ - + @@ -3992,7 +3992,7 @@ - + @@ -4001,6 +4001,14 @@ + + + + + + + + @@ -4013,7 +4021,7 @@ - + @@ -4028,7 +4036,7 @@ - + diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 3dcd46b30..9c7b217e5 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -597,43 +597,33 @@ void CheckStl::invalidPointerError(const Token *tok, const std::string &func, co void CheckStl::stlBoundaries() { const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase(); - const std::size_t functions = symbolDatabase->functionScopes.size(); - for (std::size_t i = 0; i < functions; ++i) { - const Scope * scope = symbolDatabase->functionScopes[i]; - for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { - // Declaring iterator.. - if (tok->str() == "<" && Token::Match(tok->previous(), "bitset|list|forward_list|map|multimap|multiset|priority_queue|queue|set|stack|hash_map|hash_multimap|hash_set|unordered_map|unordered_multimap|unordered_set|unordered_multiset")) { - if (!tok->link()) - continue; - const std::string& container_name(tok->strAt(-1)); - tok = tok->link(); + for (unsigned int iteratorId = 1; iteratorId < symbolDatabase->getVariableListSize(); iteratorId++) { + const Variable* var = symbolDatabase->getVariableFromVarId(iteratorId); + if (!var || !var->scope() || !var->scope()->isExecutable()) + continue; - if (Token::Match(tok, "> :: iterator|const_iterator %var% =|;")) { - const unsigned int iteratorid(tok->tokAt(3)->varId()); + const Library::Container* container = _settings->library.detectContainer(var->typeStartToken(), true); + if (!container || container->opLessAllowed) + continue; - // Using "iterator < ..." is not allowed - const Token* const end = tok->scope()->classEnd; - for (const Token *tok2 = tok; tok2 != end; tok2 = tok2->next()) { - if (Token::Match(tok2, "!!* %varid% <", iteratorid)) { - stlBoundariesError(tok2, container_name); - } else if (Token::Match(tok2, "> %varid% !!.", iteratorid)) { - stlBoundariesError(tok2, container_name); - } - } - } + const Token* const end = var->scope()->classEnd; + for (const Token *tok = var->nameToken(); tok != end; tok = tok->next()) { + if (Token::Match(tok, "!!* %varid% <", var->declarationId())) { + stlBoundariesError(tok); + } else if (Token::Match(tok, "> %varid% !!.", var->declarationId())) { + stlBoundariesError(tok); } } } } // Error message for bad boundary usage.. -void CheckStl::stlBoundariesError(const Token *tok, const std::string &container_name) +void CheckStl::stlBoundariesError(const Token *tok) { reportError(tok, Severity::error, "stlBoundaries", - "Dangerous iterator comparison using operator< on 'std::" + container_name + "'.\n" - "Iterator of container 'std::" + container_name + "' compared with operator<. " - "This is dangerous since the order of items in the container is not guaranteed. " - "One should use operator!= instead to compare iterators."); + "Dangerous comparison using operator< on iterator.\n" + "Iterator compared with operator<. This is dangerous since the order of items in the " + "container is not guaranteed. One should use operator!= instead to compare iterators."); } static bool if_findCompare(const Token * const tokBack) diff --git a/lib/checkstl.h b/lib/checkstl.h index bed6b5abb..2316f84f6 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -165,7 +165,7 @@ private: void mismatchingContainersError(const Token *tok); void invalidIteratorError(const Token *tok, const std::string &func, const std::string &iterator_name); void invalidPointerError(const Token *tok, const std::string &func, const std::string &pointer_name); - void stlBoundariesError(const Token *tok, const std::string &container_name); + void stlBoundariesError(const Token *tok); void if_findError(const Token *tok, bool str); void sizeError(const Token *tok); void redundantIfRemoveError(const Token *tok); @@ -194,7 +194,7 @@ private: c.stlOutOfBoundsError(0, "i", "foo", false); c.invalidIteratorError(0, "push_back|push_front|insert", "iterator"); c.invalidPointerError(0, "push_back", "pointer"); - c.stlBoundariesError(0, "container"); + c.stlBoundariesError(0); c.if_findError(0, false); c.if_findError(0, true); c.string_c_strError(0); diff --git a/lib/library.cpp b/lib/library.cpp index 342e2bc36..7e61df1cb 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -322,6 +322,12 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) const char* const endPattern = node->Attribute("endPattern"); if (endPattern) container.endPattern = endPattern; + const char* const itEndPattern = node->Attribute("itEndPattern"); + if (itEndPattern) + container.itEndPattern = itEndPattern; + const char* const opLessAllowed = node->Attribute("opLessAllowed"); + if (opLessAllowed) + container.opLessAllowed = std::string(opLessAllowed) == "true"; for (const tinyxml2::XMLElement *containerNode = node->FirstChildElement(); containerNode; containerNode = containerNode->NextSiblingElement()) { const std::string containerNodeName = containerNode->Name(); @@ -778,7 +784,7 @@ bool Library::isScopeNoReturn(const Token *end, std::string *unknownFunc) const return false; } -const Library::Container* Library::detectContainer(const Token* typeStart) const +const Library::Container* Library::detectContainer(const Token* typeStart, bool iterator) const { for (std::map::const_iterator i = containers.begin(); i != containers.end(); ++i) { const Container& container = i->second; @@ -786,12 +792,13 @@ const Library::Container* Library::detectContainer(const Token* typeStart) const continue; if (Token::Match(typeStart, container.startPattern.c_str())) { - if (container.endPattern.empty()) + if (!iterator && container.endPattern.empty()) // If endPattern is undefined, it will always match, but itEndPattern has to be defined. return &container; for (const Token* tok = typeStart; tok && !tok->varId(); tok = tok->next()) { if (tok->link()) { - if (Token::Match(tok->link(), container.endPattern.c_str())) + const std::string& endPattern = iterator ? container.itEndPattern : container.endPattern; + if (Token::Match(tok->link(), endPattern.c_str())) return &container; break; } diff --git a/lib/library.h b/lib/library.h index 60c9530f5..d03743a59 100644 --- a/lib/library.h +++ b/lib/library.h @@ -137,7 +137,8 @@ public: type_templateArgNo(-1), size_templateArgNo(-1), arrayLike_indexOp(false), - stdStringLike(false) { + stdStringLike(false), + opLessAllowed(true) { } enum Action { @@ -152,12 +153,13 @@ public: Action action; Yield yield; }; - std::string startPattern, endPattern; + std::string startPattern, endPattern, itEndPattern; std::map functions; int type_templateArgNo; int size_templateArgNo; bool arrayLike_indexOp; bool stdStringLike; + bool opLessAllowed; Action getAction(const std::string& function) const { std::map::const_iterator i = functions.find(function); @@ -174,7 +176,7 @@ public: } }; std::map containers; - const Container* detectContainer(const Token* typeStart) const; + const Container* detectContainer(const Token* typeStart, bool iterator = false) const; class ArgumentChecks { public: diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index 52403d1c2..45e3b8f7f 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -382,7 +382,7 @@ private: void container() const { const char xmldata[] = "\n" "\n" - " \n" + " \n" " \n" " \n" " \n" @@ -402,7 +402,7 @@ private: " \n" " \n" " \n" - " \n" + " \n" " \n" // Inherits all but templateParameter " \n" " \n" @@ -422,8 +422,10 @@ private: ASSERT_EQUALS(A.size_templateArgNo, 4); ASSERT_EQUALS(A.startPattern, "std :: A <"); ASSERT_EQUALS(A.endPattern, "> !!::"); + ASSERT_EQUALS(A.itEndPattern, "> :: iterator"); ASSERT_EQUALS(A.stdStringLike, false); ASSERT_EQUALS(A.arrayLike_indexOp, false); + ASSERT_EQUALS(A.opLessAllowed, true); ASSERT_EQUALS(Library::Container::SIZE, A.getYield("size")); ASSERT_EQUALS(Library::Container::EMPTY, A.getYield("empty")); ASSERT_EQUALS(Library::Container::AT_INDEX, A.getYield("at")); @@ -444,7 +446,9 @@ private: ASSERT_EQUALS(B.size_templateArgNo, 3); ASSERT_EQUALS(B.startPattern, "std :: B <"); ASSERT_EQUALS(B.endPattern, "> !!::"); + ASSERT_EQUALS(B.itEndPattern, "> :: iterator"); ASSERT_EQUALS(B.functions.size(), A.functions.size()); + ASSERT_EQUALS(B.opLessAllowed, false); ASSERT(C.functions.empty()); ASSERT_EQUALS(C.type_templateArgNo, -1); diff --git a/test/teststl.cpp b/test/teststl.cpp index bf5647a68..e8cf40500 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1406,8 +1406,7 @@ private: void stlBoundaries1() { const std::string stlCont[] = { - "list", "set", "multiset", "map", - "multimap", "hash_map", "hash_multimap", "hash_set" + "list", "set", "multiset", "map", "multimap" }; for (size_t i = 0; i < getArraylength(stlCont); ++i) { @@ -1418,16 +1417,16 @@ private: " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous iterator comparison using operator< on 'std::" + stlCont[i] + "'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous comparison using operator< on iterator.\n", errout.str()); } check("void f() {\n" " std::forward_list::iterator it;\n" " for (it = ab.begin(); ab.end() > it; ++it) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous iterator comparison using operator< on 'std::forward_list'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous comparison using operator< on iterator.\n", errout.str()); - // #5926 no FP Dangerous iterator comparison using operator< on 'std::deque'. + // #5926 no FP Dangerous comparison using operator< on iterator on std::deque check("void f() {\n" " std::deque::iterator it;\n" " for (it = ab.begin(); ab.end() > it; ++it) {}\n" @@ -1473,7 +1472,7 @@ private: " std::forward_list>>::iterator it;\n" " for (it = ab.begin(); ab.end() > it; ++it) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous iterator comparison using operator< on 'std::forward_list'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous comparison using operator< on iterator.\n", errout.str()); // don't crash check("void f() {\n" @@ -1487,7 +1486,7 @@ private: " for (it = ab.begin(); ab.end() > it; ++it) {}\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous iterator comparison using operator< on 'std::forward_list'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous comparison using operator< on iterator.\n", errout.str()); } void stlBoundaries5() {