barancsuk created this revision.
barancsuk added a project: clang-tools-extra.
Redundant Expression Checker is updated to be able to detect expressions that
contain macro constants. Also, other small details are modified to improve the
current implementation.
The improvements in detail are as follows:
**1.)** 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 correctness of a macro
expression is decided based on solely the operators involved in the expression.
Examples:
The following expression is always redundant, independently of the macro
values, because the subexpression containing the larger constant can be
eliminated without changing the meaning of the expression:
(X < MACRO && X < OTHER_MACRO)
On the contrary, the following expression is considered meaningful, because
some macro values (e.g.: MACRO = 3, OTHER_MACRO = 2) produce a valid interval:
(X < MACRO && X > OTHER_MACRO)
**2.) **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.
**3.)** The checker is now able to detect redundant CXXFunctionalCastExprs as
well (it matched only CStyleCastExprs before). A corresponding test case is
added.
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
@@ -87,6 +87,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 +97,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,6 +109,7 @@
return areEquivalentNameSpecifier(
cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
+
case Stmt::DeclRefExprClass:
return cast<DeclRefExpr>(Left)->getDecl() ==
cast<DeclRefExpr>(Right)->getDecl();
@@ -118,6 +121,10 @@
return cast<CStyleCastExpr>(Left)->getTypeAsWritten() ==
cast<CStyleCastExpr>(Right)->getTypeAsWritten();
+ case Stmt::CXXFunctionalCastExprClass:
+ return cast<CXXFunctionalCastExpr>(Left)->getTypeAsWritten() ==
+ cast<CXXFunctionalCastExpr>(Right)->getTypeAsWritten();
+
case Stmt::CallExprClass:
case Stmt::ImplicitCastExprClass:
case Stmt::ArraySubscriptExprClass:
@@ -128,6 +135,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 +290,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,20 +304,64 @@
return Node.isIntegerConstantExpr(Finder->getASTContext());
}
+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, 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;
+}
+
// Returns a matcher for integer constant expression.
static ast_matchers::internal::Matcher<Expr>
matchIntegerConstantExpr(StringRef Id) {
std::string CstId = (Id + "-const").str();
return expr(isIntegerConstantExpr()).bind(CstId);
}
-// Retrieve the integer value matched by 'matchIntegerConstantExpr' with name
+// Retrieves the integer value matched by 'matchIntegerConstantExpr' with name
// 'Id' and store it into 'Value'.
static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
- StringRef Id, APSInt &Value) {
+ 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);
+}
+
+// Overloaded `retrieveIntegerConstantExprs` for compatibility purposes
+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
@@ -319,7 +372,7 @@
expr(unless(isIntegerConstantExpr())).bind(SymId));
}
-// Retrieve the expression matched by 'matchSymbolicExpr' with name 'Id' and
+// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
// store it into 'SymExpr'.
static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
StringRef Id, const Expr *&SymExpr) {
@@ -331,7 +384,7 @@
return false;
}
-// Match a binary operator between a symbolic expression and an integer constant
+// Matches a binary operator between a symbolic expression and an integer constant
// expression.
static ast_matchers::internal::Matcher<Expr>
matchBinOpIntegerConstantExpr(StringRef Id) {
@@ -348,7 +401,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,
@@ -388,6 +441,7 @@
hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
.bind(NegateId);
+ // Do not bind to double negation, it is uneffective.
const auto NegateNegateRelationalExpr =
unaryOperator(hasOperatorName("!"),
hasUnaryOperand(unaryOperator(
@@ -398,22 +452,21 @@
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).
@@ -431,43 +484,153 @@
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 void 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();
+
+ assert((BinOpLhs->getOpcode() == BinOpRhs->getOpcode()) &&
+ "Sides of the binary operator must be equivalent expressions!");
+
+ SideOpcode = BinOpLhs->getOpcode();
}
-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())
+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;
}
@@ -481,6 +644,7 @@
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 +663,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 +778,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 +839,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 +899,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"))
+ 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;
+
+ retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode, LhsConst,
+ RhsConst, Result.Context);
+
+ 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
