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

[Jason Merrill <jason@redhat.com>]

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

On Fri, Sep 30, 2016 at 1:07 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> 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 ?

Done thus:

[-- Attachment #2: cond-bool.diff --]
[-- Type: text/plain, Size: 2129 bytes --]

commit 0a124bbb6f0598345c98e3a91f8c69548518d4c3
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Sep 30 17:52:21 2016 -0400

            Delay folding of bool conversion into COND_EXPR.
    
    gcc/c-family/
            * c-common.c (c_common_truthvalue_conversion): Don't distribute
            into COND_EXPR in C++.
    gcc/cp/
            * cp-gimplify.c (cp_fold): Distribute cp_truthvalue_conversion
            into COND_EXPR.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index f7a5d62..dbdb276 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -4694,21 +4694,8 @@ c_common_truthvalue_conversion (location_t location, tree expr)
 	}
       /* Distribute the conversion into the arms of a COND_EXPR.  */
       if (c_dialect_cxx ())
-	{
-	  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);
-	  if (!VOID_TYPE_P (TREE_TYPE (op2)))
-	    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;
-	}
+	/* Avoid premature folding.  */
+	break;
       else
 	{
 	  int w = warn_int_in_bool_context;
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 5aca8f2..4879632 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2253,6 +2253,15 @@ cp_fold (tree x)
       op1 = cp_fold (TREE_OPERAND (x, 1));
       op2 = cp_fold (TREE_OPERAND (x, 2));
 
+      if (TREE_CODE (TREE_TYPE (x)) == BOOLEAN_TYPE)
+	{
+	  warning_sentinel (warn_int_in_bool_context);
+	  if (!VOID_TYPE_P (TREE_TYPE (op1)))
+	    op1 = cp_truthvalue_conversion (op1);
+	  if (!VOID_TYPE_P (TREE_TYPE (op2)))
+	    op2 = cp_truthvalue_conversion (op2);
+	}
+
       if (op0 != TREE_OPERAND (x, 0)
 	  || op1 != TREE_OPERAND (x, 1)
 	  || op2 != TREE_OPERAND (x, 2))