diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 547b997d0..fc6c41e27 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -157,7 +157,16 @@ bool isSameExpression(const Token *tok1, const Token *tok2, const std::set &constFunctions) { - if (!cond1 || !cond1->isComparisonOp() || !cond2 || !cond2->isComparisonOp()) + if (!cond1 || !cond2) + return false; + + if (cond1->str() == "!") + return isSameExpression(cond1->astOperand1(), cond2, constFunctions); + + if (cond2->str() == "!") + return isSameExpression(cond1, cond2->astOperand1(), constFunctions); + + if (!cond1->isComparisonOp() || !cond2->isComparisonOp()) return false; const std::string &comp1 = cond1->str(); @@ -1260,7 +1269,16 @@ void CheckOther::checkIncorrectLogicOperator() const Scope * scope = symbolDatabase->functionScopes[ii]; for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { - if (Token::Match(tok, "&&|%oror%")) { + // Opposite comparisons + if (Token::Match(tok, "%oror%|&&") && + (tok->astOperand1()->isName() || tok->astOperand2()->isName()) && + isOppositeCond(tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) { + + const bool alwaysTrue(tok->str() == "||"); + incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue); + } + + else if (Token::Match(tok, "&&|%oror%")) { // Comparison #1 (LHS) const Token *comp1 = tok->astOperand1(); if (comp1 && comp1->str() == tok->str()) diff --git a/test/testother.cpp b/test/testother.cpp index 5d0f9b23e..dbd74ea7a 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -134,6 +134,7 @@ private: TEST_CASE(incorrectLogicOperator4); TEST_CASE(incorrectLogicOperator5); // complex expressions TEST_CASE(incorrectLogicOperator6); // char literals + TEST_CASE(incorrectLogicOperator7); // opposite expressions: (expr || !expr) TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError); TEST_CASE(incorrectLogicOp_condSwapping); @@ -4199,6 +4200,13 @@ private: TODO_ASSERT_EQUALS("error", "", errout.str()); } + void incorrectLogicOperator7() { // opposite expressions + check("void f(int i) {\n" + " if (i || !i) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: i||!i.\n", errout.str()); + } + void secondAlwaysTrueFalseWhenFirstTrueError() { check("void f(int x) {\n" " if (x > 5 && x != 1)\n"