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

CheckBool: Rely on ValueType, removed a redundant check

This commit is contained in:
PKEuS 2016-02-05 15:48:51 +01:00
parent 618ea498e9
commit e71e9bd538
5 changed files with 25 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
lib/astutils.cpp
View File

@ -45,6 +45,11 @@ bool astIsFloat(const Token *tok, bool unknown)
return vt->type >= ValueType::Type::FLOAT && vt->pointer == 0U;
}
bool astIsBool(const Token *tok)
{
return tok && (tok->isBoolean() || tok->valueType() && tok->valueType()->type == ValueType::Type::BOOL);
}
std::string astCanonicalType(const Token *expr)
{
if (!expr)

2
lib/astutils.h
View File

@ -33,6 +33,8 @@ bool astIsSignedChar(const Token *tok);
bool astIsIntegral(const Token *tok, bool unknown);
/** Is expression of floating point type? */
bool astIsFloat(const Token *tok, bool unknown);
/** Is expression of boolean type? */
bool astIsBool(const Token *tok);
/**
* Get canonical type of expression. const/static/etc are not included and neither *&.

58
lib/checkbool.cpp
View File

@ -21,6 +21,7 @@
#include "checkbool.h"
#include "mathlib.h"
#include "symboldatabase.h"
#include "astutils.h"
//---------------------------------------------------------------------------
// Register this check class (by creating a static instance of it)
@ -31,11 +32,6 @@ namespace {
static const CWE CWE571(571);
static const CWE CWE587(587);
static bool astIsBool(const Token *expr)
{
return Token::Match(expr, "%comp%|%bool%|%oror%|&&|!") && !expr->link();
}
//---------------------------------------------------------------------------
//---------------------------------------------------------------------------
void CheckBool::checkIncrementBoolean()
@ -137,60 +133,20 @@ void CheckBool::checkComparisonOfBoolWithInt()
const Token* const left = tok->astOperand1();
const Token* const right = tok->astOperand2();
if (left && right && tok->isComparisonOp()) {
if ((left->varId() && right->isNumber()) || (left->isNumber() && right->varId())) { // Comparing variable with number
const Token* varTok = left;
const Token* numTok = right;
if (left->isNumber() && right->varId()) // num with var
std::swap(varTok, numTok);
if (isBool(varTok->variable()) && // Variable has to be a boolean
((tok->str() != "==" && tok->str() != "!=") ||
(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->str() == "==" || tok->str() == "!=");
}
} else if (left->isBoolean() && right->varId()) { // Comparing boolean constant with variable
if (isNonBoolStdType(right->variable())) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(right, left->str(), false);
} else if (tok->str() != "==" && tok->str() != "!=") {
if (left->isBoolean() && right->varId()) { // Comparing boolean constant with variable
if (tok->str() != "==" && tok->str() != "!=") {
comparisonOfBoolWithInvalidComparator(right, left->str());
}
} else if (left->varId() && right->isBoolean()) { // Comparing variable with boolean constant
if (isNonBoolStdType(left->variable())) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(left, right->str(), false);
} else if (tok->str() != "==" && tok->str() != "!=") {
if (tok->str() != "==" && tok->str() != "!=") {
comparisonOfBoolWithInvalidComparator(right, left->str());
}
} else if (left->isNumber() && right->isBoolean()) { // number constant with boolean constant
comparisonOfBoolWithIntError(left, right->str(), false);
} else if (left->isBoolean() && right->isNumber()) { // number constant with boolean constant
comparisonOfBoolWithIntError(left, left->str(), false);
} else if (left->varId() && right->varId()) { // Comparing two variables, one of them boolean, one of them integer
const Variable* var1 = right->variable();
const Variable* var2 = left->variable();
if (isBool(var1) && isNonBoolStdType(var2)) // Comparing boolean with non-bool standard type
comparisonOfBoolWithIntError(left, var1->name(), false);
else if (isNonBoolStdType(var1) && isBool(var2)) // Comparing non-bool standard type with boolean
comparisonOfBoolWithIntError(left, var2->name(), false);
}
}
}
}
}
void CheckBool::comparisonOfBoolWithIntError(const Token *tok, const std::string &expression, bool n0o1)
{
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 an 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 an integer value.");
}
void CheckBool::comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression)
{
reportError(tok, Severity::warning, "comparisonOfBoolWithInvalidComparator",
@ -391,10 +347,6 @@ void CheckBool::checkComparisonOfBoolExpressionWithInt()
if (!numTok || !boolExpr)
continue;
if (Token::Match(boolExpr,"%bool%"))
// The CheckBool::checkComparisonOfBoolWithInt warns about this.
continue;
if (boolExpr->isOp() && numTok->isName() && Token::Match(tok, "==|!="))
// there is weird code such as: ((a<b)==c)
// but it is probably written this way by design.
@ -410,7 +362,7 @@ void CheckBool::checkComparisonOfBoolExpressionWithInt()
: Token::Match(tok, ">|==|!=")))
continue;
comparisonOfBoolExpressionWithIntError(tok, true);
} else if (isNonBoolStdType(numTok->variable()))
} else if (isNonBoolStdType(numTok->variable()) && _tokenizer->isCPP())
comparisonOfBoolExpressionWithIntError(tok, false);
}
}

3
lib/checkbool.h
View File

@ -102,7 +102,6 @@ private:
void comparisonOfTwoFuncsReturningBoolError(const Token *tok, const std::string &expression1, const std::string &expression2);
void comparisonOfBoolWithBoolError(const Token *tok, const std::string &expression);
void incrementBooleanError(const Token *tok);
void comparisonOfBoolWithIntError(const Token *tok, const std::string &expression, bool n0o1);
void comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression);
void assignBoolToPointerError(const Token *tok);
void assignBoolToFloatError(const Token *tok);
@ -119,7 +118,6 @@ private:
c.comparisonOfTwoFuncsReturningBoolError(0, "func_name1", "func_name2");
c.comparisonOfBoolWithBoolError(0, "var_name");
c.incrementBooleanError(0);
c.comparisonOfBoolWithIntError(0, "varname", true);
c.bitwiseOnBooleanError(0, "varname", "&&");
c.comparisonOfBoolExpressionWithIntError(0, true);
c.pointerArithBoolError(0);
@ -132,7 +130,6 @@ private:
std::string classInfo() const {
return "Boolean type checks\n"
"- using increment on boolean\n"
"- comparison of a boolean with a non-zero integer\n"
"- comparison of a boolean expression with an integer other than 0 or 1\n"
"- comparison of a function returning boolean value using relational operator\n"
"- comparison of a boolean value with boolean value using relational operator\n"

24
test/testbool.cpp
View File

@ -392,7 +392,7 @@ private:
const char code[] = "void f(int x, bool y) { if ( x != y ) {} }";
check(code, false, "test.cpp");
ASSERT_EQUALS("[test.cpp:1]: (warning) Comparison of a boolean with an integer.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
check(code, false, "test.c");
ASSERT_EQUALS("", errout.str());
@ -581,7 +581,8 @@ private:
" else\n"
" return false;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of a boolean expression with an integer.\n"
"[test.cpp:3]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str());
}
void checkComparisonOfFuncReturningBool4() {
@ -809,14 +810,14 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer.\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(bool x) {\n"
" if (10 >= x) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer.\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(bool x) {\n"
" if (x != 0) {\n"
@ -836,14 +837,14 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer that is neither 1 nor 0.\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(bool x) {\n"
" if (x == 10) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer that is neither 1 nor 0.\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(bool x) {\n"
" if (x == 0) {\n"
@ -862,14 +863,14 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer.\n", errout.str());
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 (x == y) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
check("void f(bool x, bool y) {\n"
" if (x == y) {\n"
@ -892,14 +893,15 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n"
"[test.cpp:2]: (warning) Comparison of a boolean value using relational operator (<, >, <= or >=).\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 an integer.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
check("void f(bool y) {\n"
" if (y == true) {\n"
@ -913,7 +915,7 @@ private:
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with an integer.\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());
}
void comparisonOfBoolWithInt4() {