diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index eaf0ac338..6dbba4d7d 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1512,15 +1512,20 @@ void CheckCondition::alwaysTrueFalse() continue; } + const Token* astTop = tok->astTop(); const bool constIfWhileExpression = - tok->astParent() && Token::Match(tok->astTop()->astOperand1(), "if|while") && !tok->astTop()->astOperand1()->isConstexpr() && + tok->astParent() && Token::Match(astTop->astOperand1(), "if|while") && !astTop->astOperand1()->isConstexpr() && (Token::Match(tok->astParent(), "%oror%|&&") || Token::Match(tok->astParent()->astOperand1(), "if|while")); const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression - const bool compExpr = Token::Match(tok, "%comp%|!|("); // a compare expression + const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression const bool ternaryExpression = Token::simpleMatch(tok->astParent(), "?"); - const bool returnExpression = Token::simpleMatch(tok->astTop(), "return") && (tok->isComparisonOp() || Token::Match(tok, "&&|%oror%")); - - if (!(constIfWhileExpression || constValExpr || compExpr || ternaryExpression || returnExpression)) + const bool isReturn = Token::simpleMatch(astTop, "return"); + const bool returnExpression = isReturn && (tok->isComparisonOp() || Token::Match(tok, "&&|%oror%")); + const bool callExpression = Token::simpleMatch(tok, "("); + if (!(constIfWhileExpression || constValExpr || compExpr || ternaryExpression || returnExpression || callExpression)) + continue; + // only warn for functions returning bool + if (isReturn && callExpression && !(scope->function && scope->function->retDef && scope->function->retDef->str() == "bool")) continue; const Token* expr1 = tok->astOperand1(), *expr2 = tok->astOperand2(); diff --git a/test/cfg/qt.cpp b/test/cfg/qt.cpp index c793e48fc..3493dfe49 100644 --- a/test/cfg/qt.cpp +++ b/test/cfg/qt.cpp @@ -29,7 +29,7 @@ void QString1(QString s) } } -int QString2() +bool QString2() { QString s; // cppcheck-suppress knownConditionTrueFalse diff --git a/test/testcondition.cpp b/test/testcondition.cpp index f85fdebab..086aa3517 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4307,15 +4307,38 @@ private: ASSERT_EQUALS("", errout.str()); // #10426 - check("void f() {\n" + check("int f() {\n" " std::string s;\n" " for (; !s.empty();) {}\n" " for (; s.empty();) {}\n" " if (s.empty()) {}\n" + " if ((bool)0) {}\n" + " return s.empty();" "}\n"); ASSERT_EQUALS("[test.cpp:3]: (style) Condition '!s.empty()' is always false\n" "[test.cpp:4]: (style) Condition 's.empty()' is always true\n" - "[test.cpp:5]: (style) Condition 's.empty()' is always true\n", + "[test.cpp:5]: (style) Condition 's.empty()' is always true\n" + "[test.cpp:6]: (style) Condition '(bool)0' is always false\n", + errout.str()); + + check("int f(bool b) {\n" + " if (b) return static_cast(1);\n" + " return (int)0;\n" + "}\n" + "bool g(bool b) {\n" + " if (b) return static_cast(1);\n" + " return (int)0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (style) Condition 'static_cast(1)' is always true\n" + "[test.cpp:7]: (style) Condition '(int)0' is always false\n", + errout.str()); + + check("int f() { return 3; }\n" + "int g() { return f(); }\n" + "int h() { if (f()) {} }\n" + "int i() { return f() == 3; }\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'f()' is always true\n" + "[test.cpp:4]: (style) Condition 'f()==3' is always true\n", errout.str()); }