Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp @@ -569,6 +569,7 @@ std::string SwapId = (Id + "-swap").str(); std::string NegateId = (Id + "-negate").str(); std::string OverloadId = (Id + "-overload").str(); + std::string ConstId = (Id + "-const").str(); const auto RelationalExpr = ignoringParenImpCasts(binaryOperator( isComparisonOperator(), expr().bind(Id), @@ -600,7 +601,9 @@ cxxOperatorCallExpr( hasAnyOverloadedOperatorName("==", "!=", "<", "<=", ">", ">="), // Filter noisy false positives. - unless(isMacro()), unless(isInTemplateInstantiation())) + unless(isMacro()), unless(isInTemplateInstantiation()), + anyOf(hasLHS(ignoringParenImpCasts(integerLiteral().bind(ConstId))), + hasRHS(ignoringParenImpCasts(integerLiteral().bind(ConstId))))) .bind(OverloadId); return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr, @@ -674,16 +677,38 @@ if (canOverloadedOperatorArgsBeModified(OverloadedOperatorExpr, false)) return false; + bool IntegerConstantIsFirstArg = false; + if (const auto *Arg = OverloadedOperatorExpr->getArg(1)) { if (!Arg->isValueDependent() && - !Arg->isIntegerConstantExpr(*Result.Context)) - return false; - } - Symbol = OverloadedOperatorExpr->getArg(0); + !Arg->isIntegerConstantExpr(*Result.Context)) { + IntegerConstantIsFirstArg = true; + if (const auto *Arg = OverloadedOperatorExpr->getArg(0)) { + if (!Arg->isValueDependent() && + !Arg->isIntegerConstantExpr(*Result.Context)) + return false; + } else + return false; + } + } else + return false; + + Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0); OperandExpr = OverloadedOperatorExpr; Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator()); - return BinaryOperator::isComparisonOp(Opcode); + if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr)) + return false; + + if (!BinaryOperator::isComparisonOp(Opcode)) + return false; + + // The call site of this function expects the constant on the RHS, + // so change the opcode accordingly. + if (IntegerConstantIsFirstArg) + Opcode = BinaryOperator::reverseComparisonOp(Opcode); + + return true; } else { return false; } Index: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp +++ clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp @@ -369,6 +369,11 @@ bool operator<=(const S &s, int i) { return s.x <= i; } // not modifying bool operator>=(const S &s, const int &i) { return s.x >= i; } // not modifying +bool operator==(int i, const S &s) { return s == i; } // not modifying +bool operator<(const int &i, const S &s) { return s > i; } // not modifying +bool operator<=(const int &i, const S &s) { return s >= i; } // not modifying +bool operator>(const int &i, const S &s) { return s < i; } // not modifying + struct S2 { S2() { x = 1; } int x; @@ -452,6 +457,25 @@ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false if (s1 >= 1 || s1 <= 1) return true; // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true + if (s1 >= 2 && s1 <= 0) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false + + // Same test as above but with swapped LHS/RHS on one side of the logical operator. + if (1 == s1 && s1 == 1) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: equivalent expression on both sides of logical operator + if (1 == s1 || s1 != 1) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true + if (1 < s1 && s1 < 1) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false + if (1 <= s1 || s1 <= 1) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true + if (2 < s1 && 0 > s1) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false + + // Test for absence of false positives (issue #54011). + if (s1 == 1 || s1 == 2) return true; + if (s1 > 1 && s1 < 3) return true; + if (s1 >= 2 || s1 <= 0) return true; // Test for overloaded operators that may modify their params. S2 s2;