Skip to content

Commit ad4fb8a

Browse files
dmcppcheckdmcppcheck
committed
tweaks
1 parent fd78a1e commit ad4fb8a

2 files changed

Lines changed: 42 additions & 29 deletions

File tree

‎lib/checkcondition.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,7 +1532,7 @@ void CheckConditionImpl::alwaysTrueFalse()
15321532
tok = tok->link();
15331533
continue;
15341534
}
1535-
if (!tok->hasKnownIntValue())
1535+
if (!tok->hasKnownIntValue() || !isConstExpression(tok, mSettings.library))
15361536
continue;
15371537
const Token* condition = nullptr;
15381538
{
@@ -1549,12 +1549,10 @@ void CheckConditionImpl::alwaysTrueFalse()
15491549
condition = parent;
15501550
else if (Token::Match(parent->previous(), "if|while ("))
15511551
condition = parent->previous();
1552-
//else if (Token::simpleMatch(parent, "return"))
1553-
// condition = parent;
15541552
else if (parent->str() == ";" && parent->astParent() && parent->astParent()->astParent() &&
15551553
Token::simpleMatch(parent->astParent()->astParent()->previous(), "for ("))
15561554
condition = parent->astParent()->astParent()->previous();
1557-
else if (Token::Match(tok, "%comp%") && Token::Match(tok->astParent(), "%oror%|&&|(|;"))
1555+
else if ((Token::Match(tok, "%comp%|!") || isConstFunctionCall(tok->previous(), mSettings.library)) && Token::Match(tok->astParent(), "%oror%|&&"))
15581556
condition = tok;
15591557
else if (hasComp && Token::Match(tok, "!|%var%") && astIsBool(parent) && Token::Match(parent, "%assign%"))
15601558
condition = tok;

‎test/testcondition.cpp‎

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3369,9 +3369,14 @@ class TestCondition : public TestFixture {
33693369
" A(x++ == 1);\n"
33703370
" A(x++ == 2);\n"
33713371
"}\n");
3372-
ASSERT_EQUALS("[test.cpp:3:9]: (style) Condition 'x++==1' is always false [knownConditionTrueFalse]\n"
3373-
"[test.cpp:4:9]: (style) Condition 'x++==2' is always false [knownConditionTrueFalse]\n",
3374-
errout_str());
3372+
ASSERT_EQUALS("", errout_str());
3373+
3374+
check("void f() {\n"
3375+
" int x = 0;\n"
3376+
" if (x++ == 1) {}\n"
3377+
" if (x++ == 2) {}\n"
3378+
"}\n");
3379+
ASSERT_EQUALS("", errout_str());
33753380

33763381
check("bool foo(int bar) {\n"
33773382
" bool ret = false;\n"
@@ -4175,7 +4180,7 @@ class TestCondition : public TestFixture {
41754180
check("bool f(bool a, bool b) {\n"
41764181
" return a || ! b || ! a;\n"
41774182
"}\n");
4178-
//ASSERT_EQUALS("[test.cpp:2:12] -> [test.cpp:2:24]: (style) Condition '!a' is always true [knownConditionTrueFalse]\n", errout_str());
4183+
ASSERT_EQUALS("[test.cpp:2:12] -> [test.cpp:2:24]: (style) Condition '!a' is always true [knownConditionTrueFalse]\n", errout_str());
41794184

41804185
// #10148
41814186
check("void f(int i) {\n"
@@ -4505,9 +4510,12 @@ class TestCondition : public TestFixture {
45054510
"void foo() {\n"
45064511
" if (bar(1) == 0 && bar(1) > 0) {}\n"
45074512
"}\n");
4508-
ASSERT_EQUALS("[test.cpp:3:16]: (style) Condition 'bar(1)==0' is always false [knownConditionTrueFalse]\n"
4509-
"[test.cpp:3:31]: (style) Condition 'bar(1)>0' is always true [knownConditionTrueFalse]\n",
4510-
errout_str());
4513+
// TODO the isConstExpression returns true for the bar() function call
4514+
// these warnings are shown if the isConstExpression is removed from the checker
4515+
TODO_ASSERT_EQUALS("[test.cpp:3:16]: (style) Condition 'bar(1)==0' is always false [knownConditionTrueFalse]\n"
4516+
"[test.cpp:3:31]: (style) Condition 'bar(1)>0' is always true [knownConditionTrueFalse]\n",
4517+
"",
4518+
errout_str());
45114519

45124520
check("struct S { int bar(int i) const; };\n"
45134521
"void foo(const S& s) {\n"
@@ -4582,9 +4590,12 @@ class TestCondition : public TestFixture {
45824590
"void f() {\n"
45834591
" if (g() == 1 && g() == -1) {}\n"
45844592
"}\n");
4585-
ASSERT_EQUALS("[test.cpp:3:13]: (style) Condition 'g()==1' is always false [knownConditionTrueFalse]\n"
4586-
"[test.cpp:3:25]: (style) Condition 'g()==-1' is always true [knownConditionTrueFalse]\n",
4587-
errout_str());
4593+
// TODO the isConstExpression returns true for the bar() function call
4594+
// these warnings are shown if the isConstExpression is removed from the checker
4595+
TODO_ASSERT_EQUALS("[test.cpp:3:13]: (style) Condition 'g()==1' is always false [knownConditionTrueFalse]\n"
4596+
"[test.cpp:3:25]: (style) Condition 'g()==-1' is always true [knownConditionTrueFalse]\n",
4597+
"",
4598+
errout_str());
45884599

45894600
// #9817
45904601
check("void f(float x) {\n"
@@ -4622,8 +4633,11 @@ class TestCondition : public TestFixture {
46224633
"int g() { return f(); }\n"
46234634
"int h() { if (f()) {} }\n"
46244635
"int i() { return f() == 3; }\n");
4625-
ASSERT_EQUALS("[test.cpp:3:16]: (style) Condition 'f()' is always true [knownConditionTrueFalse]\n",
4626-
errout_str());
4636+
// TODO the isConstExpression thinks that the f() function call is non-const
4637+
// if the isConstExpression is removed from the checker then this warning is shown
4638+
TODO_ASSERT_EQUALS("[test.cpp:3:16]: (style) Condition 'f()' is always true [knownConditionTrueFalse]\n",
4639+
"",
4640+
errout_str());
46274641

46284642
check("int f() {\n"
46294643
" const char *n;\n"
@@ -4690,12 +4704,10 @@ class TestCondition : public TestFixture {
46904704
"void f() {\n"
46914705
" int i = 5;\n"
46924706
" int* p = &i;\n"
4693-
" g(i == 7);\n"
4694-
" g(p == nullptr);\n"
4707+
" g(i == 7);\n" // <- argument is always false but cannot be removed therefore warning should NOT be written
4708+
" g(p == nullptr);\n" // <- argument is always false but cannot be removed therefore warning should NOT be written
46954709
"}\n");
4696-
ASSERT_EQUALS("[test.cpp:5:9]: (style) Condition 'i==7' is always false [knownConditionTrueFalse]\n"
4697-
"[test.cpp:6:9]: (style) Condition 'p==nullptr' is always false [knownConditionTrueFalse]\n",
4698-
errout_str());
4710+
ASSERT_EQUALS("", errout_str());
46994711

47004712
check("enum E { E0, E1 };\n"
47014713
"void f() {\n"
@@ -4740,7 +4752,8 @@ class TestCondition : public TestFixture {
47404752
" int i = 0;\n"
47414753
" if ((i = g(), 1) != 0) {}\n"
47424754
"}\n");
4743-
ASSERT_EQUALS("[test.cpp:3:22]: (style) Condition '(i=g(),1)!=0' is always true [knownConditionTrueFalse]\n", errout_str());
4755+
// the condition is always true but we can't remove the condition therefore no warning is expected
4756+
ASSERT_EQUALS("", errout_str());
47444757

47454758
check("void f(unsigned i) {\n"
47464759
" const int a[2] = {};\n"
@@ -4914,9 +4927,11 @@ class TestCondition : public TestFixture {
49144927
" if (b()) {}\n"
49154928
" if (!b()) {}\n"
49164929
"}\n");
4917-
ASSERT_EQUALS("[test.cpp:3:10]: (style) Condition 'b()' is always false [knownConditionTrueFalse]\n"
4918-
"[test.cpp:4:9]: (style) Condition '!b()' is always true [knownConditionTrueFalse]\n",
4919-
errout_str());
4930+
// TODO the isConstExpression thinks that b() is a non constant expression
4931+
TODO_ASSERT_EQUALS("[test.cpp:3:10]: (style) Condition 'b()' is always false [knownConditionTrueFalse]\n"
4932+
"[test.cpp:4:9]: (style) Condition '!b()' is always true [knownConditionTrueFalse]\n",
4933+
"",
4934+
errout_str());
49204935

49214936
check("int g();\n" // a value modified inside a nested branch must be lowered to possible
49224937
"void f(int outer, int inner) {\n"
@@ -5583,7 +5598,7 @@ class TestCondition : public TestFixture {
55835598
check("bool f(const std::string& a, const std::string& b) {\n"
55845599
" return a.empty() || (b.empty() && a.empty());\n"
55855600
"}\n");
5586-
// FIXME ASSERT_EQUALS("[test.cpp:2:19] -> [test.cpp:2:46]: (style) Condition 'a.empty()' is always false [knownConditionTrueFalse]\n", errout_str());
5601+
ASSERT_EQUALS("[test.cpp:2:19] -> [test.cpp:2:46]: (style) Condition 'a.empty()' is always false [knownConditionTrueFalse]\n", errout_str());
55875602

55885603
check("struct A {\n"
55895604
" struct iterator;\n"
@@ -6469,12 +6484,12 @@ class TestCondition : public TestFixture {
64696484
"}\n");
64706485
ASSERT_EQUALS("", errout_str());
64716486

6487+
// the assignments are always false/true.. but "assignment in condition" would be a different checker
6488+
// knownConditionTrueFalse only warns about const code that can be removed.
64726489
check("void f(uint32_t u) {\n" // #2490
64736490
" if ((u = 0x00000000) || (u = 0xffffffff)) {}\n"
64746491
"}\n");
6475-
ASSERT_EQUALS("[test.cpp:2:12]: (style) Condition 'u=0x00000000' is always false [knownConditionTrueFalse]\n"
6476-
"[test.cpp:2:32]: (style) Condition 'u=0xffffffff' is always true [knownConditionTrueFalse]\n",
6477-
errout_str());
6492+
ASSERT_EQUALS("", errout_str());
64786493
}
64796494

64806495
void compareOutOfTypeRange() {

0 commit comments

Comments
 (0)