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

Improved CheckOther::checkComparisonOfBoolWithInt and CheckOther::checkComparisonOfBoolExpressionWithInt: 

- Added support for comparision of bool constant with number constant (-> fixed #1877) and integer variable with boolean expression
- Moved a check from checkComparisonOfBoolWithInt to checkComparisonOfBoolExpressionWithInt
- Generalized some patterns
- Made error message more accurate concnerning the "neither 0 nor 1" part.
- Reduced number of Token::Match calls
This commit is contained in:
PKEuS 2012-04-02 15:45:51 +02:00
parent fc8749e952
commit c1fc7a2218
3 changed files with 166 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

149
lib/checkother.cpp
View File

@ -1671,63 +1671,55 @@ void CheckOther::checkComparisonOfBoolWithInt()
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %num%")) { // Comparing variable with number
const Token *varTok = tok;
const Token *numTok = tok->tokAt(2);
if (isBool(symbolDatabase->getVariableFromVarId(varTok->varId())) && // Variable has to be a boolean
((tok->strAt(1) != "==" && tok->strAt(1) != "!=") ||
(MathLib::toLongNumber(numTok->str()) != 0 && MathLib::toLongNumber(numTok->str()) != 1))) { // == 0 and != 0 are allowed, for C also == 1 and != 1
comparisonOfBoolWithIntError(varTok, numTok->str());
}
} else if (Token::Match(tok, "%num% >|>=|==|!=|<=|< %var%")) { // Comparing number with variable
const Token *varTok = tok->tokAt(2);
const Token *numTok = tok;
if (isBool(symbolDatabase->getVariableFromVarId(varTok->varId())) && // Variable has to be a boolean
((tok->strAt(1) != "==" && tok->strAt(1) != "!=") ||
(MathLib::toLongNumber(numTok->str()) != 0 && MathLib::toLongNumber(numTok->str()) != 1))) { // == 0 and != 0 are allowed, for C also == 1 and != 1
comparisonOfBoolWithIntError(varTok, numTok->str());
}
} else if (Token::Match(tok, "true|false >|>=|==|!=|<=|< %var%")) { // Comparing boolean constant with variable
const Token *varTok = tok->tokAt(2);
const Token *constTok = tok;
if (isNonBoolStdType(symbolDatabase->getVariableFromVarId(varTok->varId()))) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(varTok, constTok->str());
}
} else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< true|false")) { // Comparing variable with boolean constant
const Token *varTok = tok;
const Token *constTok = tok->tokAt(2);
if (isNonBoolStdType(symbolDatabase->getVariableFromVarId(varTok->varId()))) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(varTok, constTok->str());
}
} else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %var%") &&
!Token::Match(tok->tokAt(3), ".|::|(")) { // Comparing two variables, one of them boolean, one of them integer
const Variable* var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
const Variable* var2 = symbolDatabase->getVariableFromVarId(tok->varId());
if (isBool(var1) && isNonBoolStdType(var2)) // Comparing boolean with non-bool standard type
comparisonOfBoolWithIntError(tok, var1->name());
else if (isNonBoolStdType(var1) && isBool(var2)) // Comparing non-bool standard type with boolean
comparisonOfBoolWithIntError(tok, var2->name());
} else if (Token::Match(tok, "( ! %var% ==|!= %num% )")) {
const Token *numTok = tok->tokAt(4);
if (numTok && numTok->str() != "0" && numTok->str() != "1") {
comparisonOfBoolWithIntError(numTok, "!"+tok->strAt(2));
}
} else if (Token::Match(tok, "( %num% ==|!= ! %var% )")) {
const Token *numTok = tok->next();
if (numTok && numTok->str() != "0" && numTok->str() != "1") {
comparisonOfBoolWithIntError(numTok, "!"+tok->strAt(4));
if (Token::Match(tok->next(), ">|>=|==|!=|<=|<") && (!tok->previous() || !tok->previous()->isArithmeticalOp()) && (!tok->tokAt(3) || !tok->tokAt(3)->isArithmeticalOp())) {
const Token* const right = tok->tokAt(2);
if ((tok->varId() && right->isNumber()) || (tok->isNumber() && right->varId())) { // Comparing variable with number
const Token* varTok = tok;
const Token* numTok = right;
if (tok->isNumber() && right->varId()) // num with var
std::swap(varTok, numTok);
if (isBool(symbolDatabase->getVariableFromVarId(varTok->varId())) && // Variable has to be a boolean
((tok->strAt(1) != "==" && tok->strAt(1) != "!=") ||
(MathLib::toLongNumber(numTok->str()) != 0 && MathLib::toLongNumber(numTok->str()) != 1))) { // == 0 and != 0 are allowed, for C also == 1 and != 1
comparisonOfBoolWithIntError(varTok, numTok->str(), tok->strAt(1) == "==" || tok->strAt(1) == "!=");
}
} else if (tok->isBoolean() && right->varId()) { // Comparing boolean constant with variable
if (isNonBoolStdType(symbolDatabase->getVariableFromVarId(right->varId()))) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(right, tok->str(), false);
}
} else if (tok->varId() && right->isBoolean()) { // Comparing variable with boolean constant
if (isNonBoolStdType(symbolDatabase->getVariableFromVarId(tok->varId()))) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(tok, right->str(), false);
}
} else if (tok->isNumber() && right->isBoolean()) { // number constant with boolean constant
comparisonOfBoolWithIntError(tok, right->str(), false);
} else if (tok->isBoolean() && right->isNumber()) { // number constant with boolean constant
comparisonOfBoolWithIntError(tok, tok->str(), false);
} else if (tok->varId() && right->varId()) { // Comparing two variables, one of them boolean, one of them integer
const Variable* var1 = symbolDatabase->getVariableFromVarId(right->varId());
const Variable* var2 = symbolDatabase->getVariableFromVarId(tok->varId());
if (isBool(var1) && isNonBoolStdType(var2)) // Comparing boolean with non-bool standard type
comparisonOfBoolWithIntError(tok, var1->name(), false);
else if (isNonBoolStdType(var1) && isBool(var2)) // Comparing non-bool standard type with boolean
comparisonOfBoolWithIntError(tok, var2->name(), false);
}
}
}
}
void CheckOther::comparisonOfBoolWithIntError(const Token *tok, const std::string &expression)
void CheckOther::comparisonOfBoolWithIntError(const Token *tok, const std::string &expression, bool n0o1)
{
reportError(tok, Severity::warning, "comparisonOfBoolWithInt",
"Comparison of a boolean with integer that is neither 1 nor 0\n"
"The expression \"" + expression + "\" is of type 'bool' "
"and it is compared against a integer value that is "
"neither 1 nor 0.");
if (n0o1)
reportError(tok, Severity::warning, "comparisonOfBoolWithInt",
"Comparison of a boolean with an integer that is neither 1 nor 0\n"
"The expression \"" + expression + "\" is of type 'bool' "
"and it is compared against a integer value that is "
"neither 1 nor 0.");
else
reportError(tok, Severity::warning, "comparisonOfBoolWithInt",
"Comparison of a boolean with an integer\n"
"The expression \"" + expression + "\" is of type 'bool' "
"and it is compared against a integer value.");
}
//---------------------------------------------------------------------------
@ -3220,29 +3212,54 @@ void CheckOther::checkComparisonOfBoolExpressionWithInt()
if (!_settings->isEnabled("style"))
return;
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "&&|%oror% %any% ) ==|!=|>|< %num%")) {
const std::string& op = tok->strAt(3);
const std::string& num = tok->strAt(4);
if ((op == "<" || num != "0") && (op == ">" || num != "1")) {
comparisonOfBoolExpressionWithIntError(tok->next());
}
const Token* numTok = 0;
const Token* opTok = 0;
char op = 0;
if (Token::Match(tok, "&&|%oror% %any% ) >|>=|==|!=|<=|< %any%")) {
numTok = tok->tokAt(4);
opTok = tok->tokAt(3);
if (Token::Match(opTok, "<|>"))
op = opTok->str()[0];
} else if (Token::Match(tok, "%any% >|>=|==|!=|<=|< ( %any% &&|%oror%")) {
numTok = tok;
opTok = tok->next();
if (Token::Match(opTok, "<|>"))
op = opTok->str()[0]=='>'?'<':'>';
}
else if (Token::Match(tok, "%num% ==|!=|>|< ( %any% &&|%oror%")) {
const std::string& op = tok->strAt(1);
const std::string& num = tok->str();
if ((op == ">" || num != "0") && (op == "<" || num != "1")) {
comparisonOfBoolExpressionWithIntError(tok->next());
}
else if (Token::Match(tok, "! %var% >|>=|==|!=|<=|< %any%")) {
numTok = tok->tokAt(3);
opTok = tok->tokAt(2);
if (Token::Match(opTok, "<|>"))
op = opTok->str()[0];
} else if (Token::Match(tok, "%any% >|>=|==|!=|<=|< ! %var%")) {
numTok = tok;
opTok = tok->next();
if (Token::Match(opTok, "<|>"))
op = opTok->str()[0]=='>'?'<':'>';
}
if (numTok && opTok) {
if (numTok->isNumber()) {
if (((numTok->str() != "0" && numTok->str() != "1") || !Token::Match(opTok, "!=|==")) && !((op == '<' && numTok->str() == "1") || (op == '>' && numTok->str() == "0")))
comparisonOfBoolExpressionWithIntError(tok, true);
} else if (isNonBoolStdType(symbolDatabase->getVariableFromVarId(numTok->varId())))
comparisonOfBoolExpressionWithIntError(tok, false);
}
}
}
void CheckOther::comparisonOfBoolExpressionWithIntError(const Token *tok)
void CheckOther::comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1)
{
reportError(tok, Severity::warning, "compareBoolExpressionWithInt",
"Comparison of a boolean expression with an integer other than 0 or 1.");
if (n0o1)
reportError(tok, Severity::warning, "compareBoolExpressionWithInt",
"Comparison of a boolean expression with an integer other than 0 or 1.");
else
reportError(tok, Severity::warning, "compareBoolExpressionWithInt",
"Comparison of a boolean expression with an integer.");
}

