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

CheckInternal::checkRedundantTokCheck(): also catch patterns of the form if(!tok || !Token::Match(tok, "foo")).

This commit is contained in:
Matthias Krüger 2018-01-01 04:19:26 +01:00
parent 70817b3d4e
commit f2b2be2166
2 changed files with 56 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
lib/checkinternal.cpp
View File

@ -88,19 +88,37 @@ void CheckInternal::checkTokenMatchPatterns()
void CheckInternal::checkRedundantTokCheck()
{
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (!Token::Match(tok, "&& Token :: simpleMatch|Match|findsimplematch|findmatch ("))
continue;
if (Token::Match(tok, "&& Token :: simpleMatch|Match|findsimplematch|findmatch (")) {
// in code like
// if (tok->previous() && Token::match(tok->previous(), "bla")) {}
// the first tok->previous() check is redundant
const Token *astOp1 = tok->astOperand1();
const Token *astOp2 = getArguments(tok->tokAt(3))[0];
// std::cout << "\n astOp1: " << tok->astOperand1()->str() << ""; // tok
// std::cout << "\n astOp2: " << getArguments(tok->tokAt(3))[0]->str() << "\n "; // tok
// std::cout << "\n";
if (astOp1->expressionString() == astOp2->expressionString()) {
checkRedundantTokCheckError(astOp2);
}
// if (!tok || !Token::match(tok, "foo"))
} else if (Token::Match(tok, "%oror% ! Token :: simpleMatch|Match|findsimplematch|findmatch (")) {
// the first tok condition is negated
const Token *negTok = tok->next()->astParent()->astOperand1();
if (Token::simpleMatch(negTok, "!")) {
const Token *astOp1 = negTok->astOperand1();
const Token *astOp2 = getArguments(tok->tokAt(4))[0];
if (astOp1->expressionString() == astOp2->expressionString()) {
checkRedundantTokCheckError(astOp2);
}
}
}
}
}
}
void CheckInternal::checkRedundantTokCheckError(const Token* tok)
{

28
test/testinternal.cpp
View File

@ -467,6 +467,34 @@ private:
" if(tok->previous() && Token::Match(tok->previous()->previous(), \"5str% foobar\")) {};\n"
"}");
ASSERT_EQUALS("", errout.str());
// if a && fn(a) triggers, make sure !a || !fn(a) triggers as well!
check("void f() {\n"
" const Token *tok;\n"
" if(!tok || !Token::simpleMatch(tok, \"foobar\")) {};\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Unnecessary check of \"tok\", match-function already checks if it is null.\n", errout.str());
// if tok || !Token::simpleMatch...
check("void f() {\n"
" const Token *tok;\n"
" if(tok || !Token::simpleMatch(tok, \"foobar\")) {};\n"
"}");
ASSERT_EQUALS("", errout.str());
// if !tok || Token::simpleMatch...
check("void f() {\n"
" const Token *tok;\n"
" if(!tok || Token::simpleMatch(tok, \"foobar\")) {};\n"
"}");
ASSERT_EQUALS("", errout.str());
// something more complex
check("void f() {\n"
" const Token *tok;\n"
" if(!tok->previous()->previous() || !Token::simpleMatch(tok->previous()->previous(), \"foobar\")) {};\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Unnecessary check of \"tok->previous()->previous()\", match-function already checks if it is null.\n", errout.str());
}
};