Re: [PATCH 3/3] catch !x & y brainos

[Josh Triplett <josh@freedesktop.org>]

[-- Attachment #1: Type: text/plain, Size: 3495 bytes --]

Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied and pushed, along with a new test case.

Per previous discussion on this change, any reason not to emit a
similar warning for one or more of x & !y, !x | y, and x | !y, with
something like the following patch?  The discussion seemed to suggest
that some people might to use some of those variants intentionally,
but that doing so still seemed really questionable, and such code
could easily use a saner, less abbreviated style instead.

Also, I suspect that some people may want a -Wno-somethingorother for
this.  Any suggestions on naming, anyone?  I called the test case
"dubious bitwise with not", but I'd love to hear a better suggestion
that sounds slightly more like reasonable English.

From 85581bf8ab9e2a103390c9dfff669b88f2c5e101 Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@freedesktop.org>
Date: Thu, 3 Apr 2008 14:12:30 -0700
Subject: [PATCH] Expand "dubious !x & y" handling to other combinations of !, &, and |.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
 evaluate.c                            |   13 ++++++++++---
 validation/dubious-bitwise-with-not.c |   19 +++++++++++++++++--
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 4928584..68211d3 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -917,9 +917,16 @@ static struct symbol *evaluate_binop(struct expression *expr)
 			rtype = integer_promotion(rtype);
 		} else {
 			// The rest do usual conversions
-			if (op == '&' && expr->left->type == EXPR_PREOP &&
-			    expr->left->op == '!')
-				warning(expr->pos, "dubious: !x & y");
+			const unsigned left_not  = expr->left->type == EXPR_PREOP
+			                           && expr->left->op == '!';
+			const unsigned right_not = expr->right->type == EXPR_PREOP
+			                           && expr->right->op == '!';
+			if ((op == '&' || op == '|') && (left_not || right_not))
+				warning(expr->pos, "dubious: %sx %c %sy",
+				        left_not ? "!" : "",
+					op,
+					right_not ? "!" : "");
+
 			ltype = usual_conversions(op, expr->left, expr->right,
 						  lclass, rclass, ltype, rtype);
 			ctype = rtype = ltype;
diff --git a/validation/dubious-bitwise-with-not.c b/validation/dubious-bitwise-with-not.c
index e076899..c48bcae 100644
--- a/validation/dubious-bitwise-with-not.c
+++ b/validation/dubious-bitwise-with-not.c
@@ -1,9 +1,24 @@
-static unsigned int ok1 = !1 && 2;
-static unsigned int bad1 = !1 & 2;
+static unsigned int ok1  = !1 &&  2;
+static unsigned int bad1 = !1 &   2;
+static unsigned int ok2  = !1 ||  2;
+static unsigned int bad2 = !1 |   2;
+static unsigned int ok3  =  1 && !2;
+static unsigned int bad3 =  1 &  !2;
+static unsigned int ok4  =  1 || !2;
+static unsigned int bad4 =  1 |  !2;
+static unsigned int ok5  = !1 && !2;
+static unsigned int bad5 = !1 &  !2;
+static unsigned int ok6  = !1 || !2;
+static unsigned int bad6 = !1 |  !2;
 /*
  * check-name: Dubious bitwise operation on !x
  *
  * check-error-start
 dubious-bitwise-with-not.c:2:31: warning: dubious: !x & y
+dubious-bitwise-with-not.c:4:31: warning: dubious: !x | y
+dubious-bitwise-with-not.c:6:31: warning: dubious: x & !y
+dubious-bitwise-with-not.c:8:31: warning: dubious: x | !y
+dubious-bitwise-with-not.c:10:31: warning: dubious: !x & !y
+dubious-bitwise-with-not.c:12:31: warning: dubious: !x | !y
  * check-error-end
  */



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]