[PATCH] clang warning when division of two integer literals loses precision

[Richard Trieu]

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.

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"

Fix-its for warnings should be on notes attached to the warning, not on the
warning itself.

On Wed, Jan 8, 2014 at 5:42 PM, Matt Arsenault <arsenm2 at gmail.com> wrote:

> On Aug 24, 2013, at 1:43 PM, Tobias Grosser <tobias at grosser.es> wrote:
>
> On 08/24/2013 03:51 AM, Yaron Keren wrote:
>
> Hello,
>
> Attached is a patch I made to make clang warn where division of two integer
> literals loses precision. For instance:
>
> double x = 1/2;
>
> Which results in x being 0. This is not the programmer's intention.
> While experienced users will know to code 1.0/2 to force floating division,
> new C/C++ programmers will fall in this trap.
>
> The patch computes the reminder in the division operation and if only if it
> is non-zero, warns and suggests a fix.
>
> This is the first code I write for LLVM so please review it carefully.
> Specifically, I was not sure whether to create another warning group
> in DiagnosticSemaKinds.td or reuse an existing one. Maybe the group should
> be renamed from "DivZero" to "DivProblems".
>
>
> Wow. This seems really helpful!
>
> As Matt suggested, please resubmit this patch to cfe-commits and add a
> test case.
>
> +//
> +  // check for integer constant division by integer constant
>
> Also the comment seems misformatted. Remove the '//', start with an
> uppercase letter and finish the sentence with a point.
>
> Also, here is an interesting test case that you may want to consider.
>
> int foo(float b) {
>  float a = 1/2 + b;
> }
>
> Suggesting here to transform this into
>
> int foo(float b) {
> float a = 1.0/2 + b;
> }
>
> may not be what the user actually wants, as this promotes the whole
> expression to 'double'. Possibly the following is more in line.
>
> int foo(float b) {
> float a = 1.0f/2 + b;
> }
>
> Cheers,
> Tobias
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> 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.
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140109/a9b56113/attachment.html>