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

Fixed #1877 (Be more strict about int vs. bool, part II)

This commit is contained in:
PKEuS 2011-10-05 20:30:36 +02:00 committed by Daniel Marjamäki
parent abd2525339
commit 494d3af3d1
3 changed files with 178 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
lib/checkother.cpp
View File

@ -22,7 +22,7 @@
#include "mathlib.h"
#include "symboldatabase.h"
#include <cctype> // std::isupper
#include <cctype>
#include <cmath> // fabs()
#include <stack>
//---------------------------------------------------------------------------
@ -1226,14 +1226,77 @@ void CheckOther::checkComparisonOfBoolWithInt()
if (!_settings->isEnabled("style"))
return;
std::map<unsigned int, bool> boolvars; // Contains all declarated standard type variables and indicates whether its a bool or not.
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
{
if (Token::Match(tok, "( ! %var% ==|!= %num% )"))
if (Token::Match(tok, "[{};(,] %type% %var% [;=,)]") && tok->tokAt(1)->isStandardType()) // Declaration of standard type variable
{
boolvars[tok->tokAt(2)->varId()] = (tok->tokAt(1)->str() == "bool");
}
else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %num%")) // Comparing variable with number
{
const Token *varTok = tok;
const Token *numTok = tok->tokAt(2);
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId());
if (iVar != boolvars.end() && iVar->second && // Variable has to be a boolean
((tok->tokAt(1)->str() != "==" && tok->tokAt(1)->str() != "!=") ||
((MathLib::toLongNumber(numTok->str()) != 0) && (!code_is_c() || 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;
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId());
if (iVar != boolvars.end() && iVar->second && // Variable has to be a boolean
((tok->tokAt(1)->str() != "==" && tok->tokAt(1)->str() != "!=") ||
((MathLib::toLongNumber(numTok->str()) != 0) && (!code_is_c() || 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;
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId());
if (iVar != boolvars.end() && !iVar->second) // 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);
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId());
if (iVar != boolvars.end() && !iVar->second) // Variable has to be of non-boolean standard type
{
comparisonOfBoolWithIntError(varTok, constTok->str());
}
}
else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %var%")) // Comparing two variables, one of them boolean, one of them integer
{
const Token *var1Tok = tok->tokAt(2);
const Token *var2Tok = tok;
std::map<unsigned int, bool>::const_iterator iVar1 = boolvars.find(var1Tok->varId());
std::map<unsigned int, bool>::const_iterator iVar2 = boolvars.find(var2Tok->varId());
if (iVar1 != boolvars.end() && iVar2 != boolvars.end())
{
if (iVar1->second && !iVar2->second) // Comparing boolean with non-bool standard type
comparisonOfBoolWithIntError(var2Tok, var1Tok->str());
else if (!iVar1->second && iVar2->second) // Comparing non-bool standard type with boolean
comparisonOfBoolWithIntError(var2Tok, var2Tok->str());
}
}
else if (Token::Match(tok, "( ! %var% ==|!= %num% )"))
{
const Token *numTok = tok->tokAt(4);
if (numTok && numTok->str() != "0")
{
comparisonOfBoolWithIntError(numTok, tok->strAt(2));
comparisonOfBoolWithIntError(numTok, "!"+tok->strAt(2));
}
}
else if (Token::Match(tok, "( %num% ==|!= ! %var% )"))
@ -1241,17 +1304,17 @@ void CheckOther::checkComparisonOfBoolWithInt()
const Token *numTok = tok->tokAt(1);
if (numTok && numTok->str() != "0")
{
comparisonOfBoolWithIntError(numTok, tok->strAt(4));
comparisonOfBoolWithIntError(numTok, "!"+tok->strAt(4));
}
}
}
}
void CheckOther::comparisonOfBoolWithIntError(const Token *tok, const std::string &varname)
void CheckOther::comparisonOfBoolWithIntError(const Token *tok, const std::string &expression)
{
reportError(tok, Severity::warning, "comparisonOfBoolWithInt",
"Comparison of a boolean with a non-zero integer\n"
"The expression \"!" + varname + "\" is of type 'bool' but is compared against a non-zero 'int'.");
"The expression \"" + expression + "\" is of type 'bool' but is compared against a non-zero 'int'.");
}
//---------------------------------------------------------------------------

2
lib/checkother.h
View File

@ -256,7 +256,7 @@ public:
void sizeofForNumericParameterError(const Token *tok);
void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string, const std::string &len);
void incrementBooleanError(const Token *tok);
void comparisonOfBoolWithIntError(const Token *tok, const std::string &varname);
void comparisonOfBoolWithIntError(const Token *tok, const std::string &expression);
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);

113
test/testother.cpp
View File

@ -61,10 +61,10 @@ private:
TEST_CASE(varScope6);
TEST_CASE(varScope7);
TEST_CASE(varScope8);
TEST_CASE(varScope9); // classes may have extra side-effects
TEST_CASE(varScope9); // classes may have extra side-effects
TEST_CASE(varScope10); // Undefined macro FOR
TEST_CASE(varScope11); // #2475 - struct initialization is not inner scope
TEST_CASE(varScope12); // variable usage in inner loop
TEST_CASE(varScope11); // #2475 - struct initialization is not inner scope
TEST_CASE(varScope12); // variable usage in inner loop
TEST_CASE(oldStylePointerCast);
@ -126,7 +126,10 @@ private:
TEST_CASE(incorrectStringCompare);
TEST_CASE(incrementBoolean);
TEST_CASE(comparisonOfBoolWithInt);
TEST_CASE(comparisonOfBoolWithInt1);
TEST_CASE(comparisonOfBoolWithInt2);
TEST_CASE(comparisonOfBoolWithInt3);
TEST_CASE(comparisonOfBoolWithInt4);
TEST_CASE(duplicateIf);
TEST_CASE(duplicateBranch);
@ -2956,7 +2959,107 @@ private:
ASSERT_EQUALS("", errout.str());
}
void comparisonOfBoolWithInt()
void comparisonOfBoolWithInt1()
{
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 a non-zero 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 a non-zero integer\n", errout.str());
check("void f(bool x) {\n"
" if (x != 0) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("", 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 a non-zero integer\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 a non-zero integer\n", errout.str());
check("void f(bool x) {\n"
" if (x == 0) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void comparisonOfBoolWithInt2()
{
check("void f(bool x, int y) {\n"
" if (x == y) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with a non-zero 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 a non-zero integer\n", errout.str());
check("void f(bool x, bool y) {\n"
" if (x == y) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(bool x, fooClass y) {\n"
" if (x == y) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void comparisonOfBoolWithInt3()
{
check("void f(int y) {\n"
" if (y > false) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with a non-zero 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 a non-zero integer\n", errout.str());
check("void f(bool y) {\n"
" if (y == true) {\n"
" printf(\"foo\");\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void comparisonOfBoolWithInt4()
{
check("void f(int x) {\n"
" if (!x == 10) {\n"