[analyzer] Remove alpha.core.IdenticalExpr Checker #114715
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-static-analyzer-1 Author: None (vabridgers) ChangesThis change removes the alpha.core.IdenticalExpr static analysis checker since it's checks are present in the clang-tidy checks misc-redundant-expression and bugprone-branch-clone. This check was implemented as a static analysis check using AST matching, and since alpha and duplicated in 2 clang-tidy checks may be removed. Patch is 108.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114715.diff 8 Files Affected:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
index 42231746149f2c..c6af207d795c5d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
@@ -1069,3 +1069,550 @@ namespace PR62693 {
}
}
}
+
+// Start of identical expressions port
+int func(void)
+{
+ return 0;
+}
+
+int func2(void)
+{
+ return 0;
+}
+
+int funcParam(int a)
+{
+ return 0;
+}
+unsigned test_unsigned(unsigned a) {
+ unsigned b = 1;
+ a = a > 5 ? b : b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+ return a;
+}
+void test_signed() {
+ int a = 0;
+ a = a > 5 ? a : a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_bool(bool a) {
+ a = a > 0 ? a : a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_float() {
+ float a = 0;
+ float b = 0;
+ a = a > 5 ? a : a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+const char *test_string() {
+ float a = 0;
+ return a > 5 ? "abc" : "abc";
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_unsigned_expr() {
+ unsigned a = 0;
+ unsigned b = 0;
+ a = a > 5 ? a+b : a+b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_signed_expr() {
+ int a = 0;
+ int b = 1;
+ a = a > 5 ? a+b : a+b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_bool_expr(bool a) {
+ bool b = 0;
+ a = a > 0 ? a&&b : a&&b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_unsigned_expr_negative() {
+ unsigned a = 0;
+ unsigned b = 0;
+ a = a > 5 ? a+b : b+a; // no warning
+}
+
+void test_signed_expr_negative() {
+ int a = 0;
+ int b = 1;
+ a = a > 5 ? b+a : a+b; // no warning
+}
+
+void test_bool_expr_negative(bool a) {
+ bool b = 0;
+ a = a > 0 ? a&&b : b&&a; // no warning
+}
+
+void test_float_expr_positive() {
+ float a = 0;
+ float b = 0;
+ a = a > 5 ? a+b : a+b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_positive_func() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+func() : a+func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_func() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+func() : a+func2(); // no warning
+}
+
+void test_expr_positive_funcParam() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+funcParam(b) : a+funcParam(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_funcParam() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+funcParam(a) : a+funcParam(b); // no warning
+}
+
+void test_expr_positive_inc() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a++ : a++;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_inc() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a++ : b++; // no warning
+}
+
+void test_expr_positive_assign() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a=1 : a=1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_assign() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a=1 : a=2; // no warning
+}
+
+void test_signed_nested_expr() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? a+b+(c+a)*(a + b*(c+a)) : a+b+(c+a)*(a + b*(c+a));
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_signed_nested_expr_negative() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? a+b+(c+a)*(a + b*(c+a)) : a+b+(c+a)*(a + b*(a+c)); // no warning
+}
+
+void test_signed_nested_cond_expr_negative() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? (b > 5 ? 1 : 4) : (b > 5 ? 2 : 4); // no warning
+}
+
+void test_signed_nested_cond_expr() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? (b > 5 ? 1 : 4) : (b > 5 ? 4 : 4);
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_identical_branches1(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ ++i;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ ++i;
+ }
+}
+
+void test_identical_branches2(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ ++i;
+ } else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ ++i;
+}
+
+void test_identical_branches3(bool b) {
+ int i = 0;
+ if (b) { // no warning
+ ++i;
+ } else {
+ i++;
+ }
+}
+
+void test_identical_branches4(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ }
+}
+
+void test_identical_branches_break(bool b) {
+ while (true) {
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: if with identical then and else branches [bugprone-branch-clone]
+ break;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ break;
+ }
+}
+
+void test_identical_branches_continue(bool b) {
+ while (true) {
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: if with identical then and else branches [bugprone-branch-clone]
+ continue;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ continue;
+ }
+}
+
+void test_identical_branches_func(bool b) {
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ func();
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: else branch starts here
+ func();
+}
+
+void test_identical_branches_func_arguments(bool b) {
+ if (b) // no-warning
+ funcParam(1);
+ else
+ funcParam(2);
+}
+
+void test_identical_branches_cast1(bool b) {
+ long v = -7;
+ if (b) // no-warning
+ v = (signed int) v;
+ else
+ v = (unsigned int) v;
+}
+
+void test_identical_branches_cast2(bool b) {
+ long v = -7;
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ v = (signed int) v;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: else branch starts here
+ v = (signed int) v;
+}
+
+int test_identical_branches_return_int(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ i++;
+ return i;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ i++;
+ return i;
+ }
+}
+
+int test_identical_branches_return_func(bool b) {
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ return func();
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ return func();
+ }
+}
+
+void test_identical_branches_for(bool b) {
+ int i;
+ int j;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ for (i = 0, j = 0; i < 10; i++)
+ j += 4;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ for (i = 0, j = 0; i < 10; i++)
+ j += 4;
+ }
+}
+
+void test_identical_branches_while(bool b) {
+ int i = 10;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ while (func())
+ i--;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ while (func())
+ i--;
+ }
+}
+
+void test_identical_branches_while_2(bool b) {
+ int i = 10;
+ if (b) { // no-warning
+ while (func())
+ i--;
+ } else {
+ while (func())
+ i++;
+ }
+}
+
+void test_identical_branches_do_while(bool b) {
+ int i = 10;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ do {
+ i--;
+ } while (func());
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ do {
+ i--;
+ } while (func());
+ }
+ // C --HECK-MESSAGES: :[[@LINE-3]]:44: warning: 'true' and 'false' expressions are equivalent [misc-redundant-expression]
+}
+
+void test_identical_branches_if(bool b, int i) {
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ if (i < 5)
+ i += 10;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ if (i < 5)
+ i += 10;
+ }
+}
+
+void test_warn_chained_if_stmts_1(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_warn_chained_if_stmts_2(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+}
+
+void test_warn_chained_if_stmts_3(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+}
+
+void test_warn_chained_if_stmts_4(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (func())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 1)
+ ;
+}
+
+void test_warn_chained_if_stmts_5(int x) {
+ if (x & 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x & 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_warn_chained_if_stmts_6(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 3 starts here
+}
+
+void test_warn_chained_if_stmts_7(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 3 starts here
+ else if (x == 5)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 4 starts here
+}
+
+
+void test_warn_chained_if_stmts_8(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 3 starts here
+ else if (x == 5)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 4 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 5 starts here
+ else if (x == 7)
+ ;
+}
+
+void test_nowarn_chained_if_stmts_1(int x) {
+ if (func())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (func()) // no-warning
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_nowarn_chained_if_stmts_2(int x) {
+ if (func())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (func()) // no-warning
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+}
+
+void test_nowarn_chained_if_stmts_3(int x) {
+ if (x++)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x++) // no-warning
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_warn_wchar() {
+ const wchar_t * a = 0 ? L"Expression" : L"Expression"; // expected-warning {{identical expressions on both sides of ':' in conditional expression}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+void test_nowarn_wchar() {
+ const wchar_t * a = 0 ? L"No" : L"Warning";
+}
+
+void test_nowarn_long() {
+ int a = 0, b = 0;
+ long c;
+ if (0) {
+ b -= a;
+ c = 0;
+ } else { // no-warning
+ b -= a;
+ c = 0LL;
+ }
+}
+
+// Identical inner conditions
+
+void test_warn_inner_if_1(int x) {
+ if (x == 1) {
+ if (x == 1) // expected-warning {{conditions of the inner and outer statements are identical}}
+ ;
+ }
+
+ // FIXME: Should warn here. The warning is currently not emitted because there
+ // is code between the conditions.
+ if (x == 1) {
+ int y = x;
+ if (x == 1)
+ ;
+ }
+}
+
+void test_nowarn_inner_if_1(int x) {
+ // Don't warn when condition has side effects.
+ if (x++ == 1) {
+ if (x++ == 1)
+ ;
+ }
+
+ // Don't warn when x is changed before inner condition.
+ if (x < 10) {
+ x++;
+ if (x < 10)
+ ;
+ }
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp
index 1b271630e0d193..f63661a03c7c89 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp
@@ -863,3 +863,1328 @@ namespace PR35857 {
decltype(x + y - (x + y)) z = 10;
}
}
+
+// Ported from identical-expression
+
+int func(void)
+{
+ return 0;
+}
+
+int func2(void)
+{
+ return 0;
+}
+
+int funcParam(int a)
+{
+ return 0;
+}
+
+/* '!=' operator*/
+
+/* '!=' with float */
+int checkNotEqualFloatLiteralCompare1(void) {
+ return (5.14F != 5.14F); // no warning
+}
+
+int checkNotEqualFloatLiteralCompare2(void) {
+ return (6.14F != 7.14F); // no warning
+}
+
+int checkNotEqualFloatDeclCompare1(void) {
+ float f = 7.1F;
+ float g = 7.1F;
+ return (f != g); // no warning
+}
+
+int checkNotEqualFloatDeclCompare12(void) {
+ float f = 7.1F;
+ return (f != f); // no warning
+}
+
+int checkNotEqualFloatDeclCompare3(void) {
+ float f = 7.1F;
+ return (f != 7.1F); // no warning
+}
+
+int checkNotEqualFloatDeclCompare4(void) {
+ float f = 7.1F;
+ return (7.1F != f); // no warning
+}
+
+int checkNotEqualFloatDeclCompare5(void) {
+ float f = 7.1F;
+ int t = 7;
+ return (t != f); // no warning
+}
+
+int checkNotEqualFloatDeclCompare6(void) {
+ float f = 7.1F;
+ int t = 7;
+ return (f != t); // no warning
+}
+
+
+
+int checkNotEqualCastFloatDeclCompare11(void) {
+ float f = 7.1F;
+ return ((int)f != (int)f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent [misc-redundant-expression]
+}
+int checkNotEqualCastFloatDeclCompare12(void) {
+ float f = 7.1F;
+ return ((char)f != (int)f); // no warning
+}
+int checkNotEqualBinaryOpFloatCompare1(void) {
+ int res;
+ float f= 3.14F;
+ res = (f + 3.14F != f + 3.14F); // no warning
+ return (0);
+}
+int checkNotEqualBinaryOpFloatCompare2(void) {
+ float f = 7.1F;
+ float g = 7.1F;
+ return (f + 3.14F != g + 3.14F); // no warning
+}
+int checkNotEqualBinaryOpFloatCompare3(void) {
+ int res;
+ float f= 3.14F;
+ res = ((int)f + 3.14F != (int)f + 3.14F); // no warning
+ return (0);
+}
+int checkNotEqualBinaryOpFloatCompare4(void) {
+ int res;
+ float f= 3.14F;
+ res = ((int)f + 3.14F != (char)f + 3.14F); // no warning
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpFloatCompare1(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ float f= 3.14F;
+ res = (((int)f + (3.14F - u)*t) != ((int)f + (3.14F - u)*t)); // no warning
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpFloatCompare2(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ float f= 3.14F;
+ res = (((int)f + (u - 3.14F)*t) != ((int)f + (3.14F - u)*t)); // no warning
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpFloatCompare3(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ float f= 3.14F;
+ res = (((int)f + (u - 3.14F)*t) != ((int)f + (3.14F - u)*(f + t != f + t))); // no warning
+ return (0);
+}
+
+
+/* end '!=' with float*/
+/* '!=' with int*/
+
+int checkNotEqualIntLiteralCompare1(void) {
+ // FALSE NEGATIVE!
+ return (5 != 5);
+ // C HECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent [misc-redundant-expression]
+}
+
+int checkNotEqualIntLiteralCompare2(void) {
+ return (6 != 7); // no warning
+}
+
+int checkNotEqualIntDeclCompare1(void) {
+ int f = 7;
+ int g = 7;
+ return (f != g); // no warning
+}
+
+int checkNotEqualIntDeclCompare3(void) {
+ int f = 7;
+ return (f != 7); // no warning
+}
+
+int checkNotEqualIntDeclCompare4(void) {
+ int f = 7;
+ return (7 != f); // no warning
+}
+
+int checkNotEqualCastIntDeclCompare11(void) {
+ int f = 7;
+ return ((int)f != (int)f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent [misc-redundant-expression]
+}
+int checkNotEqualCastIntDeclCompare12(void) {
+ int f = 7;
+ return ((char)f != (int)f); // no warning
+}
+int checkNotEqualBinaryOpIntCompare1(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 4;
+ res = (f + 4 != f + 4);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: both sides of operator are equivalent [misc-redundant-expression]
+ return (0);
+}
+int checkNotEqualBinaryOpIntCompare2(void) {
+ int f = 7;
+ int g = 7;
+ return (f + 4 != g + 4); // no warning
+}
+
+int checkNotEqualBinaryOpIntCompare3(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 4;
+ res = ((int)f + 4 != (int)f + 4);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: both sides of operator are equivalent [misc-redundant-expression]
+ return (0);
+}
+int checkNotEqualBinaryOpIntCompare4(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 4;
+ res = ((int)f + 4 != (char)f + 4); // no warning
+ return (0);
+}
+int checkNotEqualBinaryOpIntCompare5(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ res = (u + t != u + t);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: both sides of operator are equivalent [misc-redundant-expression]
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpIntCompare1(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 3;
+ res = (((int)f + (3 - u)*t) !=...
[truncated]
|
|
@llvm/pr-subscribers-clang-tools-extra Author: None (vabridgers) ChangesThis change removes the alpha.core.IdenticalExpr static analysis checker since it's checks are present in the clang-tidy checks misc-redundant-expression and bugprone-branch-clone. This check was implemented as a static analysis check using AST matching, and since alpha and duplicated in 2 clang-tidy checks may be removed. Patch is 108.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114715.diff 8 Files Affected:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
index 42231746149f2c..c6af207d795c5d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
@@ -1069,3 +1069,550 @@ namespace PR62693 {
}
}
}
+
+// Start of identical expressions port
+int func(void)
+{
+ return 0;
+}
+
+int func2(void)
+{
+ return 0;
+}
+
+int funcParam(int a)
+{
+ return 0;
+}
+unsigned test_unsigned(unsigned a) {
+ unsigned b = 1;
+ a = a > 5 ? b : b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+ return a;
+}
+void test_signed() {
+ int a = 0;
+ a = a > 5 ? a : a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_bool(bool a) {
+ a = a > 0 ? a : a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_float() {
+ float a = 0;
+ float b = 0;
+ a = a > 5 ? a : a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+const char *test_string() {
+ float a = 0;
+ return a > 5 ? "abc" : "abc";
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_unsigned_expr() {
+ unsigned a = 0;
+ unsigned b = 0;
+ a = a > 5 ? a+b : a+b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_signed_expr() {
+ int a = 0;
+ int b = 1;
+ a = a > 5 ? a+b : a+b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_bool_expr(bool a) {
+ bool b = 0;
+ a = a > 0 ? a&&b : a&&b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_unsigned_expr_negative() {
+ unsigned a = 0;
+ unsigned b = 0;
+ a = a > 5 ? a+b : b+a; // no warning
+}
+
+void test_signed_expr_negative() {
+ int a = 0;
+ int b = 1;
+ a = a > 5 ? b+a : a+b; // no warning
+}
+
+void test_bool_expr_negative(bool a) {
+ bool b = 0;
+ a = a > 0 ? a&&b : b&&a; // no warning
+}
+
+void test_float_expr_positive() {
+ float a = 0;
+ float b = 0;
+ a = a > 5 ? a+b : a+b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_positive_func() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+func() : a+func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_func() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+func() : a+func2(); // no warning
+}
+
+void test_expr_positive_funcParam() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+funcParam(b) : a+funcParam(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_funcParam() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a+funcParam(a) : a+funcParam(b); // no warning
+}
+
+void test_expr_positive_inc() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a++ : a++;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_inc() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a++ : b++; // no warning
+}
+
+void test_expr_positive_assign() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a=1 : a=1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_expr_negative_assign() {
+ unsigned a = 0;
+ unsigned b = 1;
+ a = a > 5 ? a=1 : a=2; // no warning
+}
+
+void test_signed_nested_expr() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? a+b+(c+a)*(a + b*(c+a)) : a+b+(c+a)*(a + b*(c+a));
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_signed_nested_expr_negative() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? a+b+(c+a)*(a + b*(c+a)) : a+b+(c+a)*(a + b*(a+c)); // no warning
+}
+
+void test_signed_nested_cond_expr_negative() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? (b > 5 ? 1 : 4) : (b > 5 ? 2 : 4); // no warning
+}
+
+void test_signed_nested_cond_expr() {
+ int a = 0;
+ int b = 1;
+ int c = 3;
+ a = a > 5 ? (b > 5 ? 1 : 4) : (b > 5 ? 4 : 4);
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+
+void test_identical_branches1(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ ++i;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ ++i;
+ }
+}
+
+void test_identical_branches2(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ ++i;
+ } else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ ++i;
+}
+
+void test_identical_branches3(bool b) {
+ int i = 0;
+ if (b) { // no warning
+ ++i;
+ } else {
+ i++;
+ }
+}
+
+void test_identical_branches4(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ }
+}
+
+void test_identical_branches_break(bool b) {
+ while (true) {
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: if with identical then and else branches [bugprone-branch-clone]
+ break;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ break;
+ }
+}
+
+void test_identical_branches_continue(bool b) {
+ while (true) {
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: if with identical then and else branches [bugprone-branch-clone]
+ continue;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ continue;
+ }
+}
+
+void test_identical_branches_func(bool b) {
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ func();
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: else branch starts here
+ func();
+}
+
+void test_identical_branches_func_arguments(bool b) {
+ if (b) // no-warning
+ funcParam(1);
+ else
+ funcParam(2);
+}
+
+void test_identical_branches_cast1(bool b) {
+ long v = -7;
+ if (b) // no-warning
+ v = (signed int) v;
+ else
+ v = (unsigned int) v;
+}
+
+void test_identical_branches_cast2(bool b) {
+ long v = -7;
+ if (b)
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ v = (signed int) v;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: else branch starts here
+ v = (signed int) v;
+}
+
+int test_identical_branches_return_int(bool b) {
+ int i = 0;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ i++;
+ return i;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ i++;
+ return i;
+ }
+}
+
+int test_identical_branches_return_func(bool b) {
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ return func();
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ return func();
+ }
+}
+
+void test_identical_branches_for(bool b) {
+ int i;
+ int j;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ for (i = 0, j = 0; i < 10; i++)
+ j += 4;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ for (i = 0, j = 0; i < 10; i++)
+ j += 4;
+ }
+}
+
+void test_identical_branches_while(bool b) {
+ int i = 10;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ while (func())
+ i--;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ while (func())
+ i--;
+ }
+}
+
+void test_identical_branches_while_2(bool b) {
+ int i = 10;
+ if (b) { // no-warning
+ while (func())
+ i--;
+ } else {
+ while (func())
+ i++;
+ }
+}
+
+void test_identical_branches_do_while(bool b) {
+ int i = 10;
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ do {
+ i--;
+ } while (func());
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ do {
+ i--;
+ } while (func());
+ }
+ // C --HECK-MESSAGES: :[[@LINE-3]]:44: warning: 'true' and 'false' expressions are equivalent [misc-redundant-expression]
+}
+
+void test_identical_branches_if(bool b, int i) {
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+ if (i < 5)
+ i += 10;
+ } else {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+ if (i < 5)
+ i += 10;
+ }
+}
+
+void test_warn_chained_if_stmts_1(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_warn_chained_if_stmts_2(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+}
+
+void test_warn_chained_if_stmts_3(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+}
+
+void test_warn_chained_if_stmts_4(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (func())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 1)
+ ;
+}
+
+void test_warn_chained_if_stmts_5(int x) {
+ if (x & 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x & 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_warn_chained_if_stmts_6(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 3 starts here
+}
+
+void test_warn_chained_if_stmts_7(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 3 starts here
+ else if (x == 5)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 4 starts here
+}
+
+
+void test_warn_chained_if_stmts_8(int x) {
+ if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+ else if (x == 2)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 3 starts here
+ else if (x == 5)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 4 starts here
+ else if (x == 3)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 5 starts here
+ else if (x == 7)
+ ;
+}
+
+void test_nowarn_chained_if_stmts_1(int x) {
+ if (func())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (func()) // no-warning
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_nowarn_chained_if_stmts_2(int x) {
+ if (func())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x == 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+ else if (func()) // no-warning
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 2 starts here
+}
+
+void test_nowarn_chained_if_stmts_3(int x) {
+ if (x++)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: repeated branch body in conditional chain
+ else if (x++) // no-warning
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: note: clone 1 starts here
+}
+
+void test_warn_wchar() {
+ const wchar_t * a = 0 ? L"Expression" : L"Expression"; // expected-warning {{identical expressions on both sides of ':' in conditional expression}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: conditional operator with identical true and false expressions [bugprone-branch-clone]
+}
+void test_nowarn_wchar() {
+ const wchar_t * a = 0 ? L"No" : L"Warning";
+}
+
+void test_nowarn_long() {
+ int a = 0, b = 0;
+ long c;
+ if (0) {
+ b -= a;
+ c = 0;
+ } else { // no-warning
+ b -= a;
+ c = 0LL;
+ }
+}
+
+// Identical inner conditions
+
+void test_warn_inner_if_1(int x) {
+ if (x == 1) {
+ if (x == 1) // expected-warning {{conditions of the inner and outer statements are identical}}
+ ;
+ }
+
+ // FIXME: Should warn here. The warning is currently not emitted because there
+ // is code between the conditions.
+ if (x == 1) {
+ int y = x;
+ if (x == 1)
+ ;
+ }
+}
+
+void test_nowarn_inner_if_1(int x) {
+ // Don't warn when condition has side effects.
+ if (x++ == 1) {
+ if (x++ == 1)
+ ;
+ }
+
+ // Don't warn when x is changed before inner condition.
+ if (x < 10) {
+ x++;
+ if (x < 10)
+ ;
+ }
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp
index 1b271630e0d193..f63661a03c7c89 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp
@@ -863,3 +863,1328 @@ namespace PR35857 {
decltype(x + y - (x + y)) z = 10;
}
}
+
+// Ported from identical-expression
+
+int func(void)
+{
+ return 0;
+}
+
+int func2(void)
+{
+ return 0;
+}
+
+int funcParam(int a)
+{
+ return 0;
+}
+
+/* '!=' operator*/
+
+/* '!=' with float */
+int checkNotEqualFloatLiteralCompare1(void) {
+ return (5.14F != 5.14F); // no warning
+}
+
+int checkNotEqualFloatLiteralCompare2(void) {
+ return (6.14F != 7.14F); // no warning
+}
+
+int checkNotEqualFloatDeclCompare1(void) {
+ float f = 7.1F;
+ float g = 7.1F;
+ return (f != g); // no warning
+}
+
+int checkNotEqualFloatDeclCompare12(void) {
+ float f = 7.1F;
+ return (f != f); // no warning
+}
+
+int checkNotEqualFloatDeclCompare3(void) {
+ float f = 7.1F;
+ return (f != 7.1F); // no warning
+}
+
+int checkNotEqualFloatDeclCompare4(void) {
+ float f = 7.1F;
+ return (7.1F != f); // no warning
+}
+
+int checkNotEqualFloatDeclCompare5(void) {
+ float f = 7.1F;
+ int t = 7;
+ return (t != f); // no warning
+}
+
+int checkNotEqualFloatDeclCompare6(void) {
+ float f = 7.1F;
+ int t = 7;
+ return (f != t); // no warning
+}
+
+
+
+int checkNotEqualCastFloatDeclCompare11(void) {
+ float f = 7.1F;
+ return ((int)f != (int)f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent [misc-redundant-expression]
+}
+int checkNotEqualCastFloatDeclCompare12(void) {
+ float f = 7.1F;
+ return ((char)f != (int)f); // no warning
+}
+int checkNotEqualBinaryOpFloatCompare1(void) {
+ int res;
+ float f= 3.14F;
+ res = (f + 3.14F != f + 3.14F); // no warning
+ return (0);
+}
+int checkNotEqualBinaryOpFloatCompare2(void) {
+ float f = 7.1F;
+ float g = 7.1F;
+ return (f + 3.14F != g + 3.14F); // no warning
+}
+int checkNotEqualBinaryOpFloatCompare3(void) {
+ int res;
+ float f= 3.14F;
+ res = ((int)f + 3.14F != (int)f + 3.14F); // no warning
+ return (0);
+}
+int checkNotEqualBinaryOpFloatCompare4(void) {
+ int res;
+ float f= 3.14F;
+ res = ((int)f + 3.14F != (char)f + 3.14F); // no warning
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpFloatCompare1(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ float f= 3.14F;
+ res = (((int)f + (3.14F - u)*t) != ((int)f + (3.14F - u)*t)); // no warning
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpFloatCompare2(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ float f= 3.14F;
+ res = (((int)f + (u - 3.14F)*t) != ((int)f + (3.14F - u)*t)); // no warning
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpFloatCompare3(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ float f= 3.14F;
+ res = (((int)f + (u - 3.14F)*t) != ((int)f + (3.14F - u)*(f + t != f + t))); // no warning
+ return (0);
+}
+
+
+/* end '!=' with float*/
+/* '!=' with int*/
+
+int checkNotEqualIntLiteralCompare1(void) {
+ // FALSE NEGATIVE!
+ return (5 != 5);
+ // C HECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent [misc-redundant-expression]
+}
+
+int checkNotEqualIntLiteralCompare2(void) {
+ return (6 != 7); // no warning
+}
+
+int checkNotEqualIntDeclCompare1(void) {
+ int f = 7;
+ int g = 7;
+ return (f != g); // no warning
+}
+
+int checkNotEqualIntDeclCompare3(void) {
+ int f = 7;
+ return (f != 7); // no warning
+}
+
+int checkNotEqualIntDeclCompare4(void) {
+ int f = 7;
+ return (7 != f); // no warning
+}
+
+int checkNotEqualCastIntDeclCompare11(void) {
+ int f = 7;
+ return ((int)f != (int)f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent [misc-redundant-expression]
+}
+int checkNotEqualCastIntDeclCompare12(void) {
+ int f = 7;
+ return ((char)f != (int)f); // no warning
+}
+int checkNotEqualBinaryOpIntCompare1(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 4;
+ res = (f + 4 != f + 4);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: both sides of operator are equivalent [misc-redundant-expression]
+ return (0);
+}
+int checkNotEqualBinaryOpIntCompare2(void) {
+ int f = 7;
+ int g = 7;
+ return (f + 4 != g + 4); // no warning
+}
+
+int checkNotEqualBinaryOpIntCompare3(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 4;
+ res = ((int)f + 4 != (int)f + 4);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: both sides of operator are equivalent [misc-redundant-expression]
+ return (0);
+}
+int checkNotEqualBinaryOpIntCompare4(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 4;
+ res = ((int)f + 4 != (char)f + 4); // no warning
+ return (0);
+}
+int checkNotEqualBinaryOpIntCompare5(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ res = (u + t != u + t);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: both sides of operator are equivalent [misc-redundant-expression]
+ return (0);
+}
+
+int checkNotEqualNestedBinaryOpIntCompare1(void) {
+ int res;
+ int t= 1;
+ int u= 2;
+ int f= 3;
+ res = (((int)f + (3 - u)*t) !=...
[truncated]
|
NagyDonat
reviewed
Nov 4, 2024
There was a problem hiding this comment.
Thanks for implementing this cleanup commit!
I like that you ported the tests from the old checker to the tidy checks. However, as these test files are very large, perhaps it would be better to put these moved tests into stand-alone files instead of adding them at the end of existing files. (@ tidy developers: what would you prefer?)
Do I understand it correctly that the two tidy checkers report all the issues that were reported by alpha.core.IdenticalExpr? (Unfortunately github cannot show a clear diff between the old and new version of the moved code.)
NagyDonat
requested review from
5chmidti,
EugeneZelenko,
erichkeane,
haoNoQ and
steakhal
I agree! |
|
Hi @NagyDonat, I can port to new and smaller tests - no problem. This was an initial, speculative PR to get the conversation going, especially for comments like this. I'll update the PR. I did take the cases one by one from identical-expressions.cpp to the tidy checks and noticed one pattern. It looks to me like identical expressions utilizing floats were excluded in the tidy checks but found in the static analysis check. Do you want to retain that through improvements in misc-redundant-expression, or leave that marked as FIXME? I'm happy to take a look at an improvement like that. Thanks! - Vince |
| ^^^^^^^^^^^^^^ | ||
|
|
||
| - The checker ``alpha.core.IdenticalExpr`` was removed because it was | ||
| duplicated in the clang-tidy checkers ``misc-redundant-expression`` and |
There was a problem hiding this comment.
Suggested change
| duplicated in the clang-tidy checkers ``misc-redundant-expression`` and | |
| duplicated in the Clang-Tidy checks ``misc-redundant-expression`` and |
If it's not too difficult, then please improve the tidy checks to cover everything that was covered by the static analyzer checker that we want to remove. (Obviously this doesn't have a major practical importance as I assume that practically nobody used this alpha checker, but it's still nice to say that we preserve every feature.) (Disclaimer: if you think that the analyzer-only features happen to be buggy and/or completely useless, then don't port them. I have seen wild things in other old alpha checkers...) If the changes are complex and you want to create several separate commits, then I think the natural commit order would be (1) adding the new features in tidy and then (2) removing the analyzer checker in a follow-up commit. However, I'd guess that it's reasonable to do these all in a single commit. |
|
@NagyDonat , sounds good. Thank you! |
|
I'm pretty sure last time I've compared this checker to the tidy implementation, I saw slight differences. Speaking of the tests, I'd also prefer migrating existing ones to uncover differences. BTW I'm all in favor of reducing code duplication, and clang-tidy is a much better place for this rule. |
|
+1 to ensuring the current clang-tidy checks have equal or better coverage than the existing CSA check. |
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
2 times, most recently
from
2508ede to
8f80764
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
I've reworked the test port in a "scaffolded" way so reviewers may see how the cases were considered one by one, and the existing tidy checkers improved to address cases IdenticalExpr was detecting and the tidy checkers were not. I needed to weaken the misc-redundant-expression AST matcher a bit, and consider an extra case for the bugprone-branch-clone checker. This picked up the integer and floating point literal cases, and also the duplicate outer/inner if cases. I'm happy to take next suggested steps to drive this improvement to completion, and would like some guidance on if the additional "scaffolded" case should be kept in place (to maintain clear differences from the original to new file in the git diff), or split the file now or maybe in a future separate commit so the differences are clear. Thanks - Vince |
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
8f80764 to
2834ec4
Compare
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
2834ec4 to
d20240f
Compare
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
d20240f to
24e27a6
Compare
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
24e27a6 to
eed1a2d
Compare
HerrCai0907
reviewed
Nov 8, 2024
|
Thanks @HerrCai0907, I'll update the documents and release notes. Best |
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
eed1a2d to
3f3df0c
Compare
5chmidti
reviewed
Nov 13, 2024
There was a problem hiding this comment.
some guidance on if the additional "scaffolded" case should be kept in place (to maintain clear differences from the original to new file in the git diff), or split the file now or maybe in a future separate commit so the differences are clear.
IMO it is better as a follow-up so that the diff of the commit stays readable, which can also remove duplicate tests.
clang-tools-extra/test/clang-tidy/checkers/bugprone/alpha-core-identicalexpr.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/bugprone/alpha-core-identicalexpr.cpp
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
Outdated
Show resolved
Hide resolved
| ^^^^^^^^^^^^^^ | ||
|
|
||
| - The checker ``alpha.core.IdenticalExpr`` was removed because it was | ||
| duplicated in the clang-tidy checkers ``misc-redundant-expression`` and |
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
2 times, most recently
from
6bcb40f to
48389a3
Compare
|
Thanks @5chmidti for the constructive comments. I believe those have been addressed. I'll follow up after this PR is approved and merged to split the LIT appropriately. Thanks for understanding this approach, it was the only one I could think of where the reviewers could transparently observe all features were maintained from the older checker. I'll happily make any other suggested improvements if I've missed something. Best |
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
48389a3 to
699d4b8
Compare
HerrCai0907
reviewed
Nov 16, 2024
| this); | ||
| Finder->addMatcher(switchStmt().bind("switch"), this); | ||
| Finder->addMatcher(conditionalOperator().bind("condOp"), this); | ||
| Finder->addMatcher(ifStmt(hasDescendant(ifStmt())).bind("ifWithDescendantIf"), |
There was a problem hiding this comment.
hasDescendant will match lots of unrelated cases which will slow the speed. move the later check to here
There was a problem hiding this comment.
Hello @HerrCai0907 , I do not fully understand your suggestion. Could you describe what you mean by "move the later check to here"? Thanks for taking the time to review. Best!
There was a problem hiding this comment.
The current matcher will check for ifs as descendants, not only as direct children. So code like
void foo() {
if (true) {
[]() {
if (false) {
}
};
}
}will still produce a match, that you filter out later, instead of in the matcher.
A matcher like ifStmt(anyOf(has(ifStmt().bind("inner_if")), has(compoundStmt(has(ifStmt().bind("inner_if"))))).bind("ifWithDescendantIf") will ensure that only direct children are looked at. This means that fewer matches are made, which are added to the match result. To enforce the requirement of the first statement inside a compound statement, adding a custom matcher would be the best IMO.
There was a problem hiding this comment.
Thanks @5chmidti. I think I had resolved the comment from @HerrCai0907 by narrowing the match. Would you agree that's sufficient, or would you like to see an improvement in this area before moving forward here? Thanks
HerrCai0907
reviewed
Nov 17, 2024
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
699d4b8 to
6bcbe9f
Compare
|
Thanks @HerrCai0907 and @5chmidti for the constructive comments. I believe all have been addressed and a way forward has been described. Please let me know how we can move this forward. Thank you! |
|
Ping! Any more comments on this change? Thanks |
HerrCai0907
approved these changes
Nov 25, 2024
This change removes the alpha.core.IdenticalExpr static analysis checker since it's checks are present in the clang-tidy checks misc-redundant-expression and bugprone-branch-clone. This check was implemented as a static analysis check using AST matching, and since alpha and duplicated in 2 clang-tidy checks may be removed. The existing LIT test was checked case by case, and the tidy checks were improved to maintain alpha.core.IdenticalExpr features.
vabridgers
force-pushed
the
remove-alpha-core-identicalexpr
branch
from
6bcbe9f to
8388930
Compare
|
Hi @HerrCai0907 and @5chmidti , I see that @HerrCai0907 approved the review. Is it ok if I merge the change? Just wanted to make sure I'm not missing any comments specifically from @5chmidti . Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change removes the alpha.core.IdenticalExpr static analysis checker since it's checks are present in the clang-tidy checks misc-redundant-expression and bugprone-branch-clone. This check was implemented as a static analysis check using AST matching, and since alpha and duplicated in 2 clang-tidy checks may be removed.