Skip to content

Commit f99ea4b

Browse files
danmardanmar
authored
Fix #15056 (knownconditiontruefalse: remove warnings for return result) (#8877)
1 parent 80f9684 commit f99ea4b

5 files changed

Lines changed: 55 additions & 46 deletions

File tree

‎.selfcheck_suppressions‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ functionStatic:externals/tinyxml2/tinyxml2.cpp
7575
funcArgNamesDifferent:externals/tinyxml2/tinyxml2.cpp
7676
funcArgNamesDifferentUnnamed:externals/tinyxml2/tinyxml2.cpp
7777
funcArgNamesDifferentUnnamed:externals/tinyxml2/tinyxml2.h
78+
knownConditionTrueFalse:externals/tinyxml2/tinyxml2.cpp
7879
nullPointerRedundantCheck:externals/tinyxml2/tinyxml2.cpp
7980
useStlAlgorithm:externals/simplecpp/simplecpp.cpp
8081
funcArgNamesDifferentUnnamed:externals/simplecpp/simplecpp.h

‎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%"))
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;

‎lib/checkio.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ void CheckIOImpl::checkFormatString(const Token * const tok,
915915
argInfo.isKnownType() && argInfo.isArrayOrPointer() &&
916916
(!Token::Match(argInfo.typeToken, "char|wchar_t") ||
917917
argInfo.typeToken->strAt(-1) == "const")) {
918-
if (!(argInfo.isArrayOrPointer() && argInfo.element && !argInfo.typeToken->isStandardType()))
918+
if (!argInfo.element || argInfo.typeToken->isStandardType())
919919
invalidScanfArgTypeError_s(tok, numFormat, specifier, &argInfo);
920920
}
921921
if (scanf_s) {

‎test/cfg/qt.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,6 @@ void QString1(QString s)
206206
bool QString2()
207207
{
208208
QString s;
209-
// cppcheck-suppress knownConditionTrueFalse
210209
return s.size();
211210
}
212211

‎test/testcondition.cpp‎

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,7 +1336,7 @@ class TestCondition : public TestFixture {
13361336
check("int f(char c) {\n"
13371337
" return (c <= 'a' && c >= 'z');\n"
13381338
"}\n"); // TODO: use s?
1339-
ASSERT_EQUALS("[test.cpp:2:13] -> [test.cpp:2:25]: (style) Return value 'c>='z'' is always false [knownConditionTrueFalse]\n", errout_str());
1339+
ASSERT_EQUALS("[test.cpp:2:13] -> [test.cpp:2:25]: (style) Condition 'c>='z'' is always false [knownConditionTrueFalse]\n", errout_str());
13401340
}
13411341

13421342
void incorrectLogicOperator7() { // opposite expressions
@@ -2181,7 +2181,7 @@ class TestCondition : public TestFixture {
21812181
" b = g();\n"
21822182
" return b;\n"
21832183
"}\n");
2184-
ASSERT_EQUALS("[test.cpp:2:9] -> [test.cpp:3:16]: (style) Return value '!b' is always false [knownConditionTrueFalse]\n", errout_str());
2184+
ASSERT_EQUALS("", errout_str());
21852185
}
21862186

21872187
void oppositeInnerConditionPointers() {
@@ -3362,16 +3362,21 @@ class TestCondition : public TestFixture {
33623362
" if(x == 0) { x++; return x == 0; }\n"
33633363
" return false;\n"
33643364
"}\n");
3365-
ASSERT_EQUALS("[test.cpp:2:8] -> [test.cpp:2:30]: (style) Return value 'x==0' is always false [knownConditionTrueFalse]\n", errout_str());
3365+
ASSERT_EQUALS("", errout_str());
33663366

33673367
check("void f() {\n" // #6898 (Token::expressionString)
33683368
" int x = 0;\n"
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" // <- no warning as there is a side effect
3377+
" if (x++ == 2) {}\n" // <- no warning as there is a side effect
3378+
"}\n");
3379+
ASSERT_EQUALS("", errout_str());
33753380

33763381
check("bool foo(int bar) {\n"
33773382
" bool ret = false;\n"
@@ -3585,7 +3590,7 @@ class TestCondition : public TestFixture {
35853590
" const int b = 52;\n"
35863591
" return a+b;\n"
35873592
"}\n");
3588-
ASSERT_EQUALS("[test.cpp:4:13]: (style) Return value 'a+b' is always true [knownConditionTrueFalse]\n", errout_str());
3593+
ASSERT_EQUALS("", errout_str());
35893594

35903595
check("int f() {\n"
35913596
" int a = 50;\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) Return value '!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"
@@ -4402,8 +4407,7 @@ class TestCondition : public TestFixture {
44024407
" if (w) {}\n"
44034408
" }\n"
44044409
"}\n");
4405-
ASSERT_EQUALS("[test.cpp:4:24]: (style) Condition 'v<2' is always true [knownConditionTrueFalse]\n"
4406-
"[test.cpp:5:7]: (style) Condition 'w' is always true [knownConditionTrueFalse]\n",
4410+
ASSERT_EQUALS("[test.cpp:5:7]: (style) Condition 'w' is always true [knownConditionTrueFalse]\n",
44074411
errout_str());
44084412

44094413
check("void f(double d) {\n" // #10792
@@ -4506,9 +4510,12 @@ class TestCondition : public TestFixture {
45064510
"void foo() {\n"
45074511
" if (bar(1) == 0 && bar(1) > 0) {}\n"
45084512
"}\n");
4509-
ASSERT_EQUALS("[test.cpp:3:16]: (style) Condition 'bar(1)==0' is always false [knownConditionTrueFalse]\n"
4510-
"[test.cpp:3:31]: (style) Condition 'bar(1)>0' is always true [knownConditionTrueFalse]\n",
4511-
errout_str());
4513+
// TODO handle function calls without side effects better
4514+
// these warnings are shown if 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());
45124519

45134520
check("struct S { int bar(int i) const; };\n"
45144521
"void foo(const S& s) {\n"
@@ -4583,9 +4590,12 @@ class TestCondition : public TestFixture {
45834590
"void f() {\n"
45844591
" if (g() == 1 && g() == -1) {}\n"
45854592
"}\n");
4586-
ASSERT_EQUALS("[test.cpp:3:13]: (style) Condition 'g()==1' is always false [knownConditionTrueFalse]\n"
4587-
"[test.cpp:3:25]: (style) Condition 'g()==-1' is always true [knownConditionTrueFalse]\n",
4588-
errout_str());
4593+
// TODO handle function calls without side effects better
4594+
// these warnings are shown if 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());
45894599

45904600
// #9817
45914601
check("void f(float x) {\n"
@@ -4606,8 +4616,7 @@ class TestCondition : public TestFixture {
46064616
ASSERT_EQUALS("[test.cpp:3:12]: (style) Condition '!s.empty()' is always false [knownConditionTrueFalse]\n"
46074617
"[test.cpp:4:19]: (style) Condition 's.empty()' is always true [knownConditionTrueFalse]\n"
46084618
"[test.cpp:5:16]: (style) Condition 's.empty()' is always true [knownConditionTrueFalse]\n"
4609-
"[test.cpp:6:9]: (style) Condition '(bool)0' is always false [knownConditionTrueFalse]\n"
4610-
"[test.cpp:7:19]: (style) Return value 's.empty()' is always true [knownConditionTrueFalse]\n",
4619+
"[test.cpp:6:9]: (style) Condition '(bool)0' is always false [knownConditionTrueFalse]\n",
46114620
errout_str());
46124621

46134622
check("int f(bool b) {\n"
@@ -4618,17 +4627,17 @@ class TestCondition : public TestFixture {
46184627
" if (b) return static_cast<int>(1);\n"
46194628
" return (int)0;\n"
46204629
"}\n");
4621-
ASSERT_EQUALS("[test.cpp:6:35]: (style) Return value 'static_cast<int>(1)' is always true [knownConditionTrueFalse]\n"
4622-
"[test.cpp:7:12]: (style) Return value '(int)0' is always false [knownConditionTrueFalse]\n",
4623-
errout_str());
4630+
ASSERT_EQUALS("", errout_str());
46244631

46254632
check("int f() { return 3; }\n"
46264633
"int g() { return f(); }\n"
46274634
"int h() { if (f()) {} }\n"
46284635
"int i() { return f() == 3; }\n");
4629-
ASSERT_EQUALS("[test.cpp:3:16]: (style) Condition 'f()' is always true [knownConditionTrueFalse]\n"
4630-
"[test.cpp:4:22]: (style) Return value 'f()==3' is always true [knownConditionTrueFalse]\n",
4631-
errout_str());
4636+
// TODO handle function calls without side effects better
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());
46324641

46334642
check("int f() {\n"
46344643
" const char *n;\n"
@@ -4695,12 +4704,10 @@ class TestCondition : public TestFixture {
46954704
"void f() {\n"
46964705
" int i = 5;\n"
46974706
" int* p = &i;\n"
4698-
" g(i == 7);\n"
4699-
" 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
47004709
"}\n");
4701-
ASSERT_EQUALS("[test.cpp:5:9]: (style) Condition 'i==7' is always false [knownConditionTrueFalse]\n"
4702-
"[test.cpp:6:9]: (style) Condition 'p==nullptr' is always false [knownConditionTrueFalse]\n",
4703-
errout_str());
4710+
ASSERT_EQUALS("", errout_str());
47044711

47054712
check("enum E { E0, E1 };\n"
47064713
"void f() {\n"
@@ -4745,7 +4752,8 @@ class TestCondition : public TestFixture {
47454752
" int i = 0;\n"
47464753
" if ((i = g(), 1) != 0) {}\n"
47474754
"}\n");
4748-
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());
47494757

47504758
check("void f(unsigned i) {\n"
47514759
" const int a[2] = {};\n"
@@ -4919,9 +4927,12 @@ class TestCondition : public TestFixture {
49194927
" if (b()) {}\n"
49204928
" if (!b()) {}\n"
49214929
"}\n");
4922-
ASSERT_EQUALS("[test.cpp:3:10]: (style) Condition 'b()' is always false [knownConditionTrueFalse]\n"
4923-
"[test.cpp:4:9]: (style) Condition '!b()' is always true [knownConditionTrueFalse]\n",
4924-
errout_str());
4930+
// TODO handle function calls without side effects better
4931+
// these warnings are shown if isConstExpression is removed from the checker
4932+
TODO_ASSERT_EQUALS("[test.cpp:3:10]: (style) Condition 'b()' is always false [knownConditionTrueFalse]\n"
4933+
"[test.cpp:4:9]: (style) Condition '!b()' is always true [knownConditionTrueFalse]\n",
4934+
"",
4935+
errout_str());
49254936

49264937
check("int g();\n" // a value modified inside a nested branch must be lowered to possible
49274938
"void f(int outer, int inner) {\n"
@@ -5023,7 +5034,7 @@ class TestCondition : public TestFixture {
50235034
" return (index++) >= s;\n"
50245035
" }\n"
50255036
"}\n");
5026-
ASSERT_EQUALS("[test.cpp:2:15] -> [test.cpp:6:26]: (style) Return value '(index++)>=s' is always false [knownConditionTrueFalse]\n", errout_str());
5037+
ASSERT_EQUALS("", errout_str());
50275038

50285039
check("struct a {\n"
50295040
" a *b() const;\n"
@@ -5460,7 +5471,7 @@ class TestCondition : public TestFixture {
54605471
check("bool f(const int *p, const int *q) {\n"
54615472
" return p != NULL && q != NULL && p == NULL;\n"
54625473
"}\n");
5463-
ASSERT_EQUALS("[test.cpp:2:40]: (style) Return value 'p==NULL' is always false [knownConditionTrueFalse]\n", errout_str());
5474+
ASSERT_EQUALS("[test.cpp:2:40]: (style) Condition 'p==NULL' is always false [knownConditionTrueFalse]\n", errout_str());
54645475

54655476
check("struct S {\n" // #11789
54665477
" std::vector<int> v;\n"
@@ -5588,7 +5599,7 @@ class TestCondition : public TestFixture {
55885599
check("bool f(const std::string& a, const std::string& b) {\n"
55895600
" return a.empty() || (b.empty() && a.empty());\n"
55905601
"}\n");
5591-
ASSERT_EQUALS("[test.cpp:2:19] -> [test.cpp:2:46]: (style) Return value 'a.empty()' is always false [knownConditionTrueFalse]\n", errout_str());
5602+
ASSERT_EQUALS("[test.cpp:2:19] -> [test.cpp:2:46]: (style) Condition 'a.empty()' is always false [knownConditionTrueFalse]\n", errout_str());
55925603

55935604
check("struct A {\n"
55945605
" struct iterator;\n"
@@ -6296,7 +6307,7 @@ class TestCondition : public TestFixture {
62966307
check("bool f(const std::string &s) {\n"
62976308
" return s.size()>2U && s[0]=='4' && s[0]=='2';\n"
62986309
"}\n");
6299-
ASSERT_EQUALS("[test.cpp:2:35] -> [test.cpp:2:48]: (style) Return value 's[0]=='2'' is always false [knownConditionTrueFalse]\n", errout_str());
6310+
ASSERT_EQUALS("[test.cpp:2:35] -> [test.cpp:2:48]: (style) Condition 's[0]=='2'' is always false [knownConditionTrueFalse]\n", errout_str());
63006311

63016312
check("void f(int i) { if (i == 1 || 2) {} }\n"); // #12487
63026313
ASSERT_EQUALS("[test.cpp:1:28]: (style) Condition 'i==1||2' is always true [knownConditionTrueFalse]\n", errout_str());
@@ -6474,12 +6485,12 @@ class TestCondition : public TestFixture {
64746485
"}\n");
64756486
ASSERT_EQUALS("", errout_str());
64766487

6488+
// the assignments are always false/true.. but "assignment in condition" would be a different checker
6489+
// knownConditionTrueFalse should only warn if code can be removed.
64776490
check("void f(uint32_t u) {\n" // #2490
64786491
" if ((u = 0x00000000) || (u = 0xffffffff)) {}\n"
64796492
"}\n");
6480-
ASSERT_EQUALS("[test.cpp:2:12]: (style) Condition 'u=0x00000000' is always false [knownConditionTrueFalse]\n"
6481-
"[test.cpp:2:32]: (style) Condition 'u=0xffffffff' is always true [knownConditionTrueFalse]\n",
6482-
errout_str());
6493+
ASSERT_EQUALS("", errout_str());
64836494
}
64846495

64856496
void compareOutOfTypeRange() {

0 commit comments

Comments
 (0)