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

[Chandler Carruth]

On Mon, Oct 29, 2012 at 2:01 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 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.

We've had to turn off -Wtautological-compare due to the frustratingly
high rate of occurrence, and too high of a false-positive rate for us
to clean up all the code for. However, some of the fine grained pieces
Richard is teasing out here will be significantly easier for us to
roll out to our users, and will help incrementally attack the problem
of the larger warning.