[PATCH, updated] Add a warning for suspicious use of conditional expressions in boolean context

[Jason Merrill]

On Wed, Sep 14, 2016 at 12:10 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> The other false positive was in dwarf2out, where we have this:
>
> ../../gcc-trunk/gcc/dwarf2out.c:26166:35: error: ?: using integer
> constants in boolean context [-Werror=int-in-bool-context]
>    if (s->refcount
>        == ((DEBUG_STR_SECTION_FLAGS & SECTION_MERGE) ? 1 : 2))
>            ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
>
> and:
> #define DEBUG_STR_SECTION_FLAGS                                 \
>    (HAVE_GAS_SHF_MERGE && flag_merge_debug_strings               \
>     ? SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1        \
>     : SECTION_DEBUG)
>
> which got folded in C++ to
>    if (s->refcount ==
>        ((flag_merge_debug_strings ? SECTION_MERGE : 0) ? 1 : 2))

Hmm, I'd think this warning should be looking at unfolded trees.

> Additional to what you suggested, I relaxed the condition even more,
> because I think it is also suspicious, if one of the branches is known
> to be outside [0:1] and the other is unknown.
>
> That caused another warning in the fortran FE, which was caused by
> wrong placement of braces in gfc_simplify_repeat.
>
> We have:
> #define mpz_sgn(Z) ((Z)->size < 0 ? -1 : (Z)->size > 0)
>
> Therefore the condition must be  .. && mpz_sgn(X) != 0)
>
> Previously that did not match, because -1 is outside [0:1]
> but (Z)->size > 0 is unknown.

But (Z)->size > 0 is known to be boolean; that seems like it should
suppress this warning.

> Furthermore the C++ test case df-warn-signedunsigned1.C also
> triggered the warning, because here we have:
>
> #define MAK (fl < 0 ? 1 : (fl ? -1 : 0))
>
> And thus "if (MAK)" should be written as if (MAK != 0)

This also seems like a false positive to me.

It's very common for code to rely on the implicit !=0 in boolean
conversion; it seems to me that the generalization to warn about
expressions with 0 arms significantly increases the frequency of false
positives, making the flag less useful.  The original form of the
warning seems like a -Wall candidate to me, but the current form
doesn't.

Jason