[clang-tools-extra] r274731 - [clang-tidy] Enhance redundant-expression check
Etienne Bergeron via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 6 21:03:05 PDT 2016
Author: etienneb
Date: Wed Jul 6 23:03:05 2016
New Revision: 274731
URL: http://llvm.org/viewvc/llvm-project?rev=274731&view=rev
Log:
[clang-tidy] Enhance redundant-expression check
Summary: This patch is adding support to recognize more complex redundant expressions.
Reviewers: alexfh
Subscribers: aaron.ballman, cfe-commits, chrisha
Differential Revision: http://reviews.llvm.org/D21392
Modified:
clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h
clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp?rev=274731&r1=274730&r2=274731&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp Wed Jul 6 23:03:05 2016
@@ -15,14 +15,25 @@
#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
+using namespace clang::tidy::matchers;
namespace clang {
namespace tidy {
namespace misc {
+namespace {
+using llvm::APSInt;
+} // namespace
+
static const char KnownBannedMacroNames[] =
"EAGAIN;EWOULDBLOCK;SIGCLD;SIGCHLD;";
+static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
+ Result = Value;
+ ++Result;
+ return Value < Result;
+}
+
static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
const NestedNameSpecifier *Right) {
llvm::FoldingSetNodeID LeftID, RightID;
@@ -110,6 +121,302 @@ static bool areEquivalentExpr(const Expr
}
}
+// For a given expression 'x', returns whether the ranges covered by the
+// relational operators are equivalent (i.e. x <= 4 is equivalent to x < 5).
+static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
+ const APSInt &ValueLHS,
+ BinaryOperatorKind OpcodeRHS,
+ const APSInt &ValueRHS) {
+ assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
+ "Values must be ordered");
+ // Handle the case where constants are the same: x <= 4 <==> x <= 4.
+ if (APSInt::compareValues(ValueLHS, ValueRHS) == 0)
+ return OpcodeLHS == OpcodeRHS;
+
+ // Handle the case where constants are off by one: x <= 4 <==> x < 5.
+ APSInt ValueLHS_plus1;
+ return ((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
+ (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
+ incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
+ APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0;
+}
+
+// For a given expression 'x', returns whether the ranges covered by the
+// relational operators are fully disjoint (i.e. x < 4 and x > 7).
+static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
+ const APSInt &ValueLHS,
+ BinaryOperatorKind OpcodeRHS,
+ const APSInt &ValueRHS) {
+ assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
+ "Values must be ordered");
+
+ // Handle cases where the constants are the same.
+ if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
+ switch (OpcodeLHS) {
+ case BO_EQ:
+ return OpcodeRHS == BO_NE || OpcodeRHS == BO_GT || OpcodeRHS == BO_LT;
+ case BO_NE:
+ return OpcodeRHS == BO_EQ;
+ case BO_LE:
+ return OpcodeRHS == BO_GT;
+ case BO_GE:
+ return OpcodeRHS == BO_LT;
+ case BO_LT:
+ return OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
+ case BO_GT:
+ return OpcodeRHS == BO_EQ || OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
+ default:
+ return false;
+ }
+ }
+
+ // Handle cases where the constants are different.
+ if ((OpcodeLHS == BO_EQ || OpcodeLHS == BO_LE || OpcodeLHS == BO_LE) &&
+ (OpcodeRHS == BO_EQ || OpcodeRHS == BO_GT || OpcodeRHS == BO_GE))
+ return true;
+
+ // Handle the case where constants are off by one: x > 5 && x < 6.
+ APSInt ValueLHS_plus1;
+ if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
+ incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
+ APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
+ return true;
+
+ return false;
+}
+
+// Returns whether the ranges covered by the union of both relational
+// expressions covers the whole domain (i.e. x < 10 and x > 0).
+static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
+ const APSInt &ValueLHS,
+ BinaryOperatorKind OpcodeRHS,
+ const APSInt &ValueRHS) {
+ assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
+ "Values must be ordered");
+
+ // Handle cases where the constants are the same: x < 5 || x >= 5.
+ if (APSInt::compareValues(ValueLHS, ValueRHS) == 0) {
+ switch (OpcodeLHS) {
+ case BO_EQ:
+ return OpcodeRHS == BO_NE;
+ case BO_NE:
+ return OpcodeRHS == BO_EQ;
+ case BO_LE:
+ return OpcodeRHS == BO_GT || OpcodeRHS == BO_GE;
+ case BO_LT:
+ return OpcodeRHS == BO_GE;
+ case BO_GE:
+ return OpcodeRHS == BO_LT || OpcodeRHS == BO_LE;
+ case BO_GT:
+ return OpcodeRHS == BO_LE;
+ default:
+ return false;
+ }
+ }
+
+ // Handle the case where constants are off by one: x <= 4 || x >= 5.
+ APSInt ValueLHS_plus1;
+ if (OpcodeLHS == BO_LE && OpcodeRHS == BO_GE &&
+ incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
+ APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
+ return true;
+
+ // Handle cases where the constants are different: x > 4 || x <= 7.
+ if ((OpcodeLHS == BO_GT || OpcodeLHS == BO_GE) &&
+ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE))
+ return true;
+
+ return false;
+}
+
+static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
+ const APSInt &ValueLHS,
+ BinaryOperatorKind OpcodeRHS,
+ const APSInt &ValueRHS) {
+ int Comparison = APSInt::compareValues(ValueLHS, ValueRHS);
+ switch (OpcodeLHS) {
+ case BO_EQ:
+ return OpcodeRHS == BO_EQ && Comparison == 0;
+ case BO_NE:
+ return (OpcodeRHS == BO_NE && Comparison == 0) ||
+ (OpcodeRHS == BO_EQ && Comparison != 0) ||
+ (OpcodeRHS == BO_LT && Comparison >= 0) ||
+ (OpcodeRHS == BO_LE && Comparison > 0) ||
+ (OpcodeRHS == BO_GT && Comparison <= 0) ||
+ (OpcodeRHS == BO_GE && Comparison < 0);
+
+ case BO_LT:
+ return ((OpcodeRHS == BO_LT && Comparison >= 0) ||
+ (OpcodeRHS == BO_LE && Comparison > 0) ||
+ (OpcodeRHS == BO_EQ && Comparison > 0));
+ case BO_GT:
+ return ((OpcodeRHS == BO_GT && Comparison <= 0) ||
+ (OpcodeRHS == BO_GE && Comparison < 0) ||
+ (OpcodeRHS == BO_EQ && Comparison < 0));
+ case BO_LE:
+ return (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE || OpcodeRHS == BO_EQ) &&
+ Comparison >= 0;
+ case BO_GE:
+ return (OpcodeRHS == BO_GT || OpcodeRHS == BO_GE || OpcodeRHS == BO_EQ) &&
+ Comparison <= 0;
+ default:
+ return false;
+ }
+}
+
+static void canonicalNegateExpr(BinaryOperatorKind &Opcode, APSInt &Value) {
+ if (Opcode == BO_Sub) {
+ Opcode = BO_Add;
+ Value = -Value;
+ }
+}
+
+AST_MATCHER(Expr, isIntegerConstantExpr) {
+ if (Node.isInstantiationDependent())
+ return false;
+ return Node.isIntegerConstantExpr(Finder->getASTContext());
+}
+
+// Returns a matcher for integer constant expression.
+static ast_matchers::internal::Matcher<Expr>
+matchIntegerConstantExpr(StringRef Id) {
+ std::string CstId = (Id + "-const").str();
+ return expr(isIntegerConstantExpr()).bind(CstId);
+}
+
+// Retrieve the integer value matched by 'matchIntegerConstantExpr' with name
+// 'Id' and store it into 'Value'.
+static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
+ StringRef Id, APSInt &Value) {
+ std::string CstId = (Id + "-const").str();
+ const auto *CstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
+ return CstExpr && CstExpr->isIntegerConstantExpr(Value, *Result.Context);
+}
+
+// Returns a matcher for a symbolic expression (any expression except ingeter
+// constant expression).
+static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
+ std::string SymId = (Id + "-sym").str();
+ return ignoringParenImpCasts(
+ expr(unless(isIntegerConstantExpr())).bind(SymId));
+}
+
+// Retrieve the expression matched by 'matchSymbolicExpr' with name 'Id' and
+// store it into 'SymExpr'.
+static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
+ StringRef Id, const Expr *&SymExpr) {
+ std::string SymId = (Id + "-sym").str();
+ if (const auto *Node = Result.Nodes.getNodeAs<Expr>(SymId)) {
+ SymExpr = Node;
+ return true;
+ }
+ return false;
+}
+
+// Match a binary operator between a symbolic expression and an integer constant
+// expression.
+static ast_matchers::internal::Matcher<Expr>
+matchBinOpIntegerConstantExpr(StringRef Id) {
+ const auto BinOpCstExpr =
+ expr(
+ anyOf(binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("|"),
+ hasOperatorName("&")),
+ hasEitherOperand(matchSymbolicExpr(Id)),
+ hasEitherOperand(matchIntegerConstantExpr(Id))),
+ binaryOperator(hasOperatorName("-"),
+ hasLHS(matchSymbolicExpr(Id)),
+ hasRHS(matchIntegerConstantExpr(Id)))))
+ .bind(Id);
+ return ignoringParenImpCasts(BinOpCstExpr);
+}
+
+// Retrieve sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
+// name 'Id'.
+static bool
+retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
+ StringRef Id, BinaryOperatorKind &Opcode,
+ const Expr *&Symbol, APSInt &Value) {
+ if (const auto *BinExpr = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
+ Opcode = BinExpr->getOpcode();
+ return retrieveSymbolicExpr(Result, Id, Symbol) &&
+ retrieveIntegerConstantExpr(Result, Id, Value);
+ }
+ return false;
+}
+
+// Matches relational expression: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
+static ast_matchers::internal::Matcher<Expr>
+matchRelationalIntegerConstantExpr(StringRef Id) {
+ std::string CastId = (Id + "-cast").str();
+ std::string SwapId = (Id + "-swap").str();
+ std::string NegateId = (Id + "-negate").str();
+
+ const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
+ isComparisonOperator(), expr().bind(Id),
+ anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
+ hasRHS(matchIntegerConstantExpr(Id))),
+ allOf(hasLHS(matchIntegerConstantExpr(Id)),
+ hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
+
+ // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
+ // to if (x != 0)).
+ const auto CastExpr =
+ implicitCastExpr(hasCastKind(CK_IntegralToBoolean),
+ hasSourceExpression(matchSymbolicExpr(Id)))
+ .bind(CastId);
+
+ const auto NegateRelationalExpr =
+ unaryOperator(hasOperatorName("!"),
+ hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
+ .bind(NegateId);
+
+ const auto NegateNegateRelationalExpr =
+ unaryOperator(hasOperatorName("!"),
+ hasUnaryOperand(unaryOperator(
+ hasOperatorName("!"),
+ hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))));
+
+ return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
+ NegateNegateRelationalExpr);
+}
+
+// Retrieve sub-expressions matched by 'matchRelationalIntegerConstantExpr' with
+// name 'Id'.
+static bool
+retrieveRelationalIntegerConstantExpr(const MatchFinder::MatchResult &Result,
+ StringRef Id, const Expr *&OperandExpr,
+ BinaryOperatorKind &Opcode,
+ const Expr *&Symbol, APSInt &Value) {
+ std::string CastId = (Id + "-cast").str();
+ std::string SwapId = (Id + "-swap").str();
+ std::string NegateId = (Id + "-negate").str();
+
+ if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
+ // Operand received with explicit comparator.
+ Opcode = Bin->getOpcode();
+ OperandExpr = Bin;
+ if (!retrieveIntegerConstantExpr(Result, Id, Value))
+ return false;
+ } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
+ // Operand received with implicit comparator (cast).
+ Opcode = BO_NE;
+ OperandExpr = Cast;
+ Value = APSInt(32, 0);
+ } else {
+ return false;
+ }
+
+ if (!retrieveSymbolicExpr(Result, Id, Symbol))
+ return false;
+
+ if (Result.Nodes.getNodeAs<Expr>(SwapId))
+ Opcode = BinaryOperator::reverseComparisonOp(Opcode);
+ if (Result.Nodes.getNodeAs<Expr>(NegateId))
+ Opcode = BinaryOperator::negateComparisonOp(Opcode);
+
+ return true;
+}
+
AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
return areEquivalentExpr(Node.getLHS(), Node.getRHS());
}
@@ -199,6 +506,206 @@ void RedundantExpressionCheck::registerM
unless(isMacro()), unless(isInTemplateInstantiation()))
.bind("call"),
this);
+
+ // Match common expressions and apply more checks to find redundant
+ // sub-expressions.
+ // a) Expr <op> K1 == K2
+ // b) Expr <op> K1 == Expr
+ // c) Expr <op> K1 == Expr <op> K2
+ // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
+ const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
+ const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
+ const auto CstRight = matchIntegerConstantExpr("rhs");
+ const auto SymRight = matchSymbolicExpr("rhs");
+
+ // Match expressions like: x <op> 0xFF == 0xF00.
+ Finder->addMatcher(binaryOperator(isComparisonOperator(),
+ hasEitherOperand(BinOpCstLeft),
+ hasEitherOperand(CstRight))
+ .bind("binop-const-compare-to-const"),
+ this);
+
+ // Match expressions like: x <op> 0xFF == x.
+ Finder->addMatcher(
+ binaryOperator(isComparisonOperator(),
+ anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
+ allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
+ .bind("binop-const-compare-to-sym"),
+ this);
+
+ // Match expressions like: x <op> 10 == x <op> 12.
+ Finder->addMatcher(binaryOperator(isComparisonOperator(),
+ hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
+ // Already reported as redundant.
+ unless(operandsAreEquivalent()))
+ .bind("binop-const-compare-to-binop-const"),
+ this);
+
+ // Match relational expressions combined with logical operators and find
+ // redundant sub-expressions.
+ // see: 'checkRelationalExpr'
+
+ // Match expressions like: x < 2 && x > 2.
+ const auto ComparisonLeft = matchRelationalIntegerConstantExpr("lhs");
+ const auto ComparisonRight = matchRelationalIntegerConstantExpr("rhs");
+ Finder->addMatcher(
+ binaryOperator(anyOf(hasOperatorName("||"), hasOperatorName("&&")),
+ hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
+ // Already reported as redundant.
+ unless(operandsAreEquivalent()))
+ .bind("comparisons-of-symbol-and-const"),
+ this);
+}
+
+void RedundantExpressionCheck::checkArithmeticExpr(
+ const MatchFinder::MatchResult &Result) {
+ APSInt LhsValue, RhsValue;
+ const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
+ BinaryOperatorKind LhsOpcode, RhsOpcode;
+
+ if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
+ "binop-const-compare-to-sym")) {
+ BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
+ if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
+ LhsValue) ||
+ !retrieveSymbolicExpr(Result, "rhs", RhsSymbol) ||
+ !areEquivalentExpr(LhsSymbol, RhsSymbol))
+ return;
+
+ // Check expressions: x + k == x or x - k == x.
+ if (LhsOpcode == BO_Add || LhsOpcode == BO_Sub) {
+ if ((LhsValue != 0 && Opcode == BO_EQ) ||
+ (LhsValue == 0 && Opcode == BO_NE))
+ diag(ComparisonOperator->getOperatorLoc(),
+ "logical expression is always false");
+ else if ((LhsValue == 0 && Opcode == BO_EQ) ||
+ (LhsValue != 0 && Opcode == BO_NE))
+ diag(ComparisonOperator->getOperatorLoc(),
+ "logical expression is always true");
+ }
+ } else if (const auto *ComparisonOperator =
+ Result.Nodes.getNodeAs<BinaryOperator>(
+ "binop-const-compare-to-binop-const")) {
+ BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
+
+ if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
+ LhsValue) ||
+ !retrieveBinOpIntegerConstantExpr(Result, "rhs", RhsOpcode, RhsSymbol,
+ RhsValue) ||
+ !areEquivalentExpr(LhsSymbol, RhsSymbol))
+ return;
+
+ canonicalNegateExpr(LhsOpcode, LhsValue);
+ canonicalNegateExpr(RhsOpcode, RhsValue);
+
+ // Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
+ if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
+ if ((Opcode == BO_EQ && APSInt::compareValues(LhsValue, RhsValue) == 0) ||
+ (Opcode == BO_NE && APSInt::compareValues(LhsValue, RhsValue) != 0)) {
+ diag(ComparisonOperator->getOperatorLoc(),
+ "logical expression is always true");
+ } else if ((Opcode == BO_EQ &&
+ APSInt::compareValues(LhsValue, RhsValue) != 0) ||
+ (Opcode == BO_NE &&
+ APSInt::compareValues(LhsValue, RhsValue) == 0)) {
+ diag(ComparisonOperator->getOperatorLoc(),
+ "logical expression is always false");
+ }
+ }
+ }
+}
+
+void RedundantExpressionCheck::checkBitwiseExpr(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
+ "binop-const-compare-to-const")) {
+ BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
+
+ APSInt LhsValue, RhsValue;
+ const Expr *LhsSymbol = nullptr;
+ BinaryOperatorKind LhsOpcode;
+ if (!retrieveBinOpIntegerConstantExpr(Result, "lhs", LhsOpcode, LhsSymbol,
+ LhsValue) ||
+ !retrieveIntegerConstantExpr(Result, "rhs", RhsValue))
+ return;
+
+ uint64_t LhsConstant = LhsValue.getZExtValue();
+ uint64_t RhsConstant = RhsValue.getZExtValue();
+ SourceLocation Loc = ComparisonOperator->getOperatorLoc();
+
+ // Check expression: x & k1 == k2 (i.e. x & 0xFF == 0xF00)
+ if (LhsOpcode == BO_And && (LhsConstant & RhsConstant) != RhsConstant) {
+ if (Opcode == BO_EQ)
+ diag(Loc, "logical expression is always false");
+ else if (Opcode == BO_NE)
+ diag(Loc, "logical expression is always true");
+ }
+
+ // Check expression: x | k1 == k2 (i.e. x | 0xFF == 0xF00)
+ if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
+ if (Opcode == BO_EQ)
+ diag(Loc, "logical expression is always false");
+ else if (Opcode == BO_NE)
+ diag(Loc, "logical expression is always true");
+ }
+ }
+}
+
+void RedundantExpressionCheck::checkRelationalExpr(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
+ "comparisons-of-symbol-and-const")) {
+ // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
+ BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
+
+ const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
+ APSInt LhsValue, RhsValue;
+ const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
+ BinaryOperatorKind LhsOpcode, RhsOpcode;
+ if (!retrieveRelationalIntegerConstantExpr(
+ Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue) ||
+ !retrieveRelationalIntegerConstantExpr(
+ Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue) ||
+ !areEquivalentExpr(LhsSymbol, RhsSymbol))
+ return;
+
+ // Bring to a canonical form: smallest constant must be on the left side.
+ if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
+ std::swap(LhsExpr, RhsExpr);
+ std::swap(LhsValue, RhsValue);
+ std::swap(LhsSymbol, RhsSymbol);
+ std::swap(LhsOpcode, RhsOpcode);
+ }
+
+ if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
+ areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
+ diag(ComparisonOperator->getOperatorLoc(),
+ "equivalent expression on both side of logical operator");
+ return;
+ }
+
+ if (Opcode == BO_LAnd) {
+ if (areExclusiveRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
+ diag(ComparisonOperator->getOperatorLoc(),
+ "logical expression is always false");
+ } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
+ diag(LhsExpr->getExprLoc(), "expression is redundant");
+ } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
+ diag(RhsExpr->getExprLoc(), "expression is redundant");
+ }
+ }
+
+ if (Opcode == BO_LOr) {
+ if (rangesFullyCoverDomain(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
+ diag(ComparisonOperator->getOperatorLoc(),
+ "logical expression is always true");
+ } else if (rangeSubsumesRange(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
+ diag(RhsExpr->getExprLoc(), "expression is redundant");
+ } else if (rangeSubsumesRange(RhsOpcode, RhsValue, LhsOpcode, LhsValue)) {
+ diag(LhsExpr->getExprLoc(), "expression is redundant");
+ }
+ }
+ }
}
void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
@@ -209,6 +716,10 @@ void RedundantExpressionCheck::check(con
if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call"))
diag(Call->getOperatorLoc(),
"both side of overloaded operator are equivalent");
+
+ checkArithmeticExpr(Result);
+ checkBitwiseExpr(Result);
+ checkRelationalExpr(Result);
}
} // namespace misc
Modified: clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h?rev=274731&r1=274730&r2=274731&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h Wed Jul 6 23:03:05 2016
@@ -26,6 +26,11 @@ public:
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void checkArithmeticExpr(const ast_matchers::MatchFinder::MatchResult &R);
+ void checkBitwiseExpr(const ast_matchers::MatchFinder::MatchResult &R);
+ void checkRelationalExpr(const ast_matchers::MatchFinder::MatchResult &R);
};
} // namespace misc
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp?rev=274731&r1=274730&r2=274731&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp Wed Jul 6 23:03:05 2016
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s misc-redundant-expression %t
+// RUN: %check_clang_tidy %s misc-redundant-expression %t -- -- -std=c++11
+
+typedef __INT64_TYPE__ I64;
struct Point {
int x;
@@ -64,7 +66,7 @@ int Test(int X, int Y) {
if ( + "dummy" == + "dummy") return 1;
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
- if (L"abc" == L"abc") return 1;
+ if (L"abc" == L"abc") return 1;
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
if (foo(0) - 2 < foo(0) - 2) return 1;
@@ -82,7 +84,7 @@ int Test(int X, int Y) {
int Valid(int X, int Y) {
if (X != Y) return 1;
- if (X == X + 0) return 1;
+ if (X == Y + 0) return 1;
if (P.x == P.y) return 1;
if (P.a[P.x] < P.a[P.y]) return 1;
if (P.a[0] < P.a[1]) return 1;
@@ -160,3 +162,324 @@ void TemplateCheck() {
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of overloaded operator are equivalent
}
void TestTemplate() { TemplateCheck<MyClass, MyClass>(); }
+
+int TestArithmetic(int X, int Y) {
+ if (X + 1 == X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always false
+ if (X + 1 != X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+ if (X - 1 == X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always false
+ if (X - 1 != X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
+ if (X + 1LL == X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
+ if (X + 1ULL == X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false
+
+ if (X == X + 1) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always false
+ if (X != X + 1) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always true
+ if (X == X - 1) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always false
+ if (X != X - 1) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always true
+
+ if (X != X - 1U) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always true
+ if (X != X - 1LL) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always true
+
+ if ((X+X) != (X+X) - 1) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
+ if (X + 1 == X + 2) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always false
+ if (X + 1 != X + 2) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
+ if (X - 1 == X - 2) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always false
+ if (X - 1 != X - 2) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
+ if (X + 1 == X - -1) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+ if (X + 1 != X - -1) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always false
+ if (X + 1 == X - -2) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always false
+ if (X + 1 != X - -2) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
+ if (X + 1 == X - (~0)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+ if (X + 1 == X - (~0U)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+ if (X + 1 == X - (~0ULL)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
+ // Should not match.
+ if (X + 0.5 == X) return 1;
+ if (X + 1 == Y) return 1;
+ if (X + 1 == Y + 1) return 1;
+ if (X + 1 == Y + 2) return 1;
+
+ return 0;
+}
+
+int TestBitwise(int X) {
+ if ((X & 0xFF) == 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+ if ((X & 0xFF) != 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always true
+ if ((X | 0xFF) == 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+ if ((X | 0xFF) != 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always true
+
+ if ((X | 0xFFULL) != 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: logical expression is always true
+ if ((X | 0xFF) != 0xF00ULL) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always true
+
+ if ((0xFF & X) == 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+ if ((0xFF & X) != 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always true
+ if ((0xFF & X) == 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+ if ((0xFF & X) != 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always true
+
+ if ((0xFFLL & X) == 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: logical expression is always false
+ if ((0xFF & X) == 0xF00ULL) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+
+ return 0;
+}
+
+int TestRelational(int X, int Y) {
+ if (X == 10 && X != 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
+ if (X == 10 && (X != 10)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
+ if (X == 10 && !(X == 10)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
+ if (!(X != 10) && !(X == 10)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+
+ if (X == 10ULL && X != 10ULL) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+ if (!(X != 10U) && !(X == 10)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: logical expression is always false
+ if (!(X != 10LL) && !(X == 10)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: logical expression is always false
+ if (!(X != 10ULL) && !(X == 10)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: logical expression is always false
+
+ if (X == 0 && X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false
+ if (X != 0 && !X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false
+ if (X && !X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always false
+
+ if (X && !!X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: equivalent expression on both side of logical operator
+ if (X != 0 && X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+ if (X != 0 && !!X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+ if (X == 0 && !X) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+
+ if (X == 10 && X > 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
+ if (X == 10 && X < 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
+ if (X < 10 && X > 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false
+ if (X <= 10 && X > 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
+ if (X < 10 && X >= 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false
+ if (X < 10 && X == 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false
+
+ if (X > 5 && X <= 5) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always false
+ if (X > -5 && X <= -5) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always false
+
+ if (X < 10 || X >= 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always true
+ if (X <= 10 || X > 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true
+ if (X <= 10 || X >= 11) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true
+
+ if (X < 7 && X < 6) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X < 7 && X < 7) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+ if (X < 7 && X < 8) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
+
+ if (X < 7 && X <= 5) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X < 7 && X <= 6) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: equivalent expression on both side of logical operator
+ if (X < 7 && X <= 7) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
+ if (X < 7 && X <= 8) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
+
+ if (X <= 7 && X < 6) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X <= 7 && X < 7) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X <= 7 && X < 8) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+
+ if (X <= 7 && X <= 5) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X <= 7 && X <= 6) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X <= 7 && X <= 7) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+ if (X <= 7 && X <= 8) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: expression is redundant
+
+ if (X == 11 && X > 10) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: expression is redundant
+ if (X == 11 && X < 12) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: expression is redundant
+ if (X > 10 && X == 11) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X < 12 && X == 11) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+
+ if (X != 11 && X == 42) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X != 11 && X > 11) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X != 11 && X < 11) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X != 11 && X < 8) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X != 11 && X > 14) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+
+ if (X < 7 || X < 6) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
+ if (X < 7 || X < 7) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+ if (X < 7 || X < 8) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+
+ // Should not match.
+ if (X == 10 && Y == 10) return 1;
+ if (X != 10 && X != 12) return 1;
+ if (X == 10 || X == 12) return 1;
+ if (X < 10 || X > 12) return 1;
+ if (X > 10 && X < 12) return 1;
+ if (X < 10 || X >= 12) return 1;
+ if (X > 10 && X <= 12) return 1;
+ if (X <= 10 || X > 12) return 1;
+ if (X >= 10 && X < 12) return 1;
+ if (X <= 10 || X >= 12) return 1;
+ if (X >= 10 && X <= 12) return 1;
+ if (X >= 10 && X <= 11) return 1;
+ if (X >= 10 && X < 11) return 1;
+ if (X > 10 && X <= 11) return 1;
+ if (X > 10 && X != 11) return 1;
+ if (X >= 10 && X <= 10) return 1;
+ if (X <= 10 && X >= 10) return 1;
+ if (!X && !Y) return 1;
+ if (!X && Y) return 1;
+ if (!X && Y == 0) return 1;
+ if (X == 10 && Y != 10) return 1;
+ if (X < 0 || X > 0) return 1;
+
+ return 0;
+}
+
+int TestValidExpression(int X) {
+ if (X - 1 == 1 - X) return 1;
+ if (2 * X == X) return 1;
+ if ((X << 1) == X) return 1;
+
+ return 0;
+}
+
+enum Color { Red, Yellow, Green };
+int TestRelatiopnalWithEnum(enum Color C) {
+ if (C == Red && C == Yellow) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false
+ if (C == Red && C != Red) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false
+
+ // Should not match.
+ if (C == Red || C == Yellow) return 1;
+ if (C != Red && C != Yellow) return 1;
+
+ return 0;
+}
+
+template<class T>
+int TestRelationalTemplated(int X) {
+ // This test causes a corner case with |isIntegerConstantExpr| where the type
+ // is dependant. There is an assert failing when evaluating
+ // sizeof(<incomplet-type>).
+ if (sizeof(T) == 4 || sizeof(T) == 8) return 1;
+
+ if (X + 0 == -X) return 1;
+ if (X + 0 < X) return 1;
+
+ return 0;
+}
+
+int TestWithSignedUnsigned(int X) {
+ if (X + 1 == X + 1ULL) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+ if ((X & 0xFFU) == 0xF00) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: logical expression is always false
+ if ((X & 0xFF) == 0xF00U) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+ if ((X & 0xFFU) == 0xF00U) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: logical expression is always false
+
+ return 0;
+}
+
+int TestWithLong(int X, I64 Y) {
+ if (X + 0ULL == -X) return 1;
+ if (Y + 0 == -Y) return 1;
+ if (Y <= 10 && X >= 10LL) return 1;
+ if (Y <= 10 && X >= 10ULL) return 1;
+ if (X <= 10 || X > 12LL) return 1;
+ if (X <= 10 || X > 12ULL) return 1;
+ if (Y <= 10 || Y > 12) return 1;
+
+ return 0;
+}
+
+int TestWithMinMaxInt(int X) {
+ if (X <= X + 0xFFFFFFFFU) return 1;
+ if (X <= X + 0x7FFFFFFF) return 1;
+ if (X <= X + 0x80000000) return 1;
+ if (X <= 0xFFFFFFFFU && X > 0) return 1;
+ if (X <= 0xFFFFFFFFU && X > 0U) return 1;
+
+ if (X + 0x80000000 == X - 0x80000000) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always true
+
+ if (X > 0x7FFFFFFF || X < ((-0x7FFFFFFF)-1)) return 1;
+ if (X <= 0x7FFFFFFF && X >= ((-0x7FFFFFFF)-1)) return 1;
+
+ return 0;
+}
More information about the cfe-commits
mailing list