[cfe-commits] Support <x>-to-bool warnings for more contexts

[David Blaikie]

On Tue, Apr 3, 2012 at 10:14 AM, John McCall <rjmccall at apple.com> wrote:
> On Apr 3, 2012, at 9:42 AM, Matt Beaumont-Gay wrote:
>> On Mon, Apr 2, 2012 at 18:03, David Blaikie <dblaikie at gmail.com> wrote:
>>> Another thing I noticed as I was exploring this. We have a warning for
>>> float-literal-to-int such as:
>>>
>>> conv.cpp:2:9: warning: implicit conversion turns literal
>>> floating-point number into integer: 'double' to 'int'
>>> [-Wliteral-conversion]
>>> int i = 3.1415;
>>>    ~   ^~~~~~
>>>
>>> But this warning is off-by-default. Why is that?
>>
>> Historical accident? I didn't even remember that it was off by
>> default. I say flip the switch to turn it on.
>>
>>> It's already
>>> relatively conservative (allowing things like : "int i = 3.0" because
>>> 3.0 converts to an int without loss of precision) - though it's not a
>>> DiagnoseRuntimeBehavior, which it could be changed to (to be
>>> consistent with similar things for integers like "unsigned char c =
>>> 256").
>>>
>>> Or is it really that common to deliberately use floating point
>>> literals to initialize integer values?
>>
>> Well, the "int kNumMicrosPerSecond = 1e6" case is very common,

Ah, well that does answer one of my questions - I was a bit surprised
that the loss-of-precision test was actually necessary to remove a
substantial number of false positives.

[I wonder how many bugs we might catch by looking for literals with a
decimal point in them - even when they're not losing precision on the
conversion (eg: void func(int); ... func(1.0);) - but perhaps people
don't tend to put ".0" on things, instead relying on int->float
conversion anyway.]

>> but the
>> warning is suppressed in that case. (Actually, that may be one reason
>> the warning is off by default -- the first implementation didn't have
>> such suppression.)
>
> I believe that's correct.
>
> I don't mind turning this on by default for literals with fractional precision.
> We're a bit close to the release, but we should get acceptable qualification
> in the next week or so.

Apparently we've been using this warning at Google for a while now, so
it's pretty well field tested. I've turned it on by default in
r154068.

- David