[cfe-commits] [PATCH] Warning for suspicious implicit conversions from floating point to bool

Andreas Eckleder aeckleder at google.com
Thu Aug 16 08:23:46 PDT 2012


Hello,

A while ago, I posted a patch introducing a warning for suspicious
implicit conversions from floating point to bool. Since then, I've run
quite an extensive analysis of true and false positives this warning
produces when applied to Google's code base.

While the initial warning produced a low 4 digits number of hits with
only about 10%-20% of them being sufficiently "suspicious" to merit
further investigation, the new patch limits the warning two two
specific cases with a 100% true positive rate:

1) Unintended swapping of function call arguments

This catches situations where e.g. as a result of a method signature
change parameters are specified in the wrong order, e.g.

void foo(bool b, float f);
foo(1.7, false);

The patch will yield a warning if there is an implicit floating point
to bool conversion for a function call argument with another implicit
conversion for one of the neighboring arguments.

This produced a low 2-digits number of hits in our code base, all of
which are true positives.

2) Misplaced brackets around function call arguments

This catches situations where brackets are placed in a complex
expression, the typical example of which I found to be something like
bool InRange = fabs( a - b < delta );

To catch these instances, the patch will produce a warning for an
implicit floating point to bool conversion of a function call result
if the last function call argument happens to be converted implicitly
from bool to float.

This produced a high 2-digits number of hits in out code base, all of
which are true positives.

I'm sure the patch does not yet catch all problematic instances of
floating point to bool conversions, so more cases could be added to
this in the future. Also, it might be interesting to evaluate whether
the two patterns above can also be observed for other implicit type
conversions, although I expect the floating point to bool case to be
the most frequent one, especially for case 2).

As before, the 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

float foof(float x) {
  return x;
}

double food(double x) {
  return x;
}

void foo(bool b, float f) { }

void bar() {

  float c = 1.7;
  bool b = c; // no warning

  double e = 1.7;
  b = e; // no warning

  b = foof(4.0); // no warning

  b = foof(c<1); // expected-warning {{implicit conversion turns
floating-point number into bool: 'float' to 'bool'}}

  b = food(e<2); // expected-warning {{implicit conversion turns
floating-point number into bool: 'double' to 'bool'}}

  foo(c, b);    // expected-warning {{implicit conversion turns
floating-point number into bool: 'float' to 'bool'}}
  foo(c, c); // no warning

}

Please, review this patch.
Thanks!

Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: warn-implicit-conversion-floating-point-to-bool.patch
Type: application/octet-stream
Size: 6378 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120816/3218c8cf/attachment.obj>


More information about the cfe-commits mailing list