8
lib/checkother.h
View File

@ -294,7 +294,7 @@ private:
void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string, const std::string &len);
void incorrectStringBooleanError(const Token *tok, const std::string& string);
void incrementBooleanError(const Token *tok);
void comparisonOfBoolWithIntError(const Token *tok, const std::string &expression);
void comparisonOfBoolWithIntError(const Token *tok, const std::string &expression, bool n0o1);
void duplicateIfError(const Token *tok1, const Token *tok2);
void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
@ -306,7 +306,7 @@ private:
void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive);
void unsignedPositiveError(const Token *tok, const std::string &varname, bool inconclusive);
void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op);
void comparisonOfBoolExpressionWithIntError(const Token *tok);
void comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1);
void SuspiciousSemicolonError(const Token *tok);
void doubleFreeError(const Token *tok, const std::string &varname);
void doubleCloseDirError(const Token *tok, const std::string &varname);
@ -353,7 +353,7 @@ private:
c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
c.incorrectStringBooleanError(0, "\"Hello World\"");
c.incrementBooleanError(0);
c.comparisonOfBoolWithIntError(0, "varname");
c.comparisonOfBoolWithIntError(0, "varname", true);
c.duplicateIfError(0, 0);
c.duplicateBranchError(0, 0);
c.duplicateExpressionError(0, 0, "&&");
@ -364,7 +364,7 @@ private:
c.unsignedLessThanZeroError(0, "varname", false);
c.unsignedPositiveError(0, "varname", false);
c.bitwiseOnBooleanError(0, "varname", "&&");
c.comparisonOfBoolExpressionWithIntError(0);
c.comparisonOfBoolExpressionWithIntError(0, true);
c.SuspiciousSemicolonError(0);
c.wrongPrintfScanfArgumentsError(0,"printf",3,2);
c.invalidScanfArgTypeError(0, "scanf", 1);

136
test/testother.cpp
View File

@ -130,7 +130,8 @@ private:
TEST_CASE(clarifyCondition4); // ticket #3110
TEST_CASE(bitwiseOnBoolean); // if (bool & bool)
TEST_CASE(comparisonOfBoolExpressionWithInt);
TEST_CASE(comparisonOfBoolExpressionWithInt1);
TEST_CASE(comparisonOfBoolExpressionWithInt2);
TEST_CASE(incorrectStringCompare);
@ -140,7 +141,6 @@ private:
TEST_CASE(comparisonOfBoolWithInt3);
TEST_CASE(comparisonOfBoolWithInt4);
TEST_CASE(comparisonOfBoolWithInt5);
TEST_CASE(comparisonOfBoolWithInt6);
TEST_CASE(duplicateIf);
TEST_CASE(duplicateBranch);
@ -3023,7 +3023,7 @@ private:
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
}
void comparisonOfBoolExpressionWithInt() {
void comparisonOfBoolExpressionWithInt1() {
check("void f(int x) {\n"
" if ((x && 0x0f)==6)\n"
" a++;\n"
@ -3043,7 +3043,8 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n"
"[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str()); // The second message appears because the code is simplified to "true == 6". Its neither wrong nor necessary.
check("void f(int x) {\n"
" if ((x || 0x0f)==0)\n"
@ -3161,6 +3162,64 @@ private:
ASSERT_EQUALS("", errout.str());
}
void comparisonOfBoolExpressionWithInt2() {
check("void f(int x) {\n"
" if (!x == 10) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
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 x) {\n"
" if (!x != 10) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
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 x) {\n"
" if (x != 10) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if (10 == !x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
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 x) {\n"
" if (10 != !x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
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 x, int y) {\n"
" if (y != !x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
check("void f(int x, bool y) {\n"
" if (y != !x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if (10 != x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void memsetZeroBytes() {
check("void f() {\n"
@ -3641,14 +3700,14 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str());
check("void f(bool x) {\n"
" if (10 >= x) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str());
check("void f(bool x) {\n"
" if (x != 0) {\n"
@ -3668,14 +3727,14 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer that is neither 1 nor 0\n", errout.str());
check("void f(bool x) {\n"
" if (x == 10) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer that is neither 1 nor 0\n", errout.str());
check("void f(bool x) {\n"
" if (x == 0) {\n"
@ -3691,14 +3750,14 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str());
check("void f(int x, bool y) {\n"
" if (x == y) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str());
check("void f(bool x, bool y) {\n"
" if (x == y) {\n"
@ -3721,14 +3780,14 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str());
check("void f(int y) {\n"
" if (true == y) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str());
check("void f(bool y) {\n"
" if (y == true) {\n"
@ -3736,60 +3795,23 @@ private:
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(bool y) {\n"
" if (false < 5) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer\n", errout.str());
}
void comparisonOfBoolWithInt4() {
check("void f(int x) {\n"
" if (!x == 10) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
check("void f(int x) {\n"
" if (!x != 10) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
check("void f(int x) {\n"
" if (x != 10) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if (10 == !x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
check("void f(int x) {\n"
" if (10 != !x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with integer that is neither 1 nor 0\n", errout.str());
check("void f(int x) {\n"
" if (10 != x) {\n"
" printf(\"x not equal to 10\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void comparisonOfBoolWithInt5() {
check("void f(int x) {\n"
" if (!x == 1) { }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void comparisonOfBoolWithInt6() {
void comparisonOfBoolWithInt5() {
check("void SetVisible(int index, bool visible) {\n"
" bool (SciTEBase::*ischarforsel)(char ch);\n"
" if (visible != GetVisible(index)) { }\n"