[Bug middle-end/101862] New: [C, C++] Potential '?:' diagnostic for always-true expressions in boolean context

[Bug middle-end/101862] New: [C, C++] Potential '?:' diagnostic for always-true expressions in boolean context

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101862

            Bug ID: 101862
           Summary: [C, C++] Potential '?:' diagnostic for always-true
                    expressions in boolean context
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tschwinge at gcc dot gnu.org
  Target Milestone: ---

Created attachment 51287
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51287&action=edit
'c-c++-common/goacc/prN.c'

Would it be possible for GCC to diagnose the "'?:' misuse" in the attached
C/C++ test case?  No matter what gets stored in 'arr', the 'assert' never
triggers, because what's actually meant here, is:

       for (int i = 0; i < 32; i++)
    -    assert (arr[i] == ((i % 2) != 0) ? i + 1 : i + 2);
    +    assert (arr[i] == (((i % 2) != 0) ? i + 1 : i + 2));

This is going to be more complicated than
'gcc/c-family/c-common.c:c_common_truthvalue_conversion', 'case COND_EXPR:',
which diagnoses "'?:' using integer constants in boolean context" for
'INTEGER_CST's -- which these are not, of course.  I suppose we'd need some
value range analysis (so, at some later stage in the pass pipeline?) to see
that 'i + 1'/'i + 2' are always-true expressions in boolean context?

Is this feasible or not?  (..., and if yes, if anybody got any pointers about
where/how to do this...)

[Bug middle-end/101862] [C, C++] Potential '?:' diagnostic for always-true expressions in boolean context

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101862

Andrew Macleod <amacleod at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amacleod at redhat dot com

--- Comment #1 from Andrew Macleod <amacleod at redhat dot com> ---
I'm not sure exactly what you want to do.
When we go into SSA we see:

  <bb 12> :
  _8 = arr[i_15];
  i.1_9 = (unsigned int) i_15;
  _10 = i.1_9 & 1;
  _11 = _10 != 0;
  _12 = (int) _11;
  if (_8 == _12)
    goto <bb 13>; [INV]
  else
    goto <bb 14>; [INV]

  <bb 13> :
  iftmp.0_27 = i_15 != -1;
  goto <bb 15>; [INV]

  <bb 14> :
  iftmp.0_26 = i_15 != -2;

  <bb 15> :
  # iftmp.0_16 = PHI <iftmp.0_27(13), iftmp.0_26(14)>
  if (iftmp.0_16 != 0)
    goto <bb 17>; [INV]
  else
    goto <bb 16>; [INV]

The EVRP pass recognizes that i_15 has a range of int [0, 32], thus folds 
 # iftmp.0_16 = PHI <iftmp.0_27(13), iftmp.0_26(14)>
into
# iftmp.0_16 = PHI <1, 1>
and continues to fold away the if that leads to the assert.

This seems to be too late to determine that a warning might be appropriate..
and I'm not sure how you would figure that out from this IL.

We have to go way back before gimple before we see the conditional expression.
I see the front end generates this is the .original file:
<D.1973>:;
    {
      if (arr[i] == (((unsigned int) i & 1) != 0) ? i != -1 : i != -2)
        {
          (void) 0;
        }
      else
        {
          __assert_fail ((const char *) "arr[i] == ((i % 2) != 0) ? i + 1 : i +
2", (const char *) "as.c", 17, (const char *) &__PRETTY_FUNCTION__);
        }

but even by the .gimple listing the COND_EXPR is gone, replaced with the iftmp
sequence.  Im not sure how you would determine that this is not working as
intended.. ?  we usually rejoice when we can fold asserts away :-)

The range machinery knows what you want to know, but IM not sure how you could
use it. Its only available once we go into SSA.

[Bug middle-end/101862] [C, C++] Potential '?:' diagnostic for always-true expressions in boolean context

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101862

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org

--- Comment #2 from Eric Gallager <egallager at gcc dot gnu.org> ---
Would this be a new flag, or go under the existing -Wint-in-bool-context flag?

[Bug middle-end/101862] [C, C++] Potential '?:' diagnostic for always-true expressions in boolean context

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101862

--- Comment #3 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
Thanks for your comments!


(In reply to Andrew Macleod from comment #1)
> I'm not sure exactly what you want to do.

Neither am I!  ;-) (Well, I do know "what", but not yet "how".)

> The EVRP pass recognizes [...]

> This seems to be too late to determine that a warning might be appropriate..
> and I'm not sure how you would figure that out from this IL.
> 
> We have to go way back before gimple before we see the conditional
> expression.

> The range machinery knows what you want to know, but IM not sure how you
> could use it. Its only available once we go into SSA.

Idea: preserve the original front end AST (or whatever is appropriate), for
"potential diagnostics" ("delayed diagnostics"?), and then (re-)evaluate once
we have VR information (and whatever else is necessary).


> we usually rejoice when we can fold asserts away :-)

A classic case of tuning for code generation vs. diagnostics.  ;-)