Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

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

On 09/29/16 22:38, Jason Merrill wrote:
> On Thu, Sep 29, 2016 at 3:58 PM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> Unfortunately, without that exception there is a false positive:
>>
>> In file included from ../../gcc-trunk/gcc/ada/gcc-interface/decl.c:30:0:
>> ../../gcc-trunk/gcc/ada/gcc-interface/decl.c: In function 'int
>> adjust_packed(tree, tree, int)':
>> ../../gcc-trunk/gcc/tree.h:1874:22: error: << on signed integer in
>> boolean context [-Werror=int-in-bool-context]
>>         ? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0)
>>           ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Ah, this issue again: the shift isn't in boolean context, it's in
> integer context.  I think we want to be a lot more conservative about
> these warnings in the arms of a COND_EXPR.  In fact, I think the
> entire
>
>        /* Distribute the conversion into the arms of a COND_EXPR.  */
>
> section is wrong now that we're doing delayed folding.
>

Could you take care of this ?


For the warning, I think I can suppress it just while
the recursing into the condition arms.

As in this updated patch.

Is it OK?


Bernd.

[-- Attachment #2: changelog-bool-context.txt --]
[-- Type: text/plain, Size: 514 bytes --]

gcc:
2016-09-29  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* doc/invoke.texi: Update -Wint-in-bool-context.

c-family:
2016-09-29  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-common.c (c_common_truthvalue_conversion): Warn for suspicious
	left shift in boolean context.

cp:
2016-09-29  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* parser.c (cp_parser_condition): Fix a warning.

testsuite:
2016-09-29  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-c++-common/Wint-in-bool-context.c: Update test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-bool-context.diff --]
[-- Type: text/x-patch; name="patch-bool-context.diff", Size: 3448 bytes --]

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 240571)
+++ gcc/c-family/c-common.c	(working copy)
@@ -4655,6 +4655,11 @@ c_common_truthvalue_conversion (location_t locatio
 	return c_common_truthvalue_conversion (location,
 					       TREE_OPERAND (expr, 0));
 
+    case LSHIFT_EXPR:
+      warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context,
+		  "<< in boolean context, did you mean '<' ?");
+      break;
+
     case COND_EXPR:
       if (warn_int_in_bool_context
 	  && !from_macro_definition_at (EXPR_LOCATION (expr)))
@@ -4676,6 +4681,8 @@ c_common_truthvalue_conversion (location_t locatio
 	{
 	  tree op1 = TREE_OPERAND (expr, 1);
 	  tree op2 = TREE_OPERAND (expr, 2);
+	  int w = warn_int_in_bool_context;
+	  warn_int_in_bool_context = 0;
 	  /* In C++ one of the arms might have void type if it is throw.  */
 	  if (!VOID_TYPE_P (TREE_TYPE (op1)))
 	    op1 = c_common_truthvalue_conversion (location, op1);
@@ -4683,10 +4690,13 @@ c_common_truthvalue_conversion (location_t locatio
 	    op2 = c_common_truthvalue_conversion (location, op2);
 	  expr = fold_build3_loc (location, COND_EXPR, truthvalue_type_node,
 				  TREE_OPERAND (expr, 0), op1, op2);
+	  warn_int_in_bool_context = w;
 	  goto ret;
 	}
       else
 	{
+	  int w = warn_int_in_bool_context;
+	  warn_int_in_bool_context = 0;
 	  /* Folding will happen later for C.  */
 	  expr = build3 (COND_EXPR, truthvalue_type_node,
 			 TREE_OPERAND (expr, 0),
@@ -4694,6 +4704,7 @@ c_common_truthvalue_conversion (location_t locatio
 							 TREE_OPERAND (expr, 1)),
 			 c_common_truthvalue_conversion (location,
 							 TREE_OPERAND (expr, 2)));
+	  warn_int_in_bool_context = w;
 	  goto ret;
 	}
 
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 240571)
+++ gcc/cp/parser.c	(working copy)
@@ -11244,7 +11244,7 @@ cp_parser_condition (cp_parser* parser)
 	{
 	  tree pushed_scope;
 	  bool non_constant_p;
-	  bool flags = LOOKUP_ONLYCONVERTING;
+	  int flags = LOOKUP_ONLYCONVERTING;
 
 	  /* Create the declaration.  */
 	  decl = start_decl (declarator, &type_specifiers,
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 240571)
+++ gcc/doc/invoke.texi	(working copy)
@@ -6028,7 +6028,8 @@ of the C++ standard.
 @opindex Wno-int-in-bool-context
 Warn for suspicious use of integer values where boolean values are expected,
 such as conditional expressions (?:) using non-boolean integer constants in
-boolean context, like @code{if (a <= b ? 2 : 3)}.
+boolean context, like @code{if (a <= b ? 2 : 3)}.  Or left shifting in
+boolean context, like @code{for (a = 0; 1 << a; a++);}.
 This warning is enabled by @option{-Wall}.
 
 @item -Wno-int-to-pointer-cast
Index: gcc/testsuite/c-c++-common/Wint-in-bool-context.c
===================================================================
--- gcc/testsuite/c-c++-common/Wint-in-bool-context.c	(revision 240571)
+++ gcc/testsuite/c-c++-common/Wint-in-bool-context.c	(working copy)
@@ -25,5 +25,7 @@ int foo (int a, int b)
   if (b ? 1+1 : 1) /* { dg-warning "boolean context" } */
     return 7;
 
+  for (a = 0; 1 << a; a++); /* { dg-warning "boolean context" } */
+
   return 0;
 }