<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: arial,helvetica,sans-serif; font-size: 10pt; color: #000000'><br><br><hr id="zwchr"><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><b>From: </b>"Richard Trieu" <rtrieu@google.com><br><b>To: </b>"Hal Finkel" <hfinkel@anl.gov><br><b>Cc: </b>"cfe-dev" <cfe-dev@lists.llvm.org><br><b>Sent: </b>Friday, April 22, 2016 6:04:52 PM<br><b>Subject: </b>Re: [cfe-dev] Which floating point to bool conversions should trigger warnings?<br><br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 22, 2016 at 3:57 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><span class="gmail-"><hr id="zwchr"><br>
> From: "Richard Trieu via cfe-dev" <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>><br>
> To: "cfe-dev" <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>><br>
> Sent: Friday, April 22, 2016 5:47:59 PM<br>
> Subject: [cfe-dev] Which floating point to bool conversions should trigger warnings?<br>
><br>
> I recently added (r267054) and then revert (r267234) some warnings<br>
> for floating point to bool conversions pending more discussion on<br>
> the topic. From my experience, bool conversions are tricky and a<br>
> source of many bugs, so a warning in this place would be good.<br>
><br>
><br>
> Bool behaves a little differently than other integer types. For<br>
> instance, 0.5 is converted to zero for integer types, except for<br>
> bool which it gets converted to true. Also, bool is the type for<br>
> conditionals and resulting type of logical operators.<br>
><br>
><br>
> However, at least one style guide says to use floating point to bool<br>
> conversion (<br>
> <a href="https://webkit.org/code-style-guidelines/#null-false-and-zero" rel="noreferrer" target="_blank">https://webkit.org/code-style-guidelines/#null-false-and-zero</a> )<br>
><br>
<br>
</span>I don't see where it says that.<br>
<br>
Regardless, I think we should always warn.<br>
<br>
-Hal<br></blockquote><div><br></div><div>It specifies that checks against 0 should be written without the == 0. For example:</div><div><br></div><div> void check(float f) {</div><div> if (f) { } // This is the same as "f != 0.0"</div><div> if (!f) { } // This is the same as "f == 0.0"<br></div><div id="DWT3921"> }</div></div></div></div></blockquote>Checks against 0 are different from checks against 0.0. Your example does not appear in the webkit document, and I hope it never does. ;)<br><br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div id="DWT3923" class="gmail_quote"><div></div><div><br></div><div id="DWT3925">I don't intend to stop warning, just split the warning into pieces so users can better control which warnings are active.</div></div></div></div></blockquote>I've seen code that uses float-to-bool conversions, for example, in cases where a parameter is never really zero unless some entire feature is disabled. Thus, they test the feature's enabled state with a float-to-bool conversion on the parameter value. Perhaps, to support a subset of this use case, one might want to warn on all cases except for exactly 0.0. How much value this adds, however, is unclear because in many cases these are runtime checks.<br><br>In any case, unless you have some particular use case for subsetting the feature, I recommend warning everywhere until a concrete use case is presented for doing otherwise.<br><br> -Hal<br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div id="DWT3923" class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div><div id="DWT3922" class="gmail-h5"><br>
><br>
> With that, I hope to get a little more discussion on this topic<br>
> before implementing the floating point to bool conversion warnings<br>
> again. The two factors for the warning are the source of the<br>
> floating point value and where the conversion happens, although<br>
> there may be other factors we need to take into account. Only the<br>
> source has been taken account so far since it is the easier way to<br>
> implement.<br>
><br>
><br>
> Source:<br>
><br>
> Exact floating point literals (0.0, 1.0)<br>
><br>
> Rounded floating point literals (0.5, 4.0)<br>
> Exact compiler time constant (5.0 - 4.0, kZero)<br>
> Inexact compiler time constant (1.0 / 2.0, kHugeNumber)<br>
> Run time values (getFloat())<br>
><br>
><br>
> Location:<br>
> Function Argument<br>
> Assigning/Initializing bool variable<br>
> Return statement of bool returning function<br>
> Condition of if statement, for loop, while loop, or do-while loop<br>
> Condition of conditional operator (?:)<br>
> Operand of logical operators(&&, ||, !)<br>
><br>
><br>
> Currently, exact floating point literals are not warned on, rounded<br>
> floating point literals are under -Wliteral-conversion, and<br>
> everything else is under -Wfloat-conversion.<br>
><br>
><br>
> Note that exclusionary groups may also be useful, for instance<br>
> "-Wfloat-conversion -Wno-some-bool-warning" if that helps filter out<br>
> some of the more noisy warnings.<br>
</div></div>> _______________________________________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
><br>
<span class="gmail-HOEnZb"><font color="#888888"><br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div></div>
</blockquote><br><br><br>-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></div></body></html>