[Bug c++/106774] New: warning about comparison to true/false

[Bug c++/106774] New: warning about comparison to true/false

[f.heckenbach@fh-soft.de]

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

            Bug ID: 106774
           Summary: warning about comparison to true/false
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: f.heckenbach@fh-soft.de
  Target Milestone: ---

gcc has "-Wbool-compare" to warn about boolean expression compared with an
integer value different from true/false. This warning is enabled by -Wall.

However, a warning for comparisons with true and false may also be useful. The
former is redundant, the latter can be replaced with "!".

Actually, this warning may also be given when comparing a non-Boolean integral
expression with true or false. The former is not redundant, but may often not
be what's intended (if so, one should compare with 1). The latter is equivalent
to comparing with 0, but still strange style. And comparing non-integral (e.g.
floating-point) expressions with Boolean literals is certainly warning-worthy
(though it may be caught by "-Wfloat-equal" as well).

To sum up, a new warning about comparing any expression with true/false
literals. 

Maybe something like "-Wbool-compare=2" (which would not be enabled by -Wall, I
guess; maybe by -Wextra, but even if it must be given manually, I'd find it
useful).

[Bug c++/106774] warning about comparison to true/false

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Do you have an example?

[Bug c++/106774] warning about comparison to true/false

[f.heckenbach@fh-soft.de]

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

--- Comment #2 from Frank Heckenbach <f.heckenbach@fh-soft.de> ---
This should cover all cases mentioned:

void t (bool a, int i, float e, double f)
{
  // Boolean literal comparisons
  if (a == true)  // better: if (a)
    return;
  if (a == false)  // better: if (!a)
    return;
  if (a != true)  // better: if (!a)
    return;
  if (a != false)  // better: if (a)
    return;
  // also reversed to be sure
  if (true == a)  // better: if (a)
    return;
  if (false != a)  // better: if (a)
    return;

  // Integer comparisons to Boolean literals
  if (i == true)  // better: if (i == 1)
    return;
  if (i == false)  // better: if (i == 0) or: if (!i)
    return;  
  if (i != true)  // better: if (i != 1)
    return;
  if (i != false)  // better: if (i != 0) or: if (i)
    return;

  // Floating-point comparisons to Boolean literals
  if (e == true)  // very strange at all, if meant so, then better: if (e ==
1.0f)
    return;
  if (f == false)  // very strange at all, if meant so, then better: if (f ==
0.0)
    return;
}

[Bug c++/106774] warning about comparison to true/false

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
that's more coding style though?

[Bug c++/106774] warning about comparison to true/false

[egallager at gcc dot gnu.org]

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

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

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

--- Comment #4 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> that's more coding style though?

Yeah I personally prefer the more explicit way of writing it with both operands
myself

[Bug c++/106774] warning about comparison to true/false

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
             Blocks|                            |87403

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Frank Heckenbach from comment #2)
> This should cover all cases mentioned:
> 
> void t (bool a, int i, float e, double f)
> {
>   // Boolean literal comparisons
>   if (a == true)  // better: if (a)
>     return;
>   if (a == false)  // better: if (!a)
>     return;
>   if (a != true)  // better: if (!a)
>     return;
>   if (a != false)  // better: if (a)
>     return;
>   // also reversed to be sure
>   if (true == a)  // better: if (a)
>     return;
>   if (false != a)  // better: if (a)
>     return;

These are definitely coding style, and I don't think a warning is warranted.

> 
>   // Integer comparisons to Boolean literals
>   if (i == true)  // better: if (i == 1)

Or maybe it was meant to be ((bool)i = true).

Warning for these does seem more useful IMO. The arithmetic promotions from
bool to int (or float) mean the comparison doesn't do what it appears to do,
and there seems no reason to use a true/false literal there.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87403
[Bug 87403] [Meta-bug] Issues that suggest a new warning

[Bug c++/106774] warning about comparison to true/false

[f.heckenbach@fh-soft.de]

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

--- Comment #6 from Frank Heckenbach <f.heckenbach@fh-soft.de> ---
> --- Comment #4 from Eric Gallager <egallager at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #3)
> > that's more coding style though?
> 
> Yeah I personally prefer the more explicit way of writing it with both operands
> myself

It's not really about operands, but the whole operation is
redundant: "mybool == true" is exactly equivalent to just "mybool".

So it's similar to a cast to the original type which GCC can also
warn about (-Wuseless-cast).

Some other existing warnings (-Wredundant-decls, -Wredundant-move)
also seem to warn about things that are no potential problems, just
redundant.

Of course, all of this can be considered a matter of style, that's
why warnings can be selected by options. (As I said, I assume it
would not be enabled by -Wall, and I don't mind whether or not it's
enabled by -Wextra.)