[#CLJ-952] bigdec does not properly convert a clojure.lang.BigInt - Clojure JIRA

Details

Type: Defect
Status: Closed
Priority: Major
Resolution: Completed
Affects Version/s: Release 1.3, Release 1.4
Fix Version/s: Release 1.5
Component/s: None
Labels:
None

Patch:
Code and Test
Approval:
Ok

Description

bigdec handles java.math.BigInteger when converting to java.math.BigDecimal, but it does not handle clojure.lang.BigInt. Instead it treats a clojure.lang.BigInt as a Number, by casting it to long. This causes the following error:

Clojure 1.4.0-beta3
user=> (bigdec (inc (bigint Long/MAX_VALUE)))
IllegalArgumentException Value out of range for long: 9223372036854775808 clojure.lang.RT.longCast (RT.java:1123)

More Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

clj-952-make-bigdec-work-on-bigints-patch1.txt

21/Mar/12 10:55 AM

2 kB

Andy Fingerhut

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Andy Fingerhut added a comment - 21/Mar/12 10:55 AM

Add a case to bigdec to handle BigInts. Also eliminate a reflection warning in the ratio case while we are in there. Paul's failing case has been added to tests, fails before the fix, and passes after. Attempted to make it as run-time efficient as possible by creating a new BigInt/toBigDecimal, patterned after the existing BigInt/toBigInteger.

Show

Andy Fingerhut added a comment - 21/Mar/12 10:55 AM Add a case to bigdec to handle BigInts. Also eliminate a reflection warning in the ratio case while we are in there. Paul's failing case has been added to tests, fails before the fix, and passes after. Attempted to make it as run-time efficient as possible by creating a new BigInt/toBigDecimal, patterned after the existing BigInt/toBigInteger.

Andy Fingerhut made changes - 21/Mar/12 10:55 AM

Field	Original Value	New Value
Attachment		clj-952-make-bigdec-work-on-bigints-patch1.txt [ 10999 ]

Andy Fingerhut made changes - 21/Mar/12 10:56 AM

Patch

Code and Test [ 10002 ]

Hide

Permalink

Paul Stadig added a comment - 21/Mar/12 11:51 AM - edited

I was originally thinking of something like (BigDecimal. (.toBigInteger ^clojure.lang.BigInt x)). Adding a toBigDecimal method to clojure.lang.BigInt saves some object allocations and such. Probably more of a micro optimization, but it works.

Clojure 1.4.0-master-SNAPSHOT
user=> (bigdec (inc (bigint Long/MAX_VALUE)))
9223372036854775808M

Thanks, Andy!

Show

Paul Stadig added a comment - 21/Mar/12 11:51 AM - edited I was originally thinking of something like (BigDecimal. (.toBigInteger ^clojure.lang.BigInt x)). Adding a toBigDecimal method to clojure.lang.BigInt saves some object allocations and such. Probably more of a micro optimization, but it works.

Clojure 1.4.0-master-SNAPSHOT user=> (bigdec (inc (bigint Long/MAX_VALUE))) 9223372036854775808M Thanks, Andy! +1

Hide

Permalink

Alan Dipert added a comment - 20/Apr/12 1:35 PM

Looks good to me, thanks.

Show

Alan Dipert added a comment - 20/Apr/12 1:35 PM Looks good to me, thanks.

Alan Dipert made changes - 20/Apr/12 1:35 PM

Approval		Screened [ 10004 ]
Fix Version/s		Release 1.5 [ 10150 ]

Rich Hickey made changes - 27/Apr/12 7:31 AM

Approval

Screened [ 10004 ]

Ok [ 10007 ]

Stuart Halloway made changes - 18/May/12 12:32 PM

Resolution		Completed [ 1 ]
Status	Open [ 1 ]	Closed [ 6 ]

People

Assignee:

Unassigned

Reporter:

Paul Stadig

Vote (2)

Watch (2)

Dates

Created:

12/Mar/12 2:21 PM

Updated:

18/May/12 12:32 PM

Resolved:

18/May/12 12:32 PM