[cfe-commits] [PATCH] Introduces a new warning for implicit conversions from floating point to bool

[Andreas Eckleder]

Hi Nico,

Thanks for the quick reply. A very rough estimation based on Google's
code base suggests a false positive rate of about 40% among the
analyzed cases.
An example of a bug would be Methods with both a bool and a float as
arguments and the arguments being swapped accidentally at the call
site.
There are some cases where it is quite difficult to say if it is a
bug, e.g. when somebody does "if (float_value) { }". Even if he means
to say if (float_value!=0) {}, I'd argue this is probably still wrong,
unless float_value is guaranteed to be initialized/set from a constant
vs. produced by floating point arithmetic in the equals 0.0f case.

I'm planning to run a more thorough analysis in an attempt to classify
the various cases.

Andreas

On Mon, Jun 11, 2012 at 5:18 PM, Nico Weber <thakis at chromium.org> wrote:
> Hi Andreas,
>
> Thanks for your patch.
>
> Have you evaluated this warning on a large-ish codebase? How many bugs
> did it find, how many false positives did it have?
>
> As far as I understand, the philosophy for warnings is that they
> should be good enough to be on by default (and be on by default), or
> they shouldn't be in the tree.
>
> Nico
>
> On Mon, Jun 11, 2012 at 7:22 AM, Andreas Eckleder <aeckleder at google.com> wrote:
>> Hello,
>>
>> I've just started working on clang and the following is hopefully
>> going to be my first contribution to the project.
>> The patch adds a diagnostic to detect implicit conversion from
>> floating point to bool and output a warning.
>>
>> This diagnostic is can be turned on using
>> -Wimplicit-conversion-floating-point-to-bool (off by-default).
>> The patch contains a test case
>> (test/SemaCXX/warn-implicit-conversion-floating-point-to-bool.cpp).
>>
>>
>> Example:
>> compile this code with -Wimplicit-conversion-floating-point-to-bool
>>
>> void bar() {
>>
>>  float f = 1.7;
>>  bool b = f;   // expected-warning {{implicit conversion turns
>> floating-point number into bool: 'float' to 'bool'}}
>>
>>  double d = 1.7;
>>  b = d;        // expected-warning {{implicit conversion turns
>> floating-point number into bool: 'double' to 'bool'}}
>>
>> }
>>
>> Compiler will emit a warning as follows:
>>
>> warn-implicit-conversion-floating-point-to-bool.cpp:6:12: warning:
>> implicit conversion turns floating-point number into
>>      bool: 'float' to 'bool' [-Wimplicit-conversion-floating-point-to-bool]
>>  bool b = f;   // expected-warning {{implicit conversion turns
>> floating-point number into bool: 'float' to 'bool'}}
>>       ~   ^
>> warn-implicit-conversion-floating-point-to-bool.cpp:9:7: warning:
>> implicit conversion turns floating-point number into bool:
>>      'double' to 'bool' [-Wimplicit-conversion-floating-point-to-bool]
>>  b = d;        // expected-warning {{implicit conversion turns
>> floating-point number into bool: 'double' to 'bool'}}
>>    ~ ^
>> 2 warnings generated.
>>
>> Please, review this patch.
>> Thanks!
>>
>> Andreas
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>