<div dir="ltr">I think this would be better implemented in CheckImplicitConversion instead of CheckMultiplyDivideOperands. CheckImplicitConversion has the information on cast from and cast to types, so you can narrow the checking to int->floating conversions.<div>
<br></div><div>I think a better fix-it would be to suggest a static_cast to the appropriate floating type instead of computing a value and adding ".0"</div><div><br></div><div>Fix-its for warnings should be on notes attached to the warning, not on the warning itself.<br>
<div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 8, 2014 at 5:42 PM, Matt Arsenault <span dir="ltr"><<a href="mailto:arsenm2@gmail.com" target="_blank">arsenm2@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word">On Aug 24, 2013, at 1:43 PM, Tobias Grosser <<a href="mailto:tobias@grosser.es" target="_blank">tobias@grosser.es</a>> wrote:<br><div><br><blockquote type="cite"><div style="font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
On 08/24/2013 03:51 AM, Yaron Keren wrote:<br><blockquote type="cite">Hello,<br><br>Attached is a patch I made to make clang warn where division of two integer<br>literals loses precision. For instance:<br><br>double x = 1/2;<br>
<br>Which results in x being 0. This is not the programmer's intention.<br>While experienced users will know to code 1.0/2 to force floating division,<br>new C/C++ programmers will fall in this trap.<br><br>The patch computes the reminder in the division operation and if only if it<br>
is non-zero, warns and suggests a fix.<br><br>This is the first code I write for LLVM so please review it carefully.<br>Specifically, I was not sure whether to create another warning group<br>in DiagnosticSemaKinds.td or reuse an existing one. Maybe the group should<br>
be renamed from "DivZero" to "DivProblems".<br></blockquote><br>Wow. This seems really helpful!<br><br>As Matt suggested, please resubmit this patch to cfe-commits and add a test case.<br><br>+//<br>+ // check for integer constant division by integer constant<br>
<br>Also the comment seems misformatted. Remove the '//', start with an uppercase letter and finish the sentence with a point.<br><br>Also, here is an interesting test case that you may want to consider.<br><br>int foo(float b) {<br>
<span style="white-space:pre-wrap"> </span>float a = 1/2 + b;<br>}<br><br>Suggesting here to transform this into<br><br>int foo(float b) {<br><span style="white-space:pre-wrap"> </span>float a = 1.0/2 + b;<br>}<br><br>may not be what the user actually wants, as this promotes the whole expression to 'double'. Possibly the following is more in line.<br>
<br>int foo(float b) {<br><span style="white-space:pre-wrap"> </span>float a = 1.0f/2 + b;<br>}<br><br>Cheers,<br>Tobias<br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote><br></div><div>This seems to have been forgotten, so I guess I’ll adopt it. This adds a test, fixes test failures and adds a new warning group for it. I haven’t yet been able to figure out how get the parent expression to check if it’s a cast to float to add the f in the warning message.</div>
<div><br></div><div></div></div><br><div style="word-wrap:break-word"><div></div></div><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div></div></div>