diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 21d791079..ddca80846 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -621,19 +621,29 @@ static bool parseComparison(const Token *comp, bool *not1, std::string *op, std: comp = comp->astOperand1(); } - if (!comp || !comp->isComparisonOp() || !comp->astOperand1() || !comp->astOperand2()) + if (!comp) return false; - if (comp->astOperand1()->isLiteral()) { + if (!comp->isComparisonOp() || !comp->astOperand1() || !comp->astOperand2()) { + *op = "!="; + *value = "0"; + *expr = comp; + } else if (comp->astOperand1()->isLiteral()) { + if (comp->astOperand1()->isExpandedMacro()) + return false; *op = invertOperatorForOperandSwap(comp->str()); *value = comp->astOperand1()->str(); *expr = comp->astOperand2(); } else if (comp->astOperand2()->isLiteral()) { + if (comp->astOperand2()->isExpandedMacro()) + return false; *op = comp->str(); *value = comp->astOperand2()->str(); *expr = comp->astOperand1(); } else { - return false; + *op = "!="; + *value = "0"; + *expr = comp; } // Only float and int values are currently handled @@ -643,6 +653,21 @@ static bool parseComparison(const Token *comp, bool *not1, std::string *op, std: return true; } +static std::string conditionString(bool not1, const Token *expr1, const std::string &op, const std::string &value1) +{ + if (expr1->astParent()->isComparisonOp()) + return std::string(not1 ? "!(" : "") + + (expr1->isName() ? expr1->str() : std::string("EXPR")) + + " " + + op + + " " + + value1 + + (not1 ? ")" : ""); + + return std::string(not1 ? "!" : "") + + (expr1->isName() ? expr1->str() : std::string("EXPR")); +} + void CheckCondition::checkIncorrectLogicOperator() { const bool printStyle = _settings->isEnabled("style"); @@ -758,8 +783,8 @@ void CheckCondition::checkIncorrectLogicOperator() secondTrue &= !(result1 && !result2); } - const std::string cond1str = std::string(not1?"!(":"") + (expr1->isName() ? expr1->str() : "EXPR") + " " + op1 + " " + value1 + std::string(not1?")":""); - const std::string cond2str = std::string(not2?"!(":"") + (expr2->isName() ? expr2->str() : "EXPR") + " " + op2 + " " + value2 + std::string(not2?")":""); + const std::string cond1str = conditionString(not1, expr1, op1, value1); + const std::string cond2str = conditionString(not2, expr2, op2, value2); if (printWarning && (alwaysTrue || alwaysFalse)) { const std::string text = cond1str + " " + tok->str() + " " + cond2str; incorrectLogicOperatorError(tok, text, alwaysTrue); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index ed6e41388..9074bba01 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -574,6 +574,16 @@ private: ); ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str()); + check("void f(int x) {\n" + " if (x<0 && !x) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 0 && !x.\n", errout.str()); + + check("void f(int x) {\n" + " if (x==0 && x) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 0 && x.\n", errout.str()); + check("void f(int x) {\n" // ast.. " if (y == 1 && x == 1 && x == 7) { }\n" "}\n");