quot overflow issues around Long/MIN_VALUE for BigInt

Description

In Clojure 1.5.1, see the following undesirable behavior regarding incorrect quot results for BigInts:

Similar issue to CLJ-1222. The root cause is that Java division of longs gives a numerically incorrect answer of Long.MIN_VALUE for (Long.MIN_VALUE / -1), because the numerically correct answer does not fit in a long. I believe this is the only pair of arguments for long division that gives a numerically incorrect answer, because division with a denominator having an absolute value of 2 or more gives a result closer to 0 than the numerator, and everything works fine for a denominator of 1 or -1, except this one case.

Related issues: CLJ-1222 for multiply, CLJ-1253 for / on longs, CLJ-1254 for quot on longs

Patch: clj-1225-2.txt
Screened by: Alex Miller

Environment

None

Attachments

2

Activity

Andy Fingerhut September 6, 2013 at 6:41 AM

Patch clj-1225-2.txt fixes this issue with quot on BigInts, with tests for quot and / on these values. / on BigInt worked fine before, but added the tests in case someone decides to change the implementation and forgets this corner case.

Andy Fingerhut September 6, 2013 at 6:37 AM

Updating description for BigInt issue only. Will create separate ticket for incorrect behavior of / and quot on long type args Long/MIN_VALUE and -1.

Rich Hickey September 5, 2013 at 2:39 PM

this is two separate issues, one with longs and one with bigints. long problem should throw

Andy Fingerhut June 25, 2013 at 5:13 PM

Another possible approach would be to create unchecked-quotient and quot', which together with quot would correspond to the existing unchecked-multiply, *' and *. That is a more significant change. One potential concern it addresses that patch clj-1225-fix-division-overflow-patch-v1.txt does not is that patch leaves a Clojure developer with no way to do a primitive Java long division except by writing Java code.

Andy Fingerhut June 25, 2013 at 5:03 PM

Patch clj-1225-fix-division-overflow-patch-v1.txt dated Jun 25 2013 may be one good way to address this issue. It modifies quot and / to return the numerically correct (BigInt) answer when given args Long/MIN_VALUE and -1.

It also removes the quotient intrinsic that does a JVM LDIV operation on longs for quot, since that operation is one of those that gives the incorrect result. I have not done any performance testing with this patch yet, but I have verified that it does not introduce any new reflection warnings when compiling Clojure itself.

Completed

Details

Assignee

Reporter

Labels

Approval

Patch

Priority

Affects versions

Fix versions

Created June 25, 2013 at 4:55 PM
Updated July 31, 2015 at 9:39 PM
Resolved July 31, 2015 at 9:39 PM