Skip to content

Commit b09bcdc

Browse files
danmardanmar
committed
Use ValueFlow for compareBoolExpressionWithInt
1 parent 0352a5d commit b09bcdc

3 files changed

Lines changed: 34 additions & 48 deletions

File tree

‎lib/checkbool.cpp‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -333,26 +333,29 @@ void CheckBool::checkComparisonOfBoolExpressionWithInt()
333333
if (astIsBool(numTok))
334334
continue;
335335

336-
if (numTok->isNumber()) {
337-
const MathLib::bigint num = MathLib::toLongNumber(numTok->str());
338-
if (num==0 &&
339-
(numInRhs ? Token::Match(tok, ">|==|!=")
340-
: Token::Match(tok, "<|==|!=")))
341-
continue;
342-
if (num==1 &&
343-
(numInRhs ? Token::Match(tok, "<|==|!=")
344-
: Token::Match(tok, ">|==|!=")))
345-
continue;
346-
comparisonOfBoolExpressionWithIntError(tok, true);
347-
} else if (astIsIntegral(numTok, false) && mTokenizer->isCPP())
348-
comparisonOfBoolExpressionWithIntError(tok, false);
336+
const ValueFlow::Value *minval = numTok->getValueLE(0, mSettings);
337+
if (minval && minval->intvalue == 0 &&
338+
(numInRhs ? Token::Match(tok, ">|==|!=")
339+
: Token::Match(tok, "<|==|!=")))
340+
minval = nullptr;
341+
342+
const ValueFlow::Value *maxval = numTok->getValueGE(1, mSettings);
343+
if (maxval && maxval->intvalue == 1 &&
344+
(numInRhs ? Token::Match(tok, "<|==|!=")
345+
: Token::Match(tok, ">|==|!=")))
346+
maxval = nullptr;
347+
348+
if (minval || maxval) {
349+
bool not0or1 = (minval && minval->intvalue < 0) || (maxval && maxval->intvalue > 1);
350+
comparisonOfBoolExpressionWithIntError(tok, not0or1);
351+
}
349352
}
350353
}
351354
}
352355

353-
void CheckBool::comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1)
356+
void CheckBool::comparisonOfBoolExpressionWithIntError(const Token *tok, bool not0or1)
354357
{
355-
if (n0o1)
358+
if (not0or1)
356359
reportError(tok, Severity::warning, "compareBoolExpressionWithInt",
357360
"Comparison of a boolean expression with an integer other than 0 or 1.", CWE398, false);
358361
else

‎lib/checkbool.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class CPPCHECKLIB CheckBool : public Check {
107107
void assignBoolToPointerError(const Token *tok);
108108
void assignBoolToFloatError(const Token *tok);
109109
void bitwiseOnBooleanError(const Token *tok, const std::string &expression, const std::string &op);
110-
void comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1);
110+
void comparisonOfBoolExpressionWithIntError(const Token *tok, bool not0or1);
111111
void pointerArithBoolError(const Token *tok);
112112
void returnValueBoolError(const Token *tok);
113113

‎test/testbool.cpp‎

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ class TestBool : public TestFixture {
308308
" a++;\n"
309309
"}\n"
310310
);
311-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n", errout.str());
311+
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
312312

313313
check("void f(int x) {\n"
314314
" if ((5 && x) < 1)\n"
@@ -322,7 +322,7 @@ class TestBool : public TestFixture {
322322
" a++;\n"
323323
"}\n"
324324
);
325-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n", errout.str());
325+
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
326326

327327

328328
check("void f(int x) {\n"
@@ -337,7 +337,7 @@ class TestBool : public TestFixture {
337337
" a++;\n"
338338
"}\n"
339339
);
340-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n", errout.str());
340+
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
341341

342342
check("void f(int x) {\n"
343343
" if (1 > (5 && x))\n"
@@ -351,7 +351,7 @@ class TestBool : public TestFixture {
351351
" a++;\n"
352352
"}\n"
353353
);
354-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n", errout.str());
354+
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
355355

356356
check("void f(bool x ) {\n"
357357
" if ( x > false )\n"
@@ -422,16 +422,6 @@ class TestBool : public TestFixture {
422422
check("int f() { return (!a+b<c); }");
423423
ASSERT_EQUALS("",errout.str());
424424

425-
{
426-
const char code[] = "void f(int x, bool y) { if ( x != y ) {} }";
427-
428-
check(code, false, "test.cpp");
429-
ASSERT_EQUALS("[test.cpp:1]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
430-
431-
check(code, false, "test.c");
432-
ASSERT_EQUALS("", errout.str());
433-
}
434-
435425
check("int f() { return (a+(b<5)<=c); }");
436426
ASSERT_EQUALS("",errout.str());
437427
}
@@ -513,7 +503,7 @@ class TestBool : public TestFixture {
513503
ASSERT_EQUALS("",errout.str());
514504

515505
check("void f(int a, int b, int c) { if (1 < !(a+b)) {} }");
516-
ASSERT_EQUALS("[test.cpp:1]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n",errout.str());
506+
ASSERT_EQUALS("[test.cpp:1]: (warning) Comparison of a boolean expression with an integer.\n",errout.str());
517507
}
518508

519509
void comparisonOfBoolExpressionWithInt3() {
@@ -533,7 +523,7 @@ class TestBool : public TestFixture {
533523
check("void f(int a, int b, int c) {\n"
534524
" return (a > b) < c;\n"
535525
"}");
536-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
526+
ASSERT_EQUALS("", errout.str());
537527

538528
check("void f(int a, int b, int c) {\n"
539529
" return x(a > b) < c;\n"
@@ -629,14 +619,8 @@ class TestBool : public TestFixture {
629619
" printf(\"foo\");\n"
630620
" }\n"
631621
"}\n"
632-
"bool compare(int temp){\n"
633-
" if(temp==4){\n"
634-
" return true;\n"
635-
" }\n"
636-
" else\n"
637-
" return false;\n"
638-
"}");
639-
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of a boolean expression with an integer.\n"
622+
"bool compare(int temp);");
623+
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of a boolean expression with an integer other than 0 or 1.\n"
640624
"[test.cpp:3]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str());
641625
}
642626

@@ -950,14 +934,14 @@ class TestBool : public TestFixture {
950934
" printf(\"foo\");\n"
951935
" }\n"
952936
"}");
953-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
937+
ASSERT_EQUALS("", errout.str());
954938

955939
check("void f(int x, bool y) {\n"
956940
" if (x == y) {\n"
957941
" printf(\"foo\");\n"
958942
" }\n"
959943
"}");
960-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
944+
ASSERT_EQUALS("", errout.str());
961945

962946
check("void f(bool x, bool y) {\n"
963947
" if (x == y) {\n"
@@ -980,15 +964,14 @@ class TestBool : public TestFixture {
980964
" printf(\"foo\");\n"
981965
" }\n"
982966
"}");
983-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n"
984-
"[test.cpp:2]: (warning) Comparison of a boolean value using relational operator (<, >, <= or >=).\n", errout.str());
967+
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean value using relational operator (<, >, <= or >=).\n", errout.str());
985968

986969
check("void f(int y) {\n"
987970
" if (true == y) {\n"
988971
" printf(\"foo\");\n"
989972
" }\n"
990973
"}");
991-
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
974+
ASSERT_EQUALS("", errout.str());
992975

993976
check("void f(bool y) {\n"
994977
" if (y == true) {\n"
@@ -1049,15 +1032,15 @@ class TestBool : public TestFixture {
10491032
" if (expectedResult == res)\n"
10501033
" throw 2;\n"
10511034
"}");
1052-
ASSERT_EQUALS("[test.cpp:4]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
1035+
ASSERT_EQUALS("", errout.str());
10531036

10541037
check("bool Fun();\n"
10551038
"void Test(bool expectedResult) {\n"
10561039
" auto res = Fun();\n"
10571040
" if (5 + expectedResult == res)\n"
10581041
" throw 2;\n"
10591042
"}");
1060-
ASSERT_EQUALS("[test.cpp:4]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
1043+
ASSERT_EQUALS("", errout.str());
10611044

10621045
check("int Fun();\n"
10631046
"void Test(bool expectedResult) {\n"
@@ -1073,7 +1056,7 @@ class TestBool : public TestFixture {
10731056
" if (expectedResult == res + 5)\n"
10741057
" throw 2;\n"
10751058
"}");
1076-
ASSERT_EQUALS("[test.cpp:4]: (warning) Comparison of a boolean expression with an integer.\n", errout.str());
1059+
TODO_ASSERT_EQUALS("error", "", errout.str());
10771060
}
10781061

10791062
void comparisonOfBoolWithInt9() { // #9304

0 commit comments

Comments
 (0)