Refactorized CheckStl::if_find():

- Added support for find()-like functions to Library::Container
- Use <container> information from library
- Fixed false positive #6402
This commit is contained in:
PKEuS 2015-01-04 12:43:31 +01:00
parent f94243f85e
commit e06a4cdf00
6 changed files with 55 additions and 54 deletions

View File

@ -1975,6 +1975,18 @@
<function name="data" yields="buffer"/>
</access>
</container>
<container id="stdSet" startPattern="std :: set &lt;" inherits="stdContainer">
<access>
<function name="find" action="find"/>
</access>
</container>
<container id="stdMap" startPattern="std :: map &lt;" inherits="stdContainer">
<type templateParameter="1"/>
<access>
<function name="at" yields="at_index"/>
<function name="find" action="find"/>
</access>
</container>
<container id="stdAllString" inherits="stdContainer">
<type string="std-like"/>
@ -1989,6 +2001,7 @@
<function name="data" yields="buffer"/>
<function name="c_str" yields="buffer-nt"/>
<function name="length" yields="size"/>
<function name="find" action="find"/>
</access>
</container>
<container id="stdBasicString" startPattern="std :: basic_string &lt;" inherits="stdAllString">

View File

@ -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);

View File

@ -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);
}

View File

@ -147,7 +147,7 @@ public:
}
enum Action {
RESIZE, CLEAR, PUSH, POP,
RESIZE, CLEAR, PUSH, POP, FIND,
NO_ACTION
};
enum Yield {

View File

@ -297,6 +297,7 @@ private:
" <function name=\"data\" yields=\"buffer\"/>\n"
" <function name=\"c_str\" yields=\"buffer-nt\"/>\n"
" <function name=\"front\" yields=\"item\"/>\n"
" <function name=\"find\" action=\"find\"/>\n"
" </access>\n"
" </container>\n"
" <container id=\"B\" startPattern=\"std :: B &lt;\" inherits=\"A\">\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);

View File

@ -1561,6 +1561,12 @@ private:
"}");
ASSERT_EQUALS("", errout.str());
// ok (dereference, #6402)
check("void f(std::set<Foo> s) {\n"
" if (s.find(12).member) { }\n"
"}");
ASSERT_EQUALS("", errout.str());
// ---------------------------
// std::find