[clang-tidy] Misc redundant expressions checker updated for macros
Redundant Expression Checker is updated to be able to detect expressions that
contain macros. Also, other small details are modified to improve the current
implementation.
The improvements in detail are as follows:
* Binary and ternary operator expressions containing two constants, with at
least one of them from a macro, are detected and tested for redundancy.
Macro expressions are treated somewhat differently from other expressions,
because the particular values of macros can vary across builds.
They can be considered correct and intentional, even if macro values equal,
produce ranges that exclude each other or fully overlap, etc.
* The code structure is slightly modified: typos are corrected,
comments are added and some functions are renamed to improve comprehensibility,
both in the checker and the test file. A few test cases are moved to another
function.
* The checker is now able to detect redundant CXXFunctionalCastExprs as well.
A corresponding test case is added.
Patch by: Lilla Barancsuk!
Differential Revision: https://reviews.llvm.org/D38688
llvm-svn: 317570
diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index 901b54c..265cb38 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -23,7 +23,6 @@
#include <algorithm>
#include <cassert>
#include <cstdint>
-#include <set>
#include <string>
#include <vector>
@@ -38,8 +37,8 @@
using llvm::APSInt;
} // namespace
-static const char KnownBannedMacroNames[] =
- "EAGAIN;EWOULDBLOCK;SIGCLD;SIGCHLD;";
+static const llvm::StringSet<> KnownBannedMacroNames = {"EAGAIN", "EWOULDBLOCK",
+ "SIGCLD", "SIGCHLD"};
static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
Result = Value;
@@ -99,7 +98,6 @@
case Stmt::StringLiteralClass:
return cast<StringLiteral>(Left)->getBytes() ==
cast<StringLiteral>(Right)->getBytes();
-
case Stmt::DependentScopeDeclRefExprClass:
if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
@@ -113,16 +111,14 @@
case Stmt::MemberExprClass:
return cast<MemberExpr>(Left)->getMemberDecl() ==
cast<MemberExpr>(Right)->getMemberDecl();
-
+ case Stmt::CXXFunctionalCastExprClass:
case Stmt::CStyleCastExprClass:
- return cast<CStyleCastExpr>(Left)->getTypeAsWritten() ==
- cast<CStyleCastExpr>(Right)->getTypeAsWritten();
-
+ return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
+ cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
case Stmt::CallExprClass:
case Stmt::ImplicitCastExprClass:
case Stmt::ArraySubscriptExprClass:
return true;
-
case Stmt::UnaryOperatorClass:
if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
return false;
@@ -282,7 +278,8 @@
}
}
-static void canonicalNegateExpr(BinaryOperatorKind &Opcode, APSInt &Value) {
+static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
+ APSInt &Value) {
if (Opcode == BO_Sub) {
Opcode = BO_Add;
Value = -Value;
@@ -295,32 +292,77 @@
return Node.isIntegerConstantExpr(Finder->getASTContext());
}
-// Returns a matcher for integer constant expression.
+AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
+ return areEquivalentExpr(Node.getLHS(), Node.getRHS());
+}
+
+AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
+ return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
+}
+
+AST_MATCHER(CallExpr, parametersAreEquivalent) {
+ return Node.getNumArgs() == 2 &&
+ areEquivalentExpr(Node.getArg(0), Node.getArg(1));
+}
+
+AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
+ return Node.getOperatorLoc().isMacroID();
+}
+
+AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
+ return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
+}
+
+AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
+
+AST_MATCHER_P(Expr, expandedByMacro, llvm::StringSet<>, Names) {
+ const SourceManager &SM = Finder->getASTContext().getSourceManager();
+ const LangOptions &LO = Finder->getASTContext().getLangOpts();
+ SourceLocation Loc = Node.getExprLoc();
+ while (Loc.isMacroID()) {
+ StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
+ if (Names.count(MacroName))
+ return true;
+ Loc = SM.getImmediateMacroCallerLoc(Loc);
+ }
+ return false;
+}
+
+// Returns a matcher for integer constant expressions.
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'.
+// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
+// name 'Id' and stores it into 'ConstExpr', the value of the expression is
+// stored into `Value`.
static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
- StringRef Id, APSInt &Value) {
+ StringRef Id, APSInt &Value,
+ const Expr *&ConstExpr) {
std::string CstId = (Id + "-const").str();
- const auto *CstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
- return CstExpr && CstExpr->isIntegerConstantExpr(Value, *Result.Context);
+ ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
+ return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
}
-// Returns a matcher for a symbolic expression (any expression except ingeter
-// constant expression).
+// Overloaded `retrieveIntegerConstantExpr` for compatibility.
+static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
+ StringRef Id, APSInt &Value) {
+ const Expr *ConstExpr = nullptr;
+ return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
+}
+
+// Returns a matcher for symbolic expressions (matches every expression except
+// ingeter constant expressions).
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'.
+// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
+// stores it into 'SymExpr'.
static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
StringRef Id, const Expr *&SymExpr) {
std::string SymId = (Id + "-sym").str();
@@ -348,7 +390,7 @@
return ignoringParenImpCasts(BinOpCstExpr);
}
-// Retrieve sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
+// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
// name 'Id'.
static bool
retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
@@ -362,7 +404,7 @@
return false;
}
-// Matches relational expression: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
+// Matches relational expressions: '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();
@@ -388,6 +430,7 @@
hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
.bind(NegateId);
+ // Do not bind to double negation.
const auto NegateNegateRelationalExpr =
unaryOperator(hasOperatorName("!"),
hasUnaryOperand(unaryOperator(
@@ -398,13 +441,12 @@
NegateNegateRelationalExpr);
}
-// Retrieve sub-expressions matched by 'matchRelationalIntegerConstantExpr' with
+// Retrieves 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) {
+static bool retrieveRelationalIntegerConstantExpr(
+ const MatchFinder::MatchResult &Result, StringRef Id,
+ const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
+ APSInt &Value, const Expr *&ConstExpr) {
std::string CastId = (Id + "-cast").str();
std::string SwapId = (Id + "-swap").str();
std::string NegateId = (Id + "-negate").str();
@@ -413,8 +455,10 @@
// Operand received with explicit comparator.
Opcode = Bin->getOpcode();
OperandExpr = Bin;
- if (!retrieveIntegerConstantExpr(Result, Id, Value))
+
+ if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
return false;
+
} else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
// Operand received with implicit comparator (cast).
Opcode = BO_NE;
@@ -431,56 +475,96 @@
Opcode = BinaryOperator::reverseComparisonOp(Opcode);
if (Result.Nodes.getNodeAs<Expr>(NegateId))
Opcode = BinaryOperator::negateComparisonOp(Opcode);
+ return true;
+}
+
+// Checks for expressions like (X == 4) && (Y != 9)
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+ const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
+ const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
+
+ if (!LhsBinOp || !RhsBinOp)
+ return false;
+
+ if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
+ LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
+ (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
+ RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
+ return true;
+ return false;
+}
+
+// Retrieves integer constant subexpressions from binary operator expressions
+// that have two equivalent sides
+// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
+static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
+ BinaryOperatorKind &MainOpcode,
+ BinaryOperatorKind &SideOpcode,
+ const Expr *&LhsConst,
+ const Expr *&RhsConst,
+ const ASTContext *AstCtx) {
+ assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
+ "Both sides of binary operator must be constant expressions!");
+
+ MainOpcode = BinOp->getOpcode();
+
+ const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
+ const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
+
+ LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
+ ? BinOpLhs->getLHS()
+ : BinOpLhs->getRHS();
+ RhsConst = BinOpRhs->getLHS()->isIntegerConstantExpr(*AstCtx)
+ ? BinOpRhs->getLHS()
+ : BinOpRhs->getRHS();
+
+ if (!LhsConst || !RhsConst)
+ return false;
+
+ assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
+ "Sides of the binary operator must be equivalent expressions!");
+
+ SideOpcode = BinOpLhs->getOpcode();
return true;
}
-AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
- return areEquivalentExpr(Node.getLHS(), Node.getRHS());
+static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
+ const Expr *RhsExpr,
+ const ASTContext *AstCtx) {
+ if (!LhsExpr || !RhsExpr)
+ return false;
+
+ SourceLocation LhsLoc = LhsExpr->getExprLoc();
+ SourceLocation RhsLoc = RhsExpr->getExprLoc();
+
+ if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
+ return false;
+
+ const SourceManager &SM = AstCtx->getSourceManager();
+ const LangOptions &LO = AstCtx->getLangOpts();
+
+ return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
+ Lexer::getImmediateMacroName(RhsLoc, SM, LO));
}
-AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
- return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
-}
+static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr, const Expr *&RhsExpr) {
+ if (!LhsExpr || !RhsExpr)
+ return false;
-AST_MATCHER(CallExpr, parametersAreEquivalent) {
- return Node.getNumArgs() == 2 &&
- areEquivalentExpr(Node.getArg(0), Node.getArg(1));
-}
+ SourceLocation LhsLoc = LhsExpr->getExprLoc();
+ SourceLocation RhsLoc = RhsExpr->getExprLoc();
-AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
- return Node.getOperatorLoc().isMacroID();
-}
-
-AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
- return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
-}
-
-AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
-
-AST_MATCHER_P(Expr, expandedByMacro, std::set<std::string>, Names) {
- const SourceManager &SM = Finder->getASTContext().getSourceManager();
- const LangOptions &LO = Finder->getASTContext().getLangOpts();
- SourceLocation Loc = Node.getExprLoc();
- while (Loc.isMacroID()) {
- std::string MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
- if (Names.find(MacroName) != Names.end())
- return true;
- Loc = SM.getImmediateMacroCallerLoc(Loc);
- }
- return false;
+ return LhsLoc.isMacroID() != RhsLoc.isMacroID();
}
void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
const auto AnyLiteralExpr = ignoringParenImpCasts(
anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
- std::vector<std::string> MacroNames =
- utils::options::parseStringList(KnownBannedMacroNames);
- std::set<std::string> Names(MacroNames.begin(), MacroNames.end());
+ const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(KnownBannedMacroNames));
- const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(Names));
-
+ // Binary with equivalent operands, like (X != 2 && X != 2).
Finder->addMatcher(
binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
hasOperatorName("%"), hasOperatorName("|"),
@@ -499,15 +583,16 @@
.bind("binary"),
this);
+ // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
Finder->addMatcher(
conditionalOperator(expressionsAreEquivalent(),
// Filter noisy false positives.
unless(conditionalOperatorIsInMacro()),
- unless(hasTrueExpression(AnyLiteralExpr)),
unless(isInTemplateInstantiation()))
.bind("cond"),
this);
+ // Overloaded operators with equivalent operands.
Finder->addMatcher(
cxxOperatorCallExpr(
anyOf(
@@ -613,8 +698,8 @@
!areEquivalentExpr(LhsSymbol, RhsSymbol))
return;
- canonicalNegateExpr(LhsOpcode, LhsValue);
- canonicalNegateExpr(RhsOpcode, RhsValue);
+ transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
+ transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
// Check expressions: x + 1 == x + 2 or x + 1 != x + 2.
if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
@@ -674,20 +759,23 @@
if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
"comparisons-of-symbol-and-const")) {
// Matched expressions are: (x <op> k1) <REL> (x <op> k2).
+ // E.g.: (X < 2) && (X > 4)
BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
- APSInt LhsValue, RhsValue;
const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
+ const Expr *LhsConst = nullptr, *RhsConst = nullptr;
BinaryOperatorKind LhsOpcode, RhsOpcode;
+ APSInt LhsValue, RhsValue;
+
if (!retrieveRelationalIntegerConstantExpr(
- Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue) ||
+ Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
!retrieveRelationalIntegerConstantExpr(
- Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue) ||
+ Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
!areEquivalentExpr(LhsSymbol, RhsSymbol))
return;
- // Bring to a canonical form: smallest constant must be on the left side.
+ // Bring expr to a canonical form: smallest constant must be on the left.
if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
std::swap(LhsExpr, RhsExpr);
std::swap(LhsValue, RhsValue);
@@ -695,10 +783,15 @@
std::swap(LhsOpcode, RhsOpcode);
}
+ // Constants come from two different macros, or one of them is a macro.
+ if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
+ areExprsMacroAndNonMacro(LhsConst, RhsConst))
+ return;
+
if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
diag(ComparisonOperator->getOperatorLoc(),
- "equivalent expression on both side of logical operator");
+ "equivalent expression on both sides of logical operator");
return;
}
@@ -727,16 +820,62 @@
}
void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
- if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary"))
- diag(BinOp->getOperatorLoc(), "both side of operator are equivalent");
- if (const auto *CondOp = Result.Nodes.getNodeAs<ConditionalOperator>("cond"))
- diag(CondOp->getColonLoc(), "'true' and 'false' expression are equivalent");
- if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call"))
- diag(Call->getOperatorLoc(),
- "both side of overloaded operator are equivalent");
+ if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
+ // If the expression's constants are macros, check whether they are
+ // intentional.
+ if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
+ const Expr *LhsConst = nullptr, *RhsConst = nullptr;
+ BinaryOperatorKind MainOpcode, SideOpcode;
+
+ if(!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode, LhsConst,
+ RhsConst, Result.Context))
+ return;
+
+ if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
+ areExprsMacroAndNonMacro(LhsConst, RhsConst))
+ return;
+ }
+
+ diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
+ }
+
+ if (const auto *CondOp =
+ Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
+ const Expr *TrueExpr = CondOp->getTrueExpr();
+ const Expr *FalseExpr = CondOp->getFalseExpr();
+
+ if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
+ areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
+ return;
+ diag(CondOp->getColonLoc(),
+ "'true' and 'false' expressions are equivalent");
+ }
+
+ if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
+ diag(Call->getOperatorLoc(),
+ "both sides of overloaded operator are equivalent");
+ }
+
+ // Check for the following bound expressions:
+ // - "binop-const-compare-to-sym",
+ // - "binop-const-compare-to-binop-const",
+ // Produced message:
+ // -> "logical expression is always false/true"
checkArithmeticExpr(Result);
+
+ // Check for the following bound expression:
+ // - "binop-const-compare-to-const",
+ // Produced message:
+ // -> "logical expression is always false/true"
checkBitwiseExpr(Result);
+
+ // Check for te following bound expression:
+ // - "comparisons-of-symbol-and-const",
+ // Produced messages:
+ // -> "equivalent expression on both sides of logical operator",
+ // -> "logical expression is always false/true"
+ // -> "expression is redundant"
checkRelationalExpr(Result);
}
diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h
index 59d2c8f..c0f8bf5ec 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h
@@ -16,7 +16,8 @@
namespace tidy {
namespace misc {
-/// Detect useless or suspicious redundant expressions.
+/// The checker detects expressions that are redundant, because they contain
+/// ineffective, useless parts.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-expression.html
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst
index ddef9af..83c29bd 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst
@@ -9,13 +9,13 @@
- redundant,
-- always be ``true``,
+- always ``true``,
-- always be ``false``,
+- always ``false``,
-- always be a constant (zero or one).
+- always a constant (zero or one).
-Example:
+Examples:
.. code-block:: c++
diff --git a/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp b/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
index e865ba0..5192f1f 100644
--- a/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
@@ -15,69 +15,71 @@
extern int bar(int x);
extern int bat(int x, int y);
-int Test(int X, int Y) {
+int TestSimpleEquivalent(int X, int Y) {
if (X - X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent [misc-redundant-expression]
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent [misc-redundant-expression]
if (X / X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X % X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X & X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X | X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X ^ X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X < X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X <= X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X > X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X >= X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X && X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X || X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X != (((X)))) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
if (X + 1 == X + 1) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
if (X + 1 != X + 1) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
if (X + 1 <= X + 1) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
if (X + 1 >= X + 1) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
if ((X != 1 || Y != 1) && (X != 1 || Y != 1)) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both sides of operator are equivalent
if (P.a[X - P.x] != P.a[X - P.x]) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: both sides of operator are equivalent
if ((int)X < (int)X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
+ if (int(X) < int(X)) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
if ( + "dummy" == + "dummy") return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent
if (L"abc" == L"abc") return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
if (foo(0) - 2 < foo(0) - 2) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent
if (foo(bar(0)) < (foo(bar((0))))) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both sides of operator are equivalent
if (P1.x < P2.x && P1.x < P2.x) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both sides of operator are equivalent
if (P2.a[P1.x + 2] < P2.x && P2.a[(P1.x) + (2)] < (P2.x)) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both sides of operator are equivalent
return 0;
}
@@ -102,23 +104,36 @@
return 0;
}
+#define COND_OP_MACRO 9
+#define COND_OP_OTHER_MACRO 9
int TestConditional(int x, int y) {
int k = 0;
k += (y < 0) ? x : x;
- // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'true' and 'false' expression are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'true' and 'false' expressions are equivalent
k += (y < 0) ? x + 1 : x + 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expression are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expressions are equivalent
+ k += (y < 0) ? COND_OP_MACRO : COND_OP_MACRO;
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'true' and 'false' expressions are equivalent
+
+ // Do not match for conditional operators with a macro and a const.
+ k += (y < 0) ? COND_OP_MACRO : 9;
+ // Do not match for conditional operators with expressions from different macros.
+ k += (y < 0) ? COND_OP_MACRO : COND_OP_OTHER_MACRO;
return k;
}
+#undef COND_OP_MACRO
+#undef COND_OP_OTHER_MACRO
struct MyStruct {
int x;
} Q;
+
bool operator==(const MyStruct& lhs, const MyStruct& rhs) { return lhs.x == rhs.x; }
-bool TestOperator(const MyStruct& S) {
+
+bool TestOperator(MyStruct& S) {
if (S == Q) return false;
- return S == S;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: both side of overloaded operator are equivalent
+ if (S == S) return true;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent
}
#define LT(x, y) (void)((x) < (y))
@@ -132,6 +147,7 @@
LT(X+1, X + 1);
COND(X < Y, X, X);
EQUALS(Q, Q);
+ return 0;
}
int TestFalsePositive(int* A, int X, float F) {
@@ -149,7 +165,8 @@
#define NOT_EAGAIN 3
if (EAGAIN == 0 | EAGAIN == 0) return 0;
if (NOT_EAGAIN == 0 | NOT_EAGAIN == 0) return 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both sides of operator are equivalent
+ return 0;
}
struct MyClass {
@@ -159,7 +176,7 @@
void TemplateCheck() {
static_assert(T::Value == U::Value, "should be identical");
static_assert(T::Value == T::Value, "should be identical");
- // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of overloaded operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both sides of overloaded operator are equivalent
}
void TestTemplate() { TemplateCheck<MyClass, MyClass>(); }
@@ -218,6 +235,7 @@
// 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
@@ -230,7 +248,8 @@
return 0;
}
-int TestBitwise(int X) {
+int TestBitwise(int X, int Y) {
+
if ((X & 0xFF) == 0xF00) return 1;
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
if ((X & 0xFF) != 0xF00) return 1;
@@ -262,7 +281,14 @@
return 0;
}
-int TestRelational(int X, int Y) {
+int TestLogical(int X, int Y){
+#define CONFIG 0
+ if (CONFIG && X) return 1; // OK, consts from macros are considered intentional
+#undef CONFIG
+#define CONFIG 1
+ if (CONFIG || X) return 1;
+#undef CONFIG
+
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;
@@ -289,14 +315,25 @@
// 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
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: equivalent expression on both sides of logical operator
if (X != 0 && X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
if (X != 0 && !!X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
if (X == 0 && !X) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
+ // 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 && !Y) return 1;
+ if (!X && Y) return 1;
+ if (!X && Y == 0) return 1;
+ if (X == 10 && Y != 10) return 1;
+}
+
+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;
@@ -323,18 +360,22 @@
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true
if (X != 7 || X != 14) return 1;
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always true
+ if (X == 7 || X != 5) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X != 7 || X == 7) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: 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
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides 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
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: equivalent expression on both sides 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;
@@ -345,14 +386,21 @@
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
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
+
+ if (X >= 7 && X > 6) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
+ 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]]:9: 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]]:9: warning: expression is redundant
if (X <= 7 && X <= 7) return 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
if (X <= 7 && X <= 8) return 1;
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: expression is redundant
@@ -379,14 +427,18 @@
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
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
if (X < 7 || X < 8) 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]]:13: warning: both sides of operator are equivalent
+ if (X > 7 || X > 8) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: 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;
@@ -401,12 +453,56 @@
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;
+}
+int TestRelationalMacros(int X){
+#define SOME_MACRO 3
+#define SOME_MACRO_SAME_VALUE 3
+#define SOME_OTHER_MACRO 9
+ // Do not match for redundant relational macro expressions that can be
+ // considered intentional, and for some particular values, non redundant.
+
+ // Test cases for expressions with the same macro on both sides.
+ if (X < SOME_MACRO && X > SOME_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always false
+ if (X < SOME_MACRO && X == SOME_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always false
+ if (X < SOME_MACRO || X >= SOME_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always true
+ if (X <= SOME_MACRO || X > SOME_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always true
+ if (X != SOME_MACRO && X > SOME_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X != SOME_MACRO && X < SOME_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+
+ // Test cases for two different macros.
+ if (X < SOME_MACRO && X > SOME_OTHER_MACRO) return 1;
+ if (X != SOME_MACRO && X >= SOME_OTHER_MACRO) return 1;
+ if (X != SOME_MACRO && X != SOME_OTHER_MACRO) return 1;
+ if (X == SOME_MACRO || X == SOME_MACRO_SAME_VALUE) return 1;
+ if (X == SOME_MACRO || X <= SOME_MACRO_SAME_VALUE) return 1;
+ if (X == SOME_MACRO || X > SOME_MACRO_SAME_VALUE) return 1;
+ if (X < SOME_MACRO && X <= SOME_OTHER_MACRO) return 1;
+ if (X == SOME_MACRO && X > SOME_OTHER_MACRO) return 1;
+ if (X == SOME_MACRO && X != SOME_OTHER_MACRO) return 1;
+ if (X == SOME_MACRO && X != SOME_MACRO_SAME_VALUE) return 1;
+ if (X == SOME_MACRO_SAME_VALUE && X == SOME_MACRO ) return 1;
+
+ // Test cases for a macro and a const.
+ if (X < SOME_MACRO && X > 9) return 1;
+ if (X != SOME_MACRO && X >= 9) return 1;
+ if (X != SOME_MACRO && X != 9) return 1;
+ if (X == SOME_MACRO || X == 3) return 1;
+ if (X == SOME_MACRO || X <= 3) return 1;
+ if (X < SOME_MACRO && X <= 9) return 1;
+ if (X == SOME_MACRO && X != 9) return 1;
+ if (X == SOME_MACRO && X == 9) return 1;
+
+#undef SOME_OTHER_MACRO
+#undef SOME_MACRO_SAME_VALUE
+#undef SOME_MACRO
return 0;
}
@@ -419,7 +515,7 @@
}
enum Color { Red, Yellow, Green };
-int TestRelatiopnalWithEnum(enum Color C) {
+int TestRelationalWithEnum(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;
@@ -437,7 +533,7 @@
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
+ // is dependent. There is an assert failing when evaluating
// sizeof(<incomplet-type>).
if (sizeof(T) == 4 || sizeof(T) == 8) return 1;
@@ -450,10 +546,13 @@
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
@@ -476,6 +575,7 @@
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;