On Sun, Jun 22, 2014 at 10:33:57PM +0200, Gerald Pfeifer wrote:
> On Mon, 2 Jun 2014, Marek Polacek wrote:
> > * c-typeck.c (parser_build_binary_op): Warn when logical not is used
> > on the left hand side operand of a comparison.
>
> This...
>
> > +/* Warn about logical not used on the left hand side operand of a
> > comparison.
>
> ...and this...
>
> > + warning_at (location, OPT_Wlogical_not_parentheses,
> > + "logical not is only applied to the left hand side of "
> > + "comparison");
>
> ...does not appear consistent with the actual warning.
>
> Why does that warning say "is _ONLY_ applied to the left hand side"?
>
> Based on the message, I naively assumed that the code should not warn
> about
>
> int same(int a, int b) {
> return !a == !b;
> }
>
> alas this is not the case. (Code like this occurs in Wine where
> bool types are emulated and !!a or a comparison like above ensure
> that those emulated bools are normalized to either 0 or 1.)
>
>
> I understand there is ambiguity in cases like
>
> return !a == b;
>
> where the warning would be approriately worded and the programmer
> might have intended !(a == b).
>
>
> I do recommend to either omit "only" from the text of the warning
> or not warn for cases where ! occurs on both sides of the comparison
> (and keep the text as is).
I think the latter is better, incidentally, g++ doesn't warn either.
The following one liner makes cc1 behave as cc1plus. Thanks for the
report.
Regtested/bootstrapped on x86_64. Joseph, is this ok?
2014-06-23 Marek Polacek <[email protected]>
* c-typeck.c (parser_build_binary_op): Don't call
warn_logical_not_parentheses if the RHS is TRUTH_NOT_EXPR.
* c-c++-common/pr49706-2.c: New test.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 63bd65e..0764630 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -3402,7 +3402,8 @@ parser_build_binary_op (location_t location, enum
tree_code code,
code1, arg1.value, code2, arg2.value);
if (warn_logical_not_paren
- && code1 == TRUTH_NOT_EXPR)
+ && code1 == TRUTH_NOT_EXPR
+ && code2 != TRUTH_NOT_EXPR)
warn_logical_not_parentheses (location, code, arg1.value, arg2.value);
/* Warn about comparisons against string literals, with the exception
diff --git gcc/testsuite/c-c++-common/pr49706-2.c
gcc/testsuite/c-c++-common/pr49706-2.c
index e69de29..09cc9eb 100644
--- gcc/testsuite/c-c++-common/pr49706-2.c
+++ gcc/testsuite/c-c++-common/pr49706-2.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-not-parentheses" } */
+
+/* Test that we don't warn if both operands of the comparison
+ are negated. */
+
+#ifndef __cplusplus
+#define bool _Bool
+#endif
+
+bool r;
+
+int
+same (int a, int b)
+{
+ r = !a == !b;
+ r = !!a == !!b;
+ r = !!a == !b;
+ r = !a == !!b;
+}
Marek
