[PATCH] PR 17896: clearer warning for &/&& glitches

[PATCH] PR 17896: clearer warning for &/&& glitches

[Thomas R. Truscott]

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 the more typical:

    a>0 & b>0

Here it does not matter whether & or && are used, the semantics are identical.
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 obscure so let's ignore it.)

This patch checks for boolean-valued side-effect-free operands.
It uses COMPARISON_CLASS_P (t) as an approximation to "boolean-valued".
truth_value_p (TREE_CODE (t)) would be better but is unavailable.


2004-10-08 Tom Truscott <trt@acm.org>

	PR 17896
	* c-typeck.c (parser_build_binary_op): Alternative warning text.
	* testsuite/gcc.dg/Wparentheses-11.c: New test.

Index: gcc/gcc/c-typeck.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/c-typeck.c,v
retrieving revision 1.386
diff -c -3 -p -r1.386 c-typeck.c
*** gcc/gcc/c-typeck.c	6 Oct 2004 22:16:26 -0000	1.386
--- gcc/gcc/c-typeck.c	8 Oct 2004 17:07:09 -0000
*************** parser_build_binary_op (enum tree_code c
*** 2268,2274 ****
  	  /* Check cases like x|y==z */
  	  if (TREE_CODE_CLASS (code1) == tcc_comparison
  	      || TREE_CODE_CLASS (code2) == tcc_comparison)
! 	    warning ("suggest parentheses around comparison in operand of |");
  	}
  
        if (code == BIT_XOR_EXPR)
--- 2268,2282 ----
  	  /* Check cases like x|y==z */
  	  if (TREE_CODE_CLASS (code1) == tcc_comparison
  	      || TREE_CODE_CLASS (code2) == tcc_comparison)
! 	    {
! 	      if (COMPARISON_CLASS_P (arg1.value)
! 		  && !TREE_SIDE_EFFECTS (arg1.value)
! 		  && COMPARISON_CLASS_P (arg2.value)
! 		  && !TREE_SIDE_EFFECTS (arg2.value))
! 		warning ("suggest || instead of | when joining booleans");
! 	      else
! 		warning ("suggest parentheses around comparison in operand of |");
! 	    }
  	}
  
        if (code == BIT_XOR_EXPR)
*************** parser_build_binary_op (enum tree_code c
*** 2292,2298 ****
  	  /* Check cases like x&y==z */
  	  if (TREE_CODE_CLASS (code1) == tcc_comparison
  	      || TREE_CODE_CLASS (code2) == tcc_comparison)
! 	    warning ("suggest parentheses around comparison in operand of &");
  	}
        /* Similarly, check for cases like 1<=i<=10 that are probably errors.  */
        if (TREE_CODE_CLASS (code) == tcc_comparison
--- 2300,2314 ----
  	  /* Check cases like x&y==z */
  	  if (TREE_CODE_CLASS (code1) == tcc_comparison
  	      || TREE_CODE_CLASS (code2) == tcc_comparison)
! 	    {
! 	      if (COMPARISON_CLASS_P (arg1.value)
! 		  && !TREE_SIDE_EFFECTS (arg1.value)
! 		  && COMPARISON_CLASS_P (arg2.value)
! 		  && !TREE_SIDE_EFFECTS (arg2.value))
! 		warning ("suggest && instead of when joining booleans");
! 	      else
! 		warning ("suggest parentheses around comparison in operand of &");
! 	    }
  	}
        /* Similarly, check for cases like 1<=i<=10 that are probably errors.  */
        if (TREE_CODE_CLASS (code) == tcc_comparison


Index: gcc/gcc/testsuite/gcc.dg/Wparentheses-11.c
===================================================================
diff -c3p /dev/null gcc/gcc/testsuite/gcc.dg/Wparentheses-11.c
*** /dev/null	Fri Aug 30 19:31:37 2002
--- gcc/gcc/testsuite/gcc.dg/Wparentheses-11.c	Fri Oct  8 12:49:08 2004
***************
*** 0 ****
--- 1,24 ----
+ /* Test operation of -Wparentheses.  booleans joined by & or | */
+ 
+ /* { dg-do compile } */
+ /* { dg-options "-Wparentheses" } */
+ 
+ int foo (int);
+ 
+ int
+ bar (int a, int b, int c)
+ {
+   foo (a == b & b == c);     /* { dg-warning "boolean" "correct warning" } */
+   foo (a == b & (b == c));   /* { dg-warning "boolean" "correct warning" } */
+   foo ((a == b) & b == c);   /* { dg-warning "boolean" "correct warning" } */
+   foo (++a == b & b == c);   /* { dg-warning "comparison" "correct warning" } */
+   foo ((a == b) & (b == c));
+ 
+   foo (a == b | b == c);     /* { dg-warning "boolean" "correct warning" } */
+   foo (a == b | (b == c));   /* { dg-warning "boolean" "correct warning" } */
+   foo ((a == b) | b == c);   /* { dg-warning "boolean" "correct warning" } */
+   foo (++a == b | b == c);   /* { dg-warning "comparison" "correct warning" } */
+   foo ((a == b) | (b == c));
+ 
+   return 0;
+ }

Re: [PATCH] PR 17896: clearer warning for &/&& glitches

[Andreas Schwab]

"Thomas R. Truscott" <trt@cs.duke.edu> writes:

> --- 2300,2314 ----
>   	  /* Check cases like x&y==z */
>   	  if (TREE_CODE_CLASS (code1) == tcc_comparison
>   	      || TREE_CODE_CLASS (code2) == tcc_comparison)
> ! 	    {
> ! 	      if (COMPARISON_CLASS_P (arg1.value)
> ! 		  && !TREE_SIDE_EFFECTS (arg1.value)
> ! 		  && COMPARISON_CLASS_P (arg2.value)
> ! 		  && !TREE_SIDE_EFFECTS (arg2.value))
> ! 		warning ("suggest && instead of when joining booleans");
                                              ^^^
Typo.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."