barancsuk updated this revision to Diff 119452.
barancsuk marked 8 inline comments as done.
barancsuk added a comment.
Herald added a subscriber: whisperity.
Address review comments.
https://reviews.llvm.org/D38688
Files:
clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tidy/misc/RedundantExpressionCheck.h
docs/clang-tidy/checks/misc-redundant-expression.rst
test/clang-tidy/misc-redundant-expression.cpp
Index: test/clang-tidy/misc-redundant-expression.cpp
===================================================================
--- test/clang-tidy/misc-redundant-expression.cpp
+++ 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,16 +386,23 @@
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
+ // 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
@@ -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,124 @@
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 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_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_OTHER_MACRO) return 1;
+
+ if (X < SOME_OTHER_MACRO || X >= SOME_MACRO) return 1;
+ if (X <= SOME_OTHER_MACRO || X > SOME_MACRO) return 1;
+ if (X < SOME_OTHER_MACRO || X > SOME_MACRO) return 1;
+ if (X <= SOME_OTHER_MACRO || X >= SOME_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;
+
+ // 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 > 9) 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;
+
+ if (X < 9 || X >= SOME_MACRO) return 1;
+ if (X <= 9 || X > SOME_MACRO) return 1;
+ if (X < 9 || X > SOME_MACRO) return 1;
+ if (X <= 9 || X >= SOME_MACRO) return 1;
+
+ if (X == SOME_MACRO || X == 3) return 1;
+ if (X == SOME_MACRO || X <= 3) return 1;
+ if (X == SOME_MACRO || X > 3) return 1;
+
+ // Match for macro expressions that are redundant, regardless of the macros` actual
+ // value.
+ // Test cases for two different macros.
+ if (X < SOME_MACRO && X <= SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: expression is redundant
+ if (X >= SOME_MACRO && X > SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X == SOME_MACRO && X > SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always false
+ if (X == SOME_MACRO && X != SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: expression is redundant
+ if (X == SOME_MACRO && X != SOME_MACRO_SAME_VALUE) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always false
+ if (X == SOME_MACRO && X == SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always false
+ if (X == SOME_MACRO_SAME_VALUE && X == SOME_MACRO ) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: both sides of operator are equivalent
+
+ if (X < SOME_MACRO || X <= SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X >= SOME_MACRO || X > SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: expression is redundant
+ if (X == SOME_MACRO || X != SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X == SOME_MACRO || X != SOME_MACRO_SAME_VALUE) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always true
+ if (X != SOME_MACRO || X != SOME_OTHER_MACRO) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always true
+ if (X != SOME_MACRO || X != SOME_MACRO_SAME_VALUE) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both sides of operator are equivalent
+
+ // Test cases for a macro and a const.
+ if (X < SOME_MACRO && X <= 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: expression is redundant
+ if (X >= SOME_MACRO && X > 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X == SOME_MACRO && X > 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always false
+ if (X == SOME_MACRO && X != 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: expression is redundant
+ if (X == SOME_MACRO && X == 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always false
+
+ if (X < SOME_MACRO || X <= 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X >= SOME_MACRO || X > 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: expression is redundant
+ if (X == SOME_MACRO || X != 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+ if (X != SOME_MACRO || X == 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: expression is redundant
+ if (X != SOME_MACRO || X != 9) return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always true
+
+ // 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
+#undef SOME_OTHER_MACRO
+#undef SOME_MACRO_SAME_VALUE
+#undef SOME_MACRO
return 0;
}
@@ -419,7 +583,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 +601,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 +614,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 +643,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;
Index: docs/clang-tidy/checks/misc-redundant-expression.rst
===================================================================
--- docs/clang-tidy/checks/misc-redundant-expression.rst
+++ 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++
Index: clang-tidy/misc/RedundantExpressionCheck.h
===================================================================
--- clang-tidy/misc/RedundantExpressionCheck.h
+++ 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
+/// uneffective, useless parts.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-expression.html
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===================================================================
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ 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;
@@ -87,6 +86,7 @@
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();
@@ -96,6 +96,7 @@
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();
@@ -107,16 +108,18 @@
return areEquivalentNameSpecifier(
cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
+
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::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:
@@ -128,6 +131,7 @@
return false;
return cast<UnaryOperator>(Left)->getOpcode() ==
cast<UnaryOperator>(Right)->getOpcode();
+
case Stmt::BinaryOperatorClass:
return cast<BinaryOperator>(Left)->getOpcode() ==
cast<BinaryOperator>(Right)->getOpcode();
@@ -282,7 +286,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 +300,81 @@
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()) {
+ std::string 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 values 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);
+
+ if (ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context))
+ return true;
+
+ return false;
+}
+
+// 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 a symbolic expression (any expression except ingeter
-// constant expression).
+// 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();
@@ -331,8 +385,8 @@
return false;
}
-// Match a binary operator between a symbolic expression and an integer constant
-// expression.
+// Matches binary operators that are between a symbolic expression and an
+// integer constant expression.
static ast_matchers::internal::Matcher<Expr>
matchBinOpIntegerConstantExpr(StringRef Id) {
const auto BinOpCstExpr =
@@ -348,7 +402,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 +416,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 +442,7 @@
hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
.bind(NegateId);
+ // Do not bind to double negation, it is uneffective.
const auto NegateNegateRelationalExpr =
unaryOperator(hasOperatorName("!"),
hasUnaryOperand(unaryOperator(
@@ -398,23 +453,24 @@
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();
if (const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>(Id)) {
// 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 +487,174 @@
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());
+// Checks for expressions like (X == 4) && (Y != 9)
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+ if (!isa<BinaryOperator>(BinOp->getLHS()) ||
+ !isa<BinaryOperator>(BinOp->getRHS()))
+ return false;
+
+ BinaryOperator* LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
+ BinaryOperator* RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
+
+ if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
+ LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
+ (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
+ RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
+ return true;
+ return false;
}
-AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
- return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
+// 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();
+
+ BinaryOperator *BinOpLhs = dyn_cast<BinaryOperator>(BinOp->getLHS());
+ BinaryOperator *BinOpRhs = dyn_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(CallExpr, parametersAreEquivalent) {
- return Node.getNumArgs() == 2 &&
- areEquivalentExpr(Node.getArg(0), Node.getArg(1));
+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();
+
+ StringRef LhsMacroName = Lexer::getImmediateMacroName(LhsLoc, SM, LO);
+ StringRef RhsMacroName = Lexer::getImmediateMacroName(RhsLoc, SM, LO);
+
+ if (LhsMacroName == RhsMacroName)
+ return false;
+ return true;
}
-AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
- return Node.getOperatorLoc().isMacroID();
+static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr, const Expr *&RhsExpr) {
+ if (!LhsExpr || !RhsExpr)
+ return false;
+
+ SourceLocation LhsLoc = LhsExpr->getExprLoc();
+ SourceLocation RhsLoc = RhsExpr->getExprLoc();
+
+ if ((LhsLoc.isMacroID() && !RhsLoc.isMacroID()) ||
+ (!LhsLoc.isMacroID() && RhsLoc.isMacroID()))
+ return true;
+ return false;
}
-AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
- return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
+static bool isLessOperator(BinaryOperatorKind &Opcode) {
+ return Opcode == BO_LT || Opcode == BO_LE;
}
-AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
+static bool isGreaterOperator(BinaryOperatorKind &Opcode) {
+ return Opcode == BO_GT || Opcode == BO_GE;
+}
-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())
+// Checks whether a set of operators are able to create a meaningful and
+// non-redundant expression.
+// The function is used by `isIntendedMacroExpression` to filter operator sets
+// like (<, &&, <=), that always introduce redundant expressions.
+static bool isOperatorSetMeaningful(BinaryOperatorKind &Opcode,
+ BinaryOperatorKind &LhsOpcode,
+ BinaryOperatorKind &RhsOpcode) {
+
+ if (!BinaryOperator::isLogicalOp(Opcode))
+ return false;
+
+ if ((isLessOperator(LhsOpcode) && isGreaterOperator(RhsOpcode)) ||
+ (isLessOperator(RhsOpcode) && isGreaterOperator(LhsOpcode)))
+ return true;
+
+ if (Opcode == BO_LAnd) {
+ if (LhsOpcode == BO_NE &&
+ (RhsOpcode == BO_NE || BinaryOperator::isRelationalOp(RhsOpcode)))
return true;
- Loc = SM.getImmediateMacroCallerLoc(Loc);
+ if (RhsOpcode == BO_NE && BinaryOperator::isRelationalOp(LhsOpcode))
+ return true;
+ return false;
}
+
+ if (Opcode == BO_LOr) {
+ if (LhsOpcode == BO_EQ &&
+ (RhsOpcode == BO_EQ || BinaryOperator::isRelationalOp(RhsOpcode)))
+ return true;
+ if (RhsOpcode == BO_EQ && BinaryOperator::isRelationalOp(LhsOpcode))
+ return true;
+ return false;
+ }
+
+ return false;
+}
+
+// Checks whether an expression containing one or two macros,
+// like: ((Expr <op> M1/K) <REL> (Expr <op> M2)), can be a meaningful one
+// based on the operators.
+// A macro expression is considered meaningful, if the macro or macros can hold
+// values, with which the expression cannot be simplified further.
+// The decison is independent from the actual values of the macros, since
+// they may vary across builds.
+// Examples:
+// The function discards (X < M1 && X <= M2), because it can always be
+// simplified to X < M, where M is the smaller one of the two macro
+// values.
+// An expresion that can be considered meaningful and intended is:
+// (X < M1 && X > M2), even when the two ranges are exclusive.
+static bool isIntendedMacroExpression(BinaryOperatorKind &Opcode,
+ BinaryOperatorKind &LhsOpcode,
+ const Expr *&LhsExpr,
+ BinaryOperatorKind &RhsOpcode,
+ const Expr *&RhsExpr,
+ const ASTContext *AstCtx) {
+ if ((areExprsFromDifferentMacros(LhsExpr, RhsExpr, AstCtx) ||
+ areExprsMacroAndNonMacro(LhsExpr, RhsExpr)) &&
+ isOperatorSetMeaningful(Opcode, LhsOpcode, RhsOpcode))
+ return true;
return false;
}
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(Names));
+ const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(KnownBannedMacroNames));
+ // Binary with equivalent operands, like (X != 2 && X != 2).
Finder->addMatcher(
binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
hasOperatorName("%"), hasOperatorName("|"),
@@ -499,15 +673,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 +788,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,31 +849,38 @@
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);
std::swap(LhsSymbol, RhsSymbol);
std::swap(LhsOpcode, RhsOpcode);
}
+ if (isIntendedMacroExpression(Opcode, LhsOpcode, LhsConst, RhsOpcode,
+ RhsConst, Result.Context))
+ 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 +909,63 @@
}
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"))
+ 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 (isIntendedMacroExpression(MainOpcode, SideOpcode, LhsConst,
+ SideOpcode, RhsConst, Result.Context))
+ return;
+ }
+
+ diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
+ }
+ if (const auto *CondOp =
+ Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
+
+ const auto *TrueExpr = CondOp->getTrueExpr();
+ const auto *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 side of overloaded operator are equivalent");
+ "both sides of overloaded operator are equivalent");
+ }
+ // Check for the following binded 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 binded expression:
+ // - "binop-const-compare-to-const",
+ // Produced message:
+ // -> "logical expression is always false/true"
checkBitwiseExpr(Result);
+
+ // Check for te following binded 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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
