[cfe-commits] [Patch] Add new warning group and warnings for questionable boolean compares

[Eli Friedman]

On Mon, Oct 29, 2012 at 1:48 PM, Richard Trieu <rtrieu at google.com> wrote:
> On Mon, Oct 29, 2012 at 1:43 PM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
>>
>> On Mon, Oct 29, 2012 at 1:28 PM, Richard Trieu <rtrieu at google.com> wrote:
>> > On Fri, Oct 26, 2012 at 5:37 PM, Eli Friedman <eli.friedman at gmail.com>
>> > wrote:
>> >>
>> >> On Fri, Oct 26, 2012 at 5:20 PM, Richard Trieu <rtrieu at google.com>
>> >> wrote:
>> >> > -Wbool-compare-tautological
>> >> > Comparisons involving a boolean and an expression evaluating to 1, 0,
>> >> > true,
>> >> > or false such that the comparison always evaluate to true or false.
>> >> > Also added to -Wtautological-compare group
>> >> > High true positive rate.
>> >> > Not previously caught by -Wtautological-compare
>> >>
>> >> Please just fix DiagnoseOutOfRangeComparison to do the right thing.
>> >>
>> >> > -Wbool-compare-tautological-out-of-range
>> >> > Comparisons of a boolean and >1 or negative values.
>> >> > Also added to -Wtautological-constant-out-of-range-compare group
>> >> > High true positive rate.
>> >> > Previously, only compares with values >1 were caught.
>> >> > (x == 5)  currently caught
>> >> > (x == -1)  not currently caught
>> >>
>> >> Same.
>> >>
>> >> -Eli
>> >
>> >
>> > What do you think of the idea of separating out warnings on bool
>> > comparisons
>> > into a sub group?
>>
>> I'm not exactly opposed... but why?  The tautological compare warning
>> is on by default anyway.
>
> I think that finer control over warnings is a good thing.  And that this
> warning would fit in nicely if a new -Wbool-compare is created.

We shouldn't add new warning groups just for the sake of having more
warning groups.  I think the value argument here is weak, but maybe
I'm missing something.

-Eli