CheckBool::checkComparisonOfBoolExpressionWithInt: Rewrite the check using AST instead of token list
This commit is contained in:
parent
9b307cf8e0
commit
581886636d
|
@ -31,7 +31,7 @@ namespace {
|
||||||
|
|
||||||
static bool astIsBool(const Token *expr)
|
static bool astIsBool(const Token *expr)
|
||||||
{
|
{
|
||||||
return Token::Match(expr, "%comp%|%bool%|%oror%|&&");
|
return Token::Match(expr, "%comp%|%bool%|%oror%|&&|!");
|
||||||
}
|
}
|
||||||
|
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
|
@ -357,37 +357,6 @@ void CheckBool::assignBoolToPointerError(const Token *tok)
|
||||||
"Boolean value assigned to pointer.");
|
"Boolean value assigned to pointer.");
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* @brief Is the result of the LHS expression non-bool?
|
|
||||||
* @param tok last token in lhs
|
|
||||||
* @return true => lhs result is non-bool. false => lhs result type is unknown or bool
|
|
||||||
*/
|
|
||||||
static bool isNonBoolLHSExpr(const Token *tok)
|
|
||||||
{
|
|
||||||
// return value. only return true if we "know" it's a non-bool expression
|
|
||||||
bool nonBoolExpr = false;
|
|
||||||
|
|
||||||
for (; tok; tok = tok->previous()) {
|
|
||||||
if (tok->str() == ")") {
|
|
||||||
if (!Token::Match(tok->link()->previous(), "&&|%oror%|( ("))
|
|
||||||
tok = tok->link();
|
|
||||||
} else if (tok->str() == "(" || tok->str() == "[")
|
|
||||||
break;
|
|
||||||
else if (tok->isNumber())
|
|
||||||
nonBoolExpr = true;
|
|
||||||
else if (tok->isArithmeticalOp()) {
|
|
||||||
return true;
|
|
||||||
} else if (tok->isComparisonOp() || (tok->str() == "!" && tok->previous()->str()=="("))
|
|
||||||
return false;
|
|
||||||
else if (Token::Match(tok,"[;{}=?:&|^,]"))
|
|
||||||
break;
|
|
||||||
else if (Token::Match(tok, "&&|%oror%|and|or"))
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
return nonBoolExpr;
|
|
||||||
}
|
|
||||||
|
|
||||||
//-----------------------------------------------------------------------------
|
//-----------------------------------------------------------------------------
|
||||||
//-----------------------------------------------------------------------------
|
//-----------------------------------------------------------------------------
|
||||||
void CheckBool::checkComparisonOfBoolExpressionWithInt()
|
void CheckBool::checkComparisonOfBoolExpressionWithInt()
|
||||||
|
@ -401,6 +370,9 @@ void CheckBool::checkComparisonOfBoolExpressionWithInt()
|
||||||
for (std::size_t i = 0; i < functions; ++i) {
|
for (std::size_t i = 0; i < functions; ++i) {
|
||||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||||
|
if (!tok->isComparisonOp())
|
||||||
|
continue;
|
||||||
|
|
||||||
// Skip template parameters
|
// Skip template parameters
|
||||||
if (tok->str() == "<" && tok->link()) {
|
if (tok->str() == "<" && tok->link()) {
|
||||||
tok = tok->link();
|
tok = tok->link();
|
||||||
|
@ -408,96 +380,42 @@ void CheckBool::checkComparisonOfBoolExpressionWithInt()
|
||||||
}
|
}
|
||||||
|
|
||||||
const Token* numTok = 0;
|
const Token* numTok = 0;
|
||||||
const Token* opTok = 0;
|
const Token* boolExpr = 0;
|
||||||
char op = 0;
|
bool numInRhs;
|
||||||
if (Token::Match(tok, "&&|%oror% %any% ) %comp% %any%")) {
|
if (astIsBool(tok->astOperand1())) {
|
||||||
numTok = tok->tokAt(4);
|
boolExpr = tok->astOperand1();
|
||||||
opTok = tok->tokAt(3);
|
numTok = tok->astOperand2();
|
||||||
if (Token::Match(opTok, "<|>"))
|
numInRhs = true;
|
||||||
op = opTok->str()[0];
|
} else if (astIsBool(tok->astOperand2())) {
|
||||||
} else if (Token::Match(tok, "%any% %comp% ( %any% &&|%oror%")) {
|
boolExpr = tok->astOperand2();
|
||||||
numTok = tok;
|
numTok = tok->astOperand1();
|
||||||
opTok = tok->next();
|
numInRhs = false;
|
||||||
if (Token::Match(opTok, "<|>"))
|
} else {
|
||||||
op = opTok->str()[0]=='>'?'<':'>';
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
else if (Token::Match(tok, "! %var% %comp% %any%") && !isNonBoolLHSExpr(tok)) {
|
if (Token::Match(boolExpr,"%bool%"))
|
||||||
numTok = tok->tokAt(3);
|
// The CheckBool::checkComparisonOfBoolWithInt warns about this.
|
||||||
opTok = tok->tokAt(2);
|
continue;
|
||||||
if (Token::Match(opTok, "<|>"))
|
|
||||||
op = opTok->str()[0];
|
|
||||||
} else if (Token::Match(tok->previous(), "(|&&|%oror% %num% %comp% !")) {
|
|
||||||
const Token *rhs = tok->tokAt(3);
|
|
||||||
while (rhs) {
|
|
||||||
if (rhs->str() == "!") {
|
|
||||||
if (Token::simpleMatch(rhs, "! ("))
|
|
||||||
rhs = rhs->next()->link();
|
|
||||||
rhs = rhs->next();
|
|
||||||
} else if (rhs->isName() || rhs->isNumber())
|
|
||||||
rhs = rhs->next();
|
|
||||||
else
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
if (Token::Match(rhs, "&&|%oror%|)")) {
|
|
||||||
numTok = tok;
|
|
||||||
opTok = tok->next();
|
|
||||||
if (Token::Match(opTok, "<|>"))
|
|
||||||
op = opTok->str()[0]=='>'?'<':'>';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// boolean result in lhs compared with <|<=|>|>=
|
if (!numTok || !boolExpr)
|
||||||
else if (tok->isComparisonOp() && !Token::Match(tok,"==|!=") && !isNonBoolLHSExpr(tok->previous())) {
|
|
||||||
const Token *lhs = tok;
|
|
||||||
while (nullptr != (lhs = lhs->previous())) {
|
|
||||||
if ((lhs->isName() && !Token::Match(lhs,"or|and")) || lhs->isNumber())
|
|
||||||
continue;
|
continue;
|
||||||
if (lhs->isArithmeticalOp())
|
|
||||||
continue;
|
|
||||||
if (Token::Match(lhs, ")|]")) {
|
|
||||||
if (Token::Match(lhs->link()->previous(), "%var% ("))
|
|
||||||
lhs = lhs->link();
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
if (lhs && (lhs->isComparisonOp() || lhs->str() == "!")) {
|
|
||||||
if (_tokenizer->isCPP() && tok->str() == ">" &&
|
|
||||||
(Token::Match(lhs->previous(), "%var% <") || lhs->str() == ">"))
|
|
||||||
continue;
|
|
||||||
while (nullptr != (lhs = lhs->previous())) {
|
|
||||||
if ((lhs->isName() && lhs->str() != "return") || lhs->isNumber())
|
|
||||||
continue;
|
|
||||||
if (Token::Match(lhs,"[+-*/.]"))
|
|
||||||
continue;
|
|
||||||
if (Token::Match(lhs, ")|]")) {
|
|
||||||
lhs = lhs->previous();
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
std::string expression;
|
if (boolExpr->isOp() && numTok->isName() && Token::Match(tok, "==|!="))
|
||||||
for (const Token *t = lhs ? lhs->next() : _tokenizer->tokens(); t != tok; t = t->next()) {
|
// there is weird code such as: ((a<b)==c)
|
||||||
if (!expression.empty())
|
// but it is probably written this way by design.
|
||||||
expression += ' ';
|
continue;
|
||||||
expression += t->str();
|
|
||||||
}
|
|
||||||
|
|
||||||
comparisonOfBoolWithInvalidComparator(tok, expression);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (numTok && opTok) {
|
|
||||||
if (numTok->isNumber()) {
|
if (numTok->isNumber()) {
|
||||||
if (((numTok->str() != "0" && numTok->str() != "1") || !Token::Match(opTok, "!=|==")) && !((op == '<' && numTok->str() == "1") || (op == '>' && numTok->str() == "0")))
|
if (numTok->str() == "0" && Token::Match(tok, numInRhs ? ">|==|!=" : "<|==|!="))
|
||||||
|
continue;
|
||||||
|
if (numTok->str() == "1" && Token::Match(tok, numInRhs ? "<|==|!=" : ">|==|!="))
|
||||||
|
continue;
|
||||||
comparisonOfBoolExpressionWithIntError(tok, true);
|
comparisonOfBoolExpressionWithIntError(tok, true);
|
||||||
} else if (isNonBoolStdType(numTok->variable()))
|
} else if (isNonBoolStdType(numTok->variable()))
|
||||||
comparisonOfBoolExpressionWithIntError(tok, false);
|
comparisonOfBoolExpressionWithIntError(tok, false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void CheckBool::comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1)
|
void CheckBool::comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1)
|
||||||
|
|
|
@ -423,7 +423,7 @@ private:
|
||||||
ASSERT_EQUALS("",errout.str());
|
ASSERT_EQUALS("",errout.str());
|
||||||
|
|
||||||
check("void f(int a, int b, int c) { if (1 < !(a+b)) {} }");
|
check("void f(int a, int b, int c) { if (1 < !(a+b)) {} }");
|
||||||
TODO_ASSERT_EQUALS("error","",errout.str());
|
ASSERT_EQUALS("[test.cpp:1]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n",errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
void comparisonOfBoolExpressionWithInt3() {
|
void comparisonOfBoolExpressionWithInt3() {
|
||||||
|
@ -438,12 +438,12 @@ private:
|
||||||
check("void f() {\n"
|
check("void f() {\n"
|
||||||
" for(int i = 4; i > -1 < 5 ; --i) {}\n"
|
" for(int i = 4; i > -1 < 5 ; --i) {}\n"
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean value using relational operator (<, >, <= or >=).\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n", errout.str());
|
||||||
|
|
||||||
check("void f(int a, int b, int c) {\n"
|
check("void f(int a, int b, int c) {\n"
|
||||||
" return (a > b) < c;\n"
|
" return (a > b) < c;\n"
|
||||||
"}");
|
"}");
|
||||||
TODO_ASSERT_EQUALS("error", "", errout.str());
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
|
||||||
|
|
||||||
check("void f(int a, int b, int c) {\n"
|
check("void f(int a, int b, int c) {\n"
|
||||||
" return x(a > b) < c;\n"
|
" return x(a > b) < c;\n"
|
||||||
|
|
Loading…
Reference in New Issue