From e0c7f7f8f2c6dc0295d55cef1ffcbe84edc652ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 6 Sep 2017 22:26:00 +0200 Subject: [PATCH] CheckCondition: Fix FP when there are method calls in condition --- lib/checkcondition.cpp | 20 ++++++++++++++++++++ test/testcondition.cpp | 18 +++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 454807756..90b2b2bd2 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -481,6 +481,7 @@ void CheckCondition::multiCondition2() if (!Token::simpleMatch(scope->classDef->linkAt(1), ") {")) continue; + bool nonConstFunctionCall = false; bool nonlocal = false; // nonlocal variable used in condition std::set vars; // variables used in condition std::stack tokens; @@ -491,6 +492,22 @@ void CheckCondition::multiCondition2() if (!cond) continue; + if (Token::Match(cond, "%name% (")) { + const Token *obj = cond->next()->astOperand1(); + while (obj && obj->str() == ".") + obj = obj->astOperand1(); + if (!obj) + nonConstFunctionCall = true; + else if (obj->variable() && obj->variable()->isConst()) + ; + else if (cond->function() && cond->function()->isConst()) + ; + else + nonConstFunctionCall = true; + if (nonConstFunctionCall) + break; + } + if (cond->varId()) { vars.insert(cond->varId()); const Variable *var = cond->variable(); @@ -510,6 +527,9 @@ void CheckCondition::multiCondition2() } } + if (nonConstFunctionCall) + continue; + // parse until second condition is reached.. enum MULTICONDITIONTYPE { INNER, AFTER } type; const Token *tok; diff --git a/test/testcondition.cpp b/test/testcondition.cpp index cb16f8d27..a4e0023a4 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -1542,7 +1542,7 @@ private: "}"); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); - check("class Fred { public: void dostuff() const; };\n" + check("class Fred { public: bool isValid() const; void dostuff() const; };\n" "void f() {\n" " Fred fred;\n" " if (fred.isValid()) {\n" @@ -1804,6 +1804,22 @@ private: " if (!i) {}\n" "}"); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Same condition '!i', second condition is always false\n", errout.str()); + + check("void C::f(Tree &coreTree) {\n" + " if(!coreTree.build())\n" + " return;\n" + " coreTree.dostuff();\n" + " if(!coreTree.build()) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void C::f(const Tree &coreTree) {\n" + " if(!coreTree.build())\n" + " return;\n" + " coreTree.dostuff();\n" + " if(!coreTree.build()) {}\n" + "}\n"); + TODO_ASSERT_EQUALS("error", "", errout.str()); } // clarify conditions with = and comparison