Bug 17896 - The expression (a>0 & b>0) should give clearer warning message (-Wparentheses)
Summary: The expression (a>0 & b>0) should give clearer warning message (-Wparentheses)
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, easyhack, patch
Depends on:
Blocks:
 
Reported: 2004-10-08 19:55 UTC by Tom Truscott
Modified: 2016-01-25 23:59 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-12-18 00:42:17


Attachments
proposed patch (938 bytes, patch)
2004-10-08 20:00 UTC, Tom Truscott
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Truscott 2004-10-08 19:55:51 UTC
gcc -Wparentheses, for the expression:
 
    a & b == c
 
warns "suggest parentheses around comparison in operand of &".
This is good, but alas the same warning is issued for:
 
    a>0 & b>0
 
Here it does not matter whether & or && are used, the semantics are identical.
(In my experience, 75% of such instances are "harmless" in this way.)
This deserves a clearer warning such as:
 
    suggest && instead of & when joining booleans
 
There is a similar issue with || versus |
And also != versus ^ but that is fairly obscure so I will ignore it.
Comment 1 Tom Truscott 2004-10-08 20:00:04 UTC
Created attachment 7310 [details]
proposed patch

I've attached a proposed patch for gcc to issue alternate warnings:
 
    suggest && instead of & when joining booleans
    suggest || instead of | when joining booleans
 
The patched gcc continues to warn (or not) as before, but with different text
when the operands are boolean-valued and without side effects.
I used COMPARISON_CLASS_P (t) as an approximation to "boolean-valued".
truth_value_p (TREE_CODE (t)) would be better but is unavailable.
Comment 2 Andrew Pinski 2004-10-08 20:11:19 UTC
As I have mentioned before patches goto gcc-patches@gcc.gnu.org.
Comment 3 Tom Truscott 2004-10-09 21:24:34 UTC
Sorry, will do.  Should bugzilla do this automatically for 'patch' attachments?
Comment 4 Andrew Pinski 2004-10-11 17:12:24 UTC
Patch here: <http://gcc.gnu.org/ml/gcc-patches/2004-10/msg00918.html>.
Confirmed.
Comment 5 Manuel López-Ibáñez 2012-04-26 22:23:59 UTC
manuel@gcc12:~$ clang -Wextra -Wall -Weverything -fsyntax-only pr17896.c
pr17896.c:4:1: warning: no previous prototype for function 'bar' [-Wmissing-prototypes]
bar (int a, int b, int c)
^
pr17896.c:7:17: warning: & has lower precedence than ==; == will be evaluated first [-Wparentheses]
    foo (a == b & (b == c));   /* { dg-warning "boolean" "correct warning" } */
         ~~~~~~~^
pr17896.c:7:17: note: place parentheses around the == expression to silence this warning
    foo (a == b & (b == c));   /* { dg-warning "boolean" "correct warning" } */
                ^
                  (       )
pr17896.c:7:17: note: place parentheses around the & expression to evaluate it first
    foo (a == b & (b == c));   /* { dg-warning "boolean" "correct warning" } */
                ^
              (           )
pr17896.c:8:19: warning: & has lower precedence than ==; == will be evaluated first [-Wparentheses]
    foo ((a == b) & b == c);   /* { dg-warning "boolean" "correct warning" } */
                  ^~~~~~~~
pr17896.c:8:19: note: place parentheses around the == expression to silence this warning
    foo ((a == b) & b == c);   /* { dg-warning "boolean" "correct warning" } */
                  ^
                    (     )
pr17896.c:8:19: note: place parentheses around the & expression to evaluate it first
    foo ((a == b) & b == c);   /* { dg-warning "boolean" "correct warning" } */
                  ^
         (           )
pr17896.c:13:16: warning: | has lower precedence than ==; == will be evaluated first [-Wparentheses]
   foo (a == b | (b == c));   /* { dg-warning "boolean" "correct warning" } */
        ~~~~~~~^
pr17896.c:13:16: note: place parentheses around the == expression to silence this warning
   foo (a == b | (b == c));   /* { dg-warning "boolean" "correct warning" } */
               ^
                 (       )
pr17896.c:13:16: note: place parentheses around the | expression to evaluate it first
   foo (a == b | (b == c));   /* { dg-warning "boolean" "correct warning" } */
               ^
             (           )
pr17896.c:14:18: warning: | has lower precedence than ==; == will be evaluated first [-Wparentheses]
   foo ((a == b) | b == c);   /* { dg-warning "boolean" "correct warning" } */
                 ^~~~~~~~
pr17896.c:14:18: note: place parentheses around the == expression to silence this warning
   foo ((a == b) | b == c);   /* { dg-warning "boolean" "correct warning" } */
                 ^
                   (     )
pr17896.c:14:18: note: place parentheses around the | expression to evaluate it first
   foo ((a == b) | b == c);   /* { dg-warning "boolean" "correct warning" } */
                 ^
        (           )
5 warnings generated.


manuel@gcc12:~$ ~/trunk/186353/build/gcc/cc1 -Wextra -Wall -Wlogical-op -fsyntax-only pr17896.c 
 bar
pr17896.c: In function ‘bar’:
pr17896.c:6:5: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
     foo (a == b & b == c);     /* { dg-warning "boolean" "correct warning" } */
     ^
pr17896.c:7:5: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
     foo (a == b & (b == c));   /* { dg-warning "boolean" "correct warning" } */
     ^
pr17896.c:8:5: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
     foo ((a == b) & b == c);   /* { dg-warning "boolean" "correct warning" } */
     ^
pr17896.c:9:5: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
     foo (++a == b & b == c);   /* { dg-warning "comparison" "correct warning" } */
     ^
pr17896.c:12:5: warning: suggest parentheses around comparison in operand of ‘|’ [-Wparentheses]
     foo (a == b | b == c);     /* { dg-warning "boolean" "correct warning" } */
     ^
pr17896.c:13:4: warning: suggest parentheses around comparison in operand of ‘|’ [-Wparentheses]
    foo (a == b | (b == c));   /* { dg-warning "boolean" "correct warning" } */
    ^
pr17896.c:14:4: warning: suggest parentheses around comparison in operand of ‘|’ [-Wparentheses]
    foo ((a == b) | b == c);   /* { dg-warning "boolean" "correct warning" } */
    ^
pr17896.c:15:4: warning: suggest parentheses around comparison in operand of ‘|’ [-Wparentheses]
    foo (++a == b | b == c);   /* { dg-warning "comparison" "correct warning" } */
    ^
Comment 6 Manuel López-Ibáñez 2016-01-25 23:59:13 UTC
FWIW, I don't agree with the suggestion in the proposed patch because: 1) it is not clear why this is suggested, 2) "joining" has no meaning in this context, 3) it does not warn for 'booleans' but only for 'boolean expressions' (more precisely, comparisons), 4) given the suggested text, it will be puzzling why adding parens silences the warning.

I think we should either follow Clang here or we should say something like "&
will evaluate the boolean expression to its right, did you mean '&&'?"
The C/C++ maintainers may even have different opinions.

Also, the patch not warning for expressions with side effects seems to be the opposite of what we want:

a>0 & b>0 vs. a>0 && b>0 is harmless.

f()>0 & f()>0 vs. f()>0 && f()>0 is most probably a bug.