On 01/30/2016 06:35 AM, Prasad Ghangal wrote:
Hi!
This is my first proposed patch for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17896. I was willing to
do it using "APPEARS_TO_BE_BOOLEAN_EXPR_P(CODE, ARG)" to check
booleans but gcc doesn't allow (bootstraping fails). Hence I am using
"TREE_CODE_CLASS (CODE) == tcc_comparison"
I would encourage you to look more closely at why GCC failed to
bootstrap. It could be the case that APPEARS_TO_BE_BOOLEAN_EXPR_P is
the wrong test to use, or it could be a bug in GCC itself.
-- Thanks and Regards, Prasad Ghangal
bug17896.patch
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c (revision 232768)
+++ gcc/c-family/c-common.c (working copy)
@@ -11596,6 +11596,11 @@
|| code_right == PLUS_EXPR || code_right == MINUS_EXPR)
warning_at (EXPR_LOC_OR_LOC (arg_right, loc), OPT_Wparentheses,
"suggest parentheses around arithmetic in operand of %<|%>");
+ /* Check cases like (x<y) | (x<z)*/
Space before the close comment.
+ else if(TREE_CODE_CLASS (code_left) == tcc_comparison
+ && (TREE_CODE_CLASS (code_right) == tcc_comparison))
+ warning_at (loc, OPT_Wparentheses,
+ "suggest %<||%> instead of %<|%> when joining booleans");
Space between the "if" and open paren.
Both of those nits are repeated in the second block of code.
I note in the patch inside the BZ that the original author verified
neither argument had side effects. Any reason you dropped that test?
Similarly in that patch there's a test for the testsuite. AFAICT your
patch doesn't even pass that test. For a change like yours we
definitely want to include a reasonable test -- you could start with the
one included in the BZ or construct your own.
As of right now I don't think your patch needs a copyright assignment,
but that might change if the patch grows too much.
Jeff
