From e06a4cdf00451a46c7e01b0393cc7af5b2e293c5 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sun, 4 Jan 2015 12:43:31 +0100 Subject: [PATCH] Refactorized CheckStl::if_find(): - Added support for find()-like functions to Library::Container - Use information from library - Fixed false positive #6402 --- cfg/std.cfg | 13 +++++++ lib/checkstl.cpp | 84 ++++++++++++++++---------------------------- lib/library.cpp | 2 ++ lib/library.h | 2 +- test/testlibrary.cpp | 2 ++ test/teststl.cpp | 6 ++++ 6 files changed, 55 insertions(+), 54 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index 4cd2a00a0..030a549e9 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -1975,6 +1975,18 @@ + + + + + + + + + + + + @@ -1989,6 +2001,7 @@ + diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index e36a581be..f1618ef97 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -759,6 +759,8 @@ static bool if_findCompare(const Token * const tokBack) return true; if (tok->isArithmeticalOp()) // result is used in some calculation return true; // TODO: check if there is a comparison of the result somewhere + if (tok->str() == ".") + return true; // Dereferencing is OK, the programmer might know that the element exists - TODO: An inconclusive warning might be appropriate if (tok->isAssignmentOp()) return if_findCompare(tok); // Go one step upwards in the AST return false; @@ -782,72 +784,48 @@ void CheckStl::if_find() tok = tok->next(); for (const Token* const end = tok->link(); tok != end; tok = (tok == end) ? end : tok->next()) { - if (Token::Match(tok, "%var% . find (")) { - const Variable *var = tok->variable(); - if (var) { - if (if_findCompare(tok->tokAt(3))) - continue; + const Token* funcTok = nullptr; + const Library::Container* container = nullptr; - // Is the variable a std::string or STL container? - const Token * decl = var->typeStartToken(); - // stl container - if (warning && Token::Match(decl, "std :: %var% <") && Token::Match(decl->linkAt(3), "> !!::")) - if_findError(tok, false); - else if (performance && var->isStlStringType()) - if_findError(tok, true); - } + if (tok->variable() && Token::Match(tok, "%var% . %var% (")) { + container = _settings->library.detectContainer(tok->variable()->typeStartToken()); + funcTok = tok->tokAt(2); } - //check also for vector-like or pointer containers + // check also for vector-like or pointer containers else if (tok->variable() && tok->astParent() && (tok->astParent()->str() == "*" || tok->astParent()->str() == "[")) { const Token *tok2 = tok->astParent(); - if (!Token::simpleMatch(tok2->astParent(), ". find (")) + if (!Token::Match(tok2->astParent(), ". %var% (")) continue; - if (if_findCompare(tok2->astParent()->tokAt(2))) - continue; + funcTok = tok2->astParent()->next(); - const Variable *var = tok->variable(); - if (var) { - //pretty bad limitation.. but it is there in order to avoid - //own implementations of 'find' or any container - if (!var->isStlType()) - continue; + if (tok->variable()->isArrayOrPointer()) + container = _settings->library.detectContainer(tok->variable()->typeStartToken()); + else { // Container of container - find the inner container + container = _settings->library.detectContainer(tok->variable()->typeStartToken()); // outer container + tok2 = Token::findsimplematch(tok->variable()->typeStartToken(), "<", tok->variable()->typeEndToken()); + if (container && container->type_templateArgNo >= 0 && tok2) { + tok2 = tok2->next(); + for (int i = 0; i < container->type_templateArgNo; i++) + tok2 = tok2->nextTemplateArgument(); - // Is the variable a std::string or STL container? - const Token * decl = var->typeStartToken(); - const unsigned int varid = tok->varId(); - - decl = decl->tokAt(2); - - if (Token::Match(decl, "%var% <")) { - decl = decl->tokAt(2); - //stl-like - if (warning && Token::Match(decl, "std :: %var% < %type% > > &| %varid%", varid)) - if_findError(tok, false); - //not stl-like, then let's hope it's a pointer or an array - else if (Token::Match(decl, "%type% >")) { - decl = decl->tokAt(2); - if (warning && (Token::Match(decl, "* &| %varid%", varid) || - Token::Match(decl, "&| %varid% [ ]| %any% ]|", varid))) - if_findError(tok, false); - } - - else if (performance && Token::Match(decl, "std :: string|wstring > &| %varid%", varid)) - if_findError(tok, true); - } - - else if (performance && var->isStlStringType()) { - decl = decl->next(); - if (Token::Match(decl, "* &| %varid%", varid) || - Token::Match(decl, "&| %varid% [ ]| %any% ]|", varid)) - if_findError(tok, true); - } + container = _settings->library.detectContainer(tok2); // innner container + } else + container = nullptr; } } - else if (warning && Token::Match(tok, "std :: find|find_if (")) { + if (container && container->getAction(funcTok->str()) == Library::Container::FIND) { + if (if_findCompare(funcTok->next())) + continue; + + if (warning && !container->stdStringLike) + if_findError(tok, false); + else if (performance && container->stdStringLike) + if_findError(tok, true); + } else if (warning && Token::Match(tok, "std :: find|find_if (")) { // check that result is checked properly if (!if_findCompare(tok->tokAt(3))) { if_findError(tok, false); diff --git a/lib/library.cpp b/lib/library.cpp index ea900f631..2d0730b25 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -434,6 +434,8 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) action = Container::PUSH; else if (actionName == "pop") action = Container::POP; + else if (actionName == "find") + action = Container::FIND; else return Error(BAD_ATTRIBUTE_VALUE, actionName); } diff --git a/lib/library.h b/lib/library.h index a1f787e02..a906fb90d 100644 --- a/lib/library.h +++ b/lib/library.h @@ -147,7 +147,7 @@ public: } enum Action { - RESIZE, CLEAR, PUSH, POP, + RESIZE, CLEAR, PUSH, POP, FIND, NO_ACTION }; enum Yield { diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index 7b35de41b..019f15007 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -297,6 +297,7 @@ private: " \n" " \n" " \n" + " \n" " \n" " \n" " \n" @@ -336,6 +337,7 @@ private: ASSERT_EQUALS(Library::Container::CLEAR, A.getAction("clear")); ASSERT_EQUALS(Library::Container::PUSH, A.getAction("push_back")); ASSERT_EQUALS(Library::Container::POP, A.getAction("pop_back")); + ASSERT_EQUALS(Library::Container::FIND, A.getAction("find")); ASSERT_EQUALS(Library::Container::NO_ACTION, A.getAction("foo")); ASSERT_EQUALS(B.type_templateArgNo, 1); diff --git a/test/teststl.cpp b/test/teststl.cpp index d9c5cfe2e..90159e1bf 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1561,6 +1561,12 @@ private: "}"); ASSERT_EQUALS("", errout.str()); + // ok (dereference, #6402) + check("void f(std::set s) {\n" + " if (s.find(12).member) { }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + // --------------------------- // std::find