Skip to content

Commit 9895ea5

Browse files
pfultz2danmar
pfultz2
authored and
danmar
committed
Fix issue 470: Condition is always true or false on logical operators (danmar#1294)
* Fix issue 470: Condition is always true or false on logical operators * Dont warn on literals * Compute logical operators using valueflow * Fix FP when using literals * Always warn on subconditions that are always true * Use percent matches first * Add test for logical operators * Check if parent is null
1 parent 373039f commit 9895ea5

File tree

4 files changed

+95
-4
lines changed

4 files changed

+95
-4
lines changed

‎lib/checkcondition.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,6 @@ void CheckCondition::checkIncorrectLogicOperator()
885885
continue;
886886
}
887887

888-
889888
// 'A && (!A || B)' is equivalent to 'A && B'
890889
// 'A || (!A && B)' is equivalent to 'A || B'
891890
if (printStyle &&
@@ -1192,9 +1191,8 @@ void CheckCondition::alwaysTrueFalse()
11921191
continue;
11931192

11941193
const bool constIfWhileExpression =
1195-
tok->astParent()
1196-
&& Token::Match(tok->astParent()->astOperand1(), "if|while")
1197-
&& !tok->isBoolean();
1194+
tok->astParent() && Token::Match(tok->astTop()->astOperand1(), "if|while") &&
1195+
(Token::Match(tok->astParent(), "%oror%|&&") || Token::Match(tok->astParent()->astOperand1(), "if|while"));
11981196
const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression
11991197
const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
12001198

‎lib/valueflow.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,18 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
444444
return;
445445
}
446446

447+
// known result when a operand is true.
448+
if (Token::simpleMatch(parent, "&&") && value.isKnown() && value.isIntValue() && value.intvalue==0) {
449+
setTokenValue(parent, value, settings);
450+
return;
451+
}
452+
453+
// known result when a operand is false.
454+
if (Token::simpleMatch(parent, "||") && value.isKnown() && value.isIntValue() && value.intvalue!=0) {
455+
setTokenValue(parent, value, settings);
456+
return;
457+
}
458+
447459
for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) {
448460
if (!value1.isIntValue() && !value1.isFloatValue() && !value1.isTokValue())
449461
continue;

‎test/testcondition.cpp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ class TestCondition : public TestFixture {
105105

106106
TEST_CASE(checkInvalidTestForOverflow);
107107
TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile);
108+
TEST_CASE(alwaysTrueFalseInLogicalOperators);
108109
TEST_CASE(pointerAdditionResultNotNull);
109110
}
110111

@@ -2507,6 +2508,36 @@ class TestCondition : public TestFixture {
25072508
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a+1' is always true\n", errout.str());
25082509
}
25092510

2511+
void alwaysTrueFalseInLogicalOperators() {
2512+
check("bool f();\n"
2513+
"void foo() { bool x = true; if(x||f()) {}}\n");
2514+
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always true\n", errout.str());
2515+
2516+
check("void foo(bool b) { bool x = true; if(x||b) {}}\n");
2517+
ASSERT_EQUALS("[test.cpp:1]: (style) Condition 'x' is always true\n", errout.str());
2518+
2519+
check("void foo(bool b) { if(true||b) {}}\n");
2520+
ASSERT_EQUALS("", errout.str());
2521+
2522+
check("bool f();\n"
2523+
"void foo() { bool x = false; if(x||f()) {}}\n");
2524+
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always false\n", errout.str());
2525+
2526+
check("bool f();\n"
2527+
"void foo() { bool x = false; if(x&&f()) {}}\n");
2528+
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always false\n", errout.str());
2529+
2530+
check("void foo(bool b) { bool x = false; if(x&&b) {}}\n");
2531+
ASSERT_EQUALS("[test.cpp:1]: (style) Condition 'x' is always false\n", errout.str());
2532+
2533+
check("void foo(bool b) { if(false&&b) {}}\n");
2534+
ASSERT_EQUALS("", errout.str());
2535+
2536+
check("bool f();\n"
2537+
"void foo() { bool x = true; if(x&&f()) {}}\n");
2538+
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always true\n", errout.str());
2539+
}
2540+
25102541
void pointerAdditionResultNotNull() {
25112542
check("void f(char *ptr) {\n"
25122543
" if (ptr + 1 != 0);\n"

‎test/testvalueflow.cpp‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,56 @@ class TestValueFlow : public TestFixture {
559559
ASSERT_EQUALS(1U, values.size());
560560
ASSERT_EQUALS(-10, values.back().intvalue);
561561

562+
// Logical and
563+
code = "void f(bool b) {\n"
564+
" bool x = false && b;\n"
565+
" bool a = x;\n"
566+
"}";
567+
ASSERT_EQUALS(true, testValueOfX(code, 3U, 0));
568+
569+
code = "void f(bool b) {\n"
570+
" bool x = b && false;\n"
571+
" bool a = x;\n"
572+
"}";
573+
ASSERT_EQUALS(true, testValueOfX(code, 3U, 0));
574+
575+
code = "void f(bool b) {\n"
576+
" bool x = true && b;\n"
577+
" bool a = x;\n"
578+
"}";
579+
ASSERT_EQUALS(false, testValueOfX(code, 3U, 1));
580+
581+
code = "void f(bool b) {\n"
582+
" bool x = b && true;\n"
583+
" bool a = x;\n"
584+
"}";
585+
ASSERT_EQUALS(false, testValueOfX(code, 3U, 1));
586+
587+
// Logical or
588+
code = "void f(bool b) {\n"
589+
" bool x = true || b;\n"
590+
" bool a = x;\n"
591+
"}";
592+
ASSERT_EQUALS(true, testValueOfX(code, 3U, 1));
593+
594+
code = "void f(bool b) {\n"
595+
" bool x = b || true;\n"
596+
" bool a = x;\n"
597+
"}";
598+
ASSERT_EQUALS(true, testValueOfX(code, 3U, 1));
599+
600+
code = "void f(bool b) {\n"
601+
" bool x = false || b;\n"
602+
" bool a = x;\n"
603+
"}";
604+
ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
605+
606+
code = "void f(bool b) {\n"
607+
" bool x = b || false;\n"
608+
" bool a = x;\n"
609+
"}";
610+
ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
611+
562612
// function call => calculation
563613
code = "void f(int x) {\n"
564614
" a = x + 8;\n"

0 commit comments

Comments
 (0)