[clang-tools-extra] r267574 - [clang-tidy] New checker for redundant expressions.
Etienne Bergeron via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 26 10:30:30 PDT 2016
Author: etienneb
Date: Tue Apr 26 12:30:30 2016
New Revision: 267574
URL: http://llvm.org/viewvc/llvm-project?rev=267574&view=rev
Log:
[clang-tidy] New checker for redundant expressions.
Summary:
This checker finds redundant expression on both side of a binary operator.
The current implementation provide a function to check whether expressions
are equivalent. This implementation is able to recognize the common
subset encounter in C++ program. Side-effects like "x++" are not considered
to be equivalent.
There are many False Positives related to macros and to floating point
computations (detecting NaN). The checker is ignoring these cases.
Example:
```
if( !dst || dst->depth != desired_depth ||
dst->nChannels != desired_num_channels ||
dst_size.width != src_size.width ||
dst_size.height != dst_size.height ) <<--- bug
{
```
Reviewers: alexfh
Subscribers: danielmarjamaki, fahlgren, jordan_rose, zaks.anna, Eugene.Zelenko, cfe-commits
Differential Revision: http://reviews.llvm.org/D19451
Added:
clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-redundant-expression.rst
clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=267574&r1=267573&r2=267574&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Tue Apr 26 12:30:30 2016
@@ -23,6 +23,7 @@ add_clang_library(clangTidyMiscModule
NoexceptMoveConstructorCheck.cpp
NonCopyableObjects.cpp
PointerAndIntegralOperationCheck.cpp
+ RedundantExpressionCheck.cpp
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
StaticAssertCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=267574&r1=267573&r2=267574&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Tue Apr 26 12:30:30 2016
@@ -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 @@ public:
"misc-non-copyable-objects");
CheckFactories.registerCheck<PointerAndIntegralOperationCheck>(
"misc-pointer-and-integral-operation");
+ CheckFactories.registerCheck<RedundantExpressionCheck>(
+ "misc-redundant-expression");
CheckFactories.registerCheck<SizeofContainerCheck>("misc-sizeof-container");
CheckFactories.registerCheck<SizeofExpressionCheck>(
"misc-sizeof-expression");
Added: 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=267574&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp Tue Apr 26 12:30:30 2016
@@ -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<Expr>(*LeftIter),
+ dyn_cast<Expr>(*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<CharacterLiteral>(Left)->getValue() ==
+ cast<CharacterLiteral>(Right)->getValue();
+ case Stmt::IntegerLiteralClass: {
+ llvm::APInt LeftLit = cast<IntegerLiteral>(Left)->getValue();
+ llvm::APInt RightLit = cast<IntegerLiteral>(Right)->getValue();
+ return LeftLit.getBitWidth() == RightLit.getBitWidth() && LeftLit == RightLit;
+ }
+ case Stmt::FloatingLiteralClass:
+ return cast<FloatingLiteral>(Left)->getValue().bitwiseIsEqual(
+ cast<FloatingLiteral>(Right)->getValue());
+ case Stmt::StringLiteralClass:
+ return cast<StringLiteral>(Left)->getBytes() ==
+ cast<StringLiteral>(Right)->getBytes();
+
+ case Stmt::DeclRefExprClass:
+ return cast<DeclRefExpr>(Left)->getDecl() ==
+ cast<DeclRefExpr>(Right)->getDecl();
+ case Stmt::MemberExprClass:
+ return cast<MemberExpr>(Left)->getMemberDecl() ==
+ cast<MemberExpr>(Right)->getMemberDecl();
+
+ case Stmt::CStyleCastExprClass:
+ return cast<CStyleCastExpr>(Left)->getTypeAsWritten() ==
+ cast<CStyleCastExpr>(Right)->getTypeAsWritten();
+
+ case Stmt::CallExprClass:
+ case Stmt::ImplicitCastExprClass:
+ case Stmt::ArraySubscriptExprClass:
+ return true;
+
+ case Stmt::UnaryOperatorClass:
+ if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
+ return false;
+ return cast<UnaryOperator>(Left)->getOpcode() ==
+ cast<UnaryOperator>(Right)->getOpcode();
+ case Stmt::BinaryOperatorClass:
+ return cast<BinaryOperator>(Left)->getOpcode() ==
+ cast<BinaryOperator>(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<BinaryOperator>("binary"))
+ diag(BinOp->getOperatorLoc(), "both side of operator are equivalent");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: 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=267574&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.h Tue Apr 26 12:30:30 2016
@@ -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
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=267574&r1=267573&r2=267574&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue Apr 26 12:30:30 2016
@@ -113,6 +113,11 @@ identified. The improvements since the
Warns about suspicious operations involving pointers and integral types.
+- New `misc-redundant-expression
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-expression.html>`_ check
+
+ Warns about redundant and equivalent expressions.
+
- New `misc-sizeof-expression
<http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-expression.html>`_ check
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=267574&r1=267573&r2=267574&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Apr 26 12:30:30 2016
@@ -67,6 +67,7 @@ Clang-Tidy Checks
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
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-redundant-expression.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-redundant-expression.rst?rev=267574&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-redundant-expression.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-redundant-expression.rst Tue Apr 26 12:30:30 2016
@@ -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
Added: 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=267574&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp Tue Apr 26 12:30:30 2016
@@ -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;
+}
More information about the cfe-commits
mailing list