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

[David Tweed]

Hi, this is clearly a useful warning that it would be good to have,
but it probably falls into the same class as "assignments in
conditionals": sometimes that is what you intended to do, so is there
some syntactic way that could be used to indicate "this is what I
intended in this usage".

In particular, I couldn't remember what clang's notion of a literal
is, so I tried replacing some of the numerals in tests in the file
with values defined by macros and the tests still passed, so I presume
that means "literals obtained by either macros or C++ template
parameters" will be subject to this warning during divisions. In some
cases of hardcore template meta programming not having a way to
suppress certain instances of this warning might be quite annoying.

On Thu, Jan 9, 2014 at 1:42 AM, 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
>



-- 
cheers, dave tweed__________________________
high-performance computing and machine vision expert: david.tweed at gmail.com
"while having code so boring anyone can maintain it, use Python." --
attempted insult seen on slashdot