Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -23,6 +23,7 @@ NoexceptMoveConstructorCheck.cpp NonCopyableObjects.cpp PointerAndIntegralOperationCheck.cpp + RedundantExpressionCheck.cpp SizeofContainerCheck.cpp SizeofExpressionCheck.cpp StaticAssertCheck.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -31,6 +31,7 @@ #include "NoexceptMoveConstructorCheck.h" #include "NonCopyableObjects.h" #include "PointerAndIntegralOperationCheck.h" +#include "RedundantExpressionCheck.h" #include "SizeofContainerCheck.h" #include "SizeofExpressionCheck.h" #include "StaticAssertCheck.h" @@ -98,6 +99,8 @@ "misc-non-copyable-objects"); CheckFactories.registerCheck( "misc-pointer-and-integral-operation"); + CheckFactories.registerCheck( + "misc-redundant-expression"); CheckFactories.registerCheck("misc-sizeof-container"); CheckFactories.registerCheck( "misc-sizeof-expression"); Index: clang-tidy/misc/RedundantExpressionCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/RedundantExpressionCheck.h @@ -0,0 +1,35 @@ +//===--- RedundantExpressionCheck.h - clang-tidy-----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_EXPRESSION_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_EXPRESSION_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Detect useless or suspicious redundant expressions. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-expression.html +class RedundantExpressionCheck : public ClangTidyCheck { +public: + RedundantExpressionCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_EXPRESSION_H Index: clang-tidy/misc/RedundantExpressionCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -0,0 +1,133 @@ +//===--- RedundantExpressionCheck.cpp - clang-tidy-------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "RedundantExpressionCheck.h" +#include "../utils/Matchers.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +static bool AreIdenticalExpr(const Expr *Left, const Expr *Right) { + if (!Left || !Right) + return !Left && !Right; + + Left = Left->IgnoreParens(); + Right = Right->IgnoreParens(); + + // Compare classes. + if (Left->getStmtClass() != Right->getStmtClass()) + return false; + + // Compare children. + Expr::const_child_iterator LeftIter = Left->child_begin(); + Expr::const_child_iterator RightIter = Right->child_begin(); + while (LeftIter != Left->child_end() && RightIter != Right->child_end()) { + if (!AreIdenticalExpr(dyn_cast(*LeftIter), + dyn_cast(*RightIter))) + return false; + ++LeftIter; + ++RightIter; + } + if (LeftIter != Left->child_end() || RightIter != Right->child_end()) + return false; + + // Perform extra checks. + switch (Left->getStmtClass()) { + default: + return false; + + case Stmt::CharacterLiteralClass: + return cast(Left)->getValue() == + cast(Right)->getValue(); + case Stmt::IntegerLiteralClass: { + llvm::APInt LeftLit = cast(Left)->getValue(); + llvm::APInt RightLit = cast(Right)->getValue(); + return LeftLit.getBitWidth() == RightLit.getBitWidth() && LeftLit == RightLit; + } + case Stmt::FloatingLiteralClass: + return cast(Left)->getValue().bitwiseIsEqual( + cast(Right)->getValue()); + case Stmt::StringLiteralClass: + return cast(Left)->getBytes() == + cast(Right)->getBytes(); + + case Stmt::DeclRefExprClass: + return cast(Left)->getDecl() == + cast(Right)->getDecl(); + case Stmt::MemberExprClass: + return cast(Left)->getMemberDecl() == + cast(Right)->getMemberDecl(); + + case Stmt::CStyleCastExprClass: + return cast(Left)->getTypeAsWritten() == + cast(Right)->getTypeAsWritten(); + + case Stmt::CallExprClass: + case Stmt::ImplicitCastExprClass: + case Stmt::ArraySubscriptExprClass: + return true; + + case Stmt::UnaryOperatorClass: + if (cast(Left)->isIncrementDecrementOp()) + return false; + return cast(Left)->getOpcode() == + cast(Right)->getOpcode(); + case Stmt::BinaryOperatorClass: + return cast(Left)->getOpcode() == + cast(Right)->getOpcode(); + } +} + +AST_MATCHER(BinaryOperator, OperandsAreEquivalent) { + return AreIdenticalExpr(Node.getLHS(), Node.getRHS()); +} + +AST_MATCHER(BinaryOperator, isInMacro) { + return Node.getOperatorLoc().isMacroID(); +} + +AST_MATCHER(Expr, isInstantiationDependent) { + return Node.isInstantiationDependent(); +} + +void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { + const auto AnyLiteralExpr = ignoringParenImpCasts( + anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral())); + + Finder->addMatcher( + binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"), + hasOperatorName("%"), hasOperatorName("|"), + hasOperatorName("&"), hasOperatorName("^"), + matchers::isComparisonOperator(), + hasOperatorName("&&"), hasOperatorName("||"), + hasOperatorName("=")), + OperandsAreEquivalent(), + // Filter noisy false positives. + unless(isInstantiationDependent()), + unless(isInMacro()), + unless(hasType(realFloatingPointType())), + unless(hasEitherOperand(hasType(realFloatingPointType()))), + unless(hasLHS(AnyLiteralExpr))) + .bind("binary"), + this); +} + +void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *BinOp = Result.Nodes.getNodeAs("binary")) + diag(BinOp->getOperatorLoc(), "both side of operator are equivalent"); +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -113,6 +113,11 @@ Warns about suspicious operations involving pointers and integral types. +- New `misc-redundant-expression + `_ check + + Warns about redundant and equivalent expressions. + - New `misc-sizeof-expression `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -67,6 +67,7 @@ misc-noexcept-move-constructor misc-non-copyable-objects misc-pointer-and-integral-operation + misc-redundant-expression misc-sizeof-container misc-sizeof-expression misc-static-assert Index: docs/clang-tidy/checks/misc-redundant-expression.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/misc-redundant-expression.rst @@ -0,0 +1,20 @@ +.. title:: clang-tidy - misc-redundant-expression + +misc-redundant-expression +========================= + +Detect redundant expressions which are typically errors due to copy-paste. + +Depending on the operator expressions may be + * redundant, + * always be `true`, + * always be `false`, + * always be a constant (zero or one) + +Example: +.. code:: c++ + + ((x+1) | (x+1)) // (x+1) is redundant + (p->x == p->x) // always true + (p->x < p->x) // always false + (speed - speed + 1 == 12) // speed - speed is always zero Index: test/clang-tidy/misc-redundant-expression.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-redundant-expression.cpp @@ -0,0 +1,120 @@ +// RUN: %check_clang_tidy %s misc-redundant-expression %t + +struct Point { + int x; + int y; + int a[5]; +} P; + +extern Point P1; +extern Point P2; + +extern int foo(int x); +extern int bar(int x); +extern int bat(int x, int y); + +int Test(int X, int Y) { + if (X - X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent [misc-redundant-expression] + if (X / X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + if (X % X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + + if (X & X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + if (X | X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + if (X ^ X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + + if (X < X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + if (X <= X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + if (X > X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + if (X >= X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + + if (X && X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + if (X || X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + + if (X != (((X)))) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent + + if (X + 1 == X + 1) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent + if (X + 1 != X + 1) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent + if (X + 1 <= X + 1) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent + if (X + 1 >= X + 1) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side 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 + 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 + + if ((int)X < (int)X) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent + + if ( + "dummy" == + "dummy") return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent + 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; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side 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 + + if (P1.x < P2.x && P1.x < P2.x) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side 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 + + return 0; +} + +int Valid(int X, int Y) { + if (X != Y) return 1; + if (X == X + 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; + + if (P.a[0] < P.a[0ULL]) return 1; + if (0 < 0ULL) return 1; + if ((int)0 < (int)0ULL) return 1; + + if (++X != ++X) return 1; + if (P.a[X]++ != P.a[X]++) return 1; + if (P.a[X++] != P.a[X++]) return 1; + + if ("abc" == "ABC") return 1; + if (foo(bar(0)) < (foo(bat(0, 1)))) return 1; + return 0; +} + +#define LT(x, y) (void)((x) < (y)) + +int TestMacro(int X, int Y) { + LT(0, 0); + LT(1, 0); + LT(X, X); + LT(X+1, X + 1); +} + +int TestFalsePositive(int* A, int X, float F) { + // Produced by bison. + X = A[(2) - (2)]; + X = A['a' - 'a']; + + // Testing NaN. + if (F != F && F == F) return 1; + return 0; +}