<< Back to previous view

[CLJ-1225] quot overflow issues around Long/MIN_VALUE for BigInt Created: 25/Jun/13  Updated: 04/Oct/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math

Attachments: Text File clj-1225-2.txt     Text File clj-1225-fix-division-overflow-patch-v1.txt    
Patch: Code and Test
Approval: Triaged

 Description   

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

user=> (quot Long/MIN_VALUE -1N)
-9223372036854775808N
user=> (quot (bigint Long/MIN_VALUE) -1)
-9223372036854775808N

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



 Comments   
Comment by Andy Fingerhut [ 25/Jun/13 11:03 AM ]

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.

Comment by Andy Fingerhut [ 25/Jun/13 11:13 AM ]

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.

Comment by Rich Hickey [ 05/Sep/13 8:39 AM ]

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

Comment by Andy Fingerhut [ 06/Sep/13 12: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.

Comment by Andy Fingerhut [ 06/Sep/13 12: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.

Generated at Fri Oct 24 11:16:19 CDT 2014 using JIRA 4.4#649-r158309.