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).
Do you have an example?
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; }
that's more coding style though?
(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
(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.
> --- 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.)