[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3
watsond added a comment. Following up. Was this checked in? Do I need to do anything further? https://reviews.llvm.org/D29858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3
watsond added a comment. In https://reviews.llvm.org/D29858#690700, @alexfh wrote: > Do you need someone to commit the patch for you? I think so. This is my only LLVM patch, so I doubt I have permissions. Thanks in advance! https://reviews.llvm.org/D29858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3
watsond updated this revision to Diff 90087. watsond marked an inline comment as done. watsond added a comment. Fixed logic error, added enum test case https://reviews.llvm.org/D29858 Files: clang-tidy/misc/RedundantExpressionCheck.cpp 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 @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant @@ -422,6 +424,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false if (C == Red && C != Red) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false + if (C != Red || C != Yellow) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always true // Should not match. if (C == Red || C == Yellow) return 1; Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE) +return true; + return false; } Index: test/clang-tidy/misc-redundant-expression.cpp === --- test/clang-tidy/misc-redundant-expression.cpp +++ test/clang-tidy/misc-redundant-expression.cpp @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant @@ -422,6 +424,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false if (C == Red && C != Red) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false + if (C != Red || C != Yellow) return 1; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always true // Should not match. if (C == Red || C == Yellow) return 1; Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE) +return true; + return false; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3
watsond marked 2 inline comments as done. watsond added a comment. In https://reviews.llvm.org/D29858#675055, @etienneb wrote: > Could you add some tests with enums (like the one in your description)? > This is missing and it's a nice to have. Added Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:244 + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) +return true; etienneb wrote: > etienneb wrote: > > The good news is that this code will be catch by this check! > > ``` > > if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) <<-- redundant expression > > ``` > > Should be: > > ``` > > if (OpcodeLHS == BO_NE || OpcodeRHS == BO_NE) > > ``` > > > > > > > btw, it should be: > > ``` > if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE) > ``` Thanks for catching! https://reviews.llvm.org/D29858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3
watsond created this revision. Herald added a subscriber: JDevlieghere. Catch trivially true statements of the form a != 1 || a != 3. Statements like these are likely errors. They are particularly easy to miss when handling enums: enum State { RUNNING, STOPPED, STARTING, ENDING } ... if (state != RUNNING || state != STARTING) ... https://reviews.llvm.org/D29858 Files: clang-tidy/misc/RedundantExpressionCheck.cpp 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 @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) +return true; + return false; } Index: test/clang-tidy/misc-redundant-expression.cpp === --- test/clang-tidy/misc-redundant-expression.cpp +++ test/clang-tidy/misc-redundant-expression.cpp @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) +return true; + return false; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29857: Catch trivially true statements of the form a != 1 || a != 3. Statements likethese are likely errors. They are particularly easy to miss when handling enums:enum State {RUNNING, STOPPE
watsond abandoned this revision. watsond added a comment. Apologies all. I messed up the formatting. Resubmitting in a second. https://reviews.llvm.org/D29857 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29857: Catch trivially true statements of the form a != 1 || a != 3. Statements likethese are likely errors. They are particularly easy to miss when handling enums:enum State {RUNNING, STOPPE
watsond updated this revision to Diff 88072. watsond added a comment. Updating D29857: Catch trivially true statements of the form a != 1 || a != 3. Statements like == these are likely errors. They are particularly easy to miss when handling enums: enum State { RUNNING, STOPPED, STARTING, ENDING } ... if (state != RUNNING || state !=... https://reviews.llvm.org/D29857 Files: clang-tidy/misc/RedundantExpressionCheck.cpp 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 @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) +return true; + return false; } Index: test/clang-tidy/misc-redundant-expression.cpp === --- test/clang-tidy/misc-redundant-expression.cpp +++ test/clang-tidy/misc-redundant-expression.cpp @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) +return true; + return false; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29857: Catch trivially true statements of the form a != 1 || a != 3. Statements likethese are likely errors. They are particularly easy to miss when handling enums:enum State {RUNNING, STOPPE
watsond created this revision. ...ENDING) ... [clang-tidy] Catch trivially true statements like a != 1 || a != 3 https://reviews.llvm.org/D29857 Files: clang-tidy/misc/RedundantExpressionCheck.cpp 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 @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) +return true; + return false; } Index: test/clang-tidy/misc-redundant-expression.cpp === --- test/clang-tidy/misc-redundant-expression.cpp +++ test/clang-tidy/misc-redundant-expression.cpp @@ -321,6 +321,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true if (X <= 10 || X >= 11) return 1; // 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 < 6) return 1; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant Index: clang-tidy/misc/RedundantExpressionCheck.cpp === --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -239,6 +239,11 @@ (OpcodeRHS == BO_LT || OpcodeRHS == BO_LE)) return true; + // Handle cases where constants are different but both ops are !=, like: + // x != 5 || x != 10 + if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE) +return true; + return false; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits