On 21/12/2011 10:22, Miles Bader wrote:
2011/12/21 David Brown<david@xxxxxxxxxxxxxxx>:
think I disagree a little on what you describe as false positives, and how
much of a problem they are. Basically, I believe that if you can't be sure
that the code is consistent and correct in all circumstances, including
different optimisation levels, then it's bad code - and it is fair to give a
warning. So I would want a warning on your "zot" code regardless of how it
is implemented - to my mind, that's not a false positive even if zot() were
a macro expanding to "0.f". It's bad code style to rely on it, so give a
warning.
You seemed to have misunderstood what I'm trying to illustrate by the
two versions of "zot."
They are examples of two _fundamentally different_ operations. If zot
returns a constant 0.f in certain cases, obviously it needs to make
that fact a documented part of its interface ("zot returns an exact
value 0.f when XXX happens"). The "return ...sin..." case, on the
other hand is an arbitrary calculation, for which interface of zot
would not make such guarantees.
Given the first version of zot, then, any subsequent comparison with
0.f, then, is _not_ relying on implementation details, it's relying on
a documented guarantee. That's certainly not "bad coding style."
However, the compiler has no way of knowing about this guarantee, so
any warning code needs to be conservative, and only warn when it's
quite clear that something dodgy is happening.
I think our key difference of opinion here is that you want a warning
only when it is quite clear that "something dodgy" is happening - I want
the warning unless it is quite clear that something dodgy is /not/
happening. I think it is bad coding style to write something that is
not clearly correct - "might be right, might be dodgy" is, to me, simply
"dodgy" and I want the warning.
So I'm pretty happy with the -Wfloat-equal flag - as I see it, there are
no false positives. One improvement might be to override it with double
parenthesis, so that "if ((x == 0.1f))" would not give a warning (in the
same way as for "if ((x = nextChar()))" avoids the "-Wparentheses" warning.
mvh.,
David
