[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3

2017-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh added a comment.

In https://reviews.llvm.org/D29858#707897, @watsond wrote:

> Following up. Was this checked in? Do I need to do anything further?


Committed the patch now. Thanks for the reminder!


Repository:
  rL LLVM

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

2017-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL298607: [clang-tidy] Catch trivially true statements like a 
!= 1 || a != 3 (authored by alexfh).

Changed prior to commit:
  https://reviews.llvm.org/D29858?vs=90087=92802#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29858

Files:
  clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp


Index: clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
+++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
+++ clang-tools-extra/trunk/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;
___
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

2017-03-22 Thread Blaise Watson via Phabricator via cfe-commits 
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

2017-03-02 Thread Blaise Watson via Phabricator via cfe-commits 
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

2017-03-02 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh added a comment.

Do you need someone to commit the patch for you?


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

2017-03-02 Thread Etienne Bergeron via Phabricator via cfe-commits 
etienneb accepted this revision.
etienneb added a comment.

thx for the improvement
lgtm


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

2017-03-02 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG unless Etienne has any concerns.


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

2017-02-28 Thread Blaise Watson via Phabricator via cfe-commits 
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

2017-02-28 Thread Blaise Watson via Phabricator via cfe-commits 
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

2017-02-14 Thread Etienne Bergeron via Phabricator via cfe-commits 
etienneb added inline comments.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:244
+  // x != 5 || x != 10
+  if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE)
+return true;

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)
```


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

2017-02-13 Thread Etienne Bergeron via Phabricator via cfe-commits 
etienneb added a comment.

Could you add some tests with enums (like the one in your description)?
This is missing and it's a nice to have.




Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:244
+  // x != 5 || x != 10
+  if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE)
+return true;

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) 
```





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

2017-02-10 Thread Blaise Watson via Phabricator via cfe-commits 
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