From a4f43fc2ad446810afe62472db829b447bf56e64 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Mon, 7 Sep 2020 00:53:41 -0500 Subject: [PATCH] Fix issue 8234: false negative: (warning) Opposite inner 'if' condition leads to a dead code block. (#2781) --- lib/astutils.cpp | 11 +++++++++++ lib/checkcondition.cpp | 6 ++++-- lib/token.h | 20 ++++++++++++++++++++ test/testcondition.cpp | 6 ++++++ test/testother.cpp | 2 +- 5 files changed, 42 insertions(+), 3 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 06728799e..791c5f20d 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -956,6 +956,17 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token if (!cond1 || !cond2) return false; + if (cond1->str() == "&&" && cond2->str() == "&&") { + for(const Token* tok1:{cond1->astOperand1(), cond1->astOperand2()}) { + for(const Token* tok2:{cond2->astOperand1(), cond2->astOperand2()}) { + if (isSameExpression(cpp, true, tok1, tok2, library, pure, followVar, errors)) { + if (isOppositeCond(isNot, cpp, tok1->astSibling(), tok2->astSibling(), library, pure, followVar, errors)) + return true; + } + } + } + } + if (cond1->str() == "!") { if (cond2->str() == "!=") { if (cond2->astOperand1() && cond2->astOperand1()->str() == "0") diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 29312c071..5badfec1e 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -701,8 +701,10 @@ void CheckCondition::multiCondition2() if (!firstCondition) return ChildrenToVisit::none; if (firstCondition->str() == "&&") { - return ChildrenToVisit::op1_and_op2; - } else if (!firstCondition->hasKnownIntValue()) { + if (!isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, true)) + return ChildrenToVisit::op1_and_op2; + } + if (!firstCondition->hasKnownIntValue()) { if (!isReturnVar && isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, true, &errorPath)) { if (!isAliased(vars)) oppositeInnerConditionError(firstCondition, cond2, errorPath); diff --git a/lib/token.h b/lib/token.h index ca44a70b1..df41a5d89 100644 --- a/lib/token.h +++ b/lib/token.h @@ -1263,6 +1263,26 @@ public: const Token * astParent() const { return mImpl->mAstParent; } + Token * astSibling() { + if (!astParent()) + return nullptr; + if (this == astParent()->astOperand1()) + return astParent()->astOperand2(); + else if (this == astParent()->astOperand2()) + return astParent()->astOperand1(); + return nullptr; + + } + const Token * astSibling() const { + if (!astParent()) + return nullptr; + if (this == astParent()->astOperand1()) + return astParent()->astOperand2(); + else if (this == astParent()->astOperand2()) + return astParent()->astOperand1(); + return nullptr; + + } Token *astTop() { Token *ret = this; while (ret->mImpl->mAstParent) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 4f7fb5d24..823947033 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -2310,6 +2310,12 @@ private: " }" "}"); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); + + check("void f(bool x, const int a, const int b) {\n" + " if(x && a < b)\n" + " if( x && a > b){}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); } void oppositeInnerConditionEmpty() { diff --git a/test/testother.cpp b/test/testother.cpp index 22b5f7c76..99f688320 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -5310,7 +5310,7 @@ private: check("void f(int* x, bool b) {\n" " if ((!x && b) || (x != 0 && b)) {}\n" "}\n"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Opposite expression on both sides of '||'.\n", errout.str()); } void oppositeExpression() {