Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/20/16 16:51, Jason Merrill wrote:
> On Tue, Sep 20, 2016 at 10:11 AM, Bernd Edlinger
> <bernd.edlin...@hotmail.de> wrote:
>> On 09/20/16 13:29, Kyrill Tkachov wrote:
>>>
>>> arm bootstrap is now failing:
>>> $SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
>>> boolean context [-Werror=int-in-bool-context]
>>>       : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>>                              ~~~~~~~~~~~~~^~~~~~~~~~
>>> $SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
>>> 'TARGET_ARM_FP'
>>>      if (TARGET_ARM_FP)
>>>
>>>
>>> The full definition of TARGET_ARM_FP is:
>>> #define TARGET_ARM_FP            \
>>>     (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4        \
>>>               : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>>                 : 0)
>>>
>>> We want it set to 0 when there's no FP but when FP is available we set
>>> it to a bitmask
>>> to suggest the level that is available. That seems like a legitimate use
>>> to me.
>>>
>>
>> Ok, I see, sorry for that.
>>
>> I think I will have to suppress the warning if the conditional is in
>> a macro somehow.
>
> from_macro_expansion_at will help with that.
>
> Though it seems to me that the issue here is more that (TARGET_FP16 ?
> 14 : 12) is not in a boolean context, it's in an integer context; only
> the outer ?: is in a boolean context.
>
> I also still think the warning message should be changed.
>

I try this:

Index: c-common.c
===================================================================
--- c-common.c  (revision 240268)
+++ c-common.c  (working copy)
@@ -4652,7 +4652,8 @@
                                               TREE_OPERAND (expr, 0));

      case COND_EXPR:
-      if (warn_int_in_bool_context)
+      if (warn_int_in_bool_context
+         && !from_macro_expansion_at (EXPR_LOCATION (expr)))
        {
          tree val1 = fold_for_warn (TREE_OPERAND (expr, 1));
          tree val2 = fold_for_warn (TREE_OPERAND (expr, 2));
@@ -4663,7 +4664,8 @@
              && (!integer_onep (val1)
                  || !integer_onep (val2)))
            warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context,
-                       "?: using integer constants in boolean context");
+                       "?: using integer constants in boolean context, "
+                       "the expression will always evaluate to %<true%>");
        }
        /* Distribute the conversion into the arms of a COND_EXPR.  */
        if (c_dialect_cxx ())


That will fix "if (TARGET_ARM_FP)" which is fine.

But this seems to suppress all such warnings from an assert
macro too.  Like for instance "assert(a?1:2)".

Sure, we would not be interested in a ?: that is part of the assert
macro itself, but the expression that is evaluated by the macro should
be checked, but that is no longer done, because the macro parameter is
now also from the macro expansion.  But it is initially from the
macro invocation point.  Ideas?


Thanks
Bernd.