Clojure

Add support for Big* numeric types to Reflector

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

This should work as expected, for example:

(Integer. 1N)

Probably for BigInt, BigInteger, and BigDecimal.

Method to look at is c.l.Reflector.paramArgTypeMatch, per Rich in irc.

Activity

Stuart Halloway made changes -
Field Original Value New Value
Fix Version/s Release.Next [ 10038 ]
Hide
Colin Jones added a comment -

Questions posed on the clojure-dev list around how this impacts bit-shift-left: http://groups.google.com/group/clojure-dev/browse_thread/thread/2191cbf0048d8ca6

Show
Colin Jones added a comment - Questions posed on the clojure-dev list around how this impacts bit-shift-left: http://groups.google.com/group/clojure-dev/browse_thread/thread/2191cbf0048d8ca6
Hide
Alexander Taggart added a comment -

Patch on CLJ-445 fixes this as well.

Show
Alexander Taggart added a comment - Patch on CLJ-445 fixes this as well.
Christopher Redinger made changes -
Waiting On alan.dipert
Hide
Colin Jones added a comment -

This patch fails a test around bit-shifting a BigInt: `(bit-shift-left 1N 10000)`. The reason is that the patch changes the dispatch of (BigInt, Long) from (Object, Object) to (long, int).

Clearly this can't be applied (unless another change makes it possible), but I'm putting it up as a start of the conversation.

Show
Colin Jones added a comment - This patch fails a test around bit-shifting a BigInt: `(bit-shift-left 1N 10000)`. The reason is that the patch changes the dispatch of (BigInt, Long) from (Object, Object) to (long, int). Clearly this can't be applied (unless another change makes it possible), but I'm putting it up as a start of the conversation.
Colin Jones made changes -
Hide
Alexander Taggart added a comment -

My comment from the mailing list:

If the test breaks it likely means Numbers.shiftLeft(long,int) was
selected over Numbers.shiftLeft(Object,Object). Given that 1N is an
Object (one that can exceed the size of a long), the method selection
is incorrect, thus the patch is broken.


The suggestion of "simply" modifying paramArgTypeMatch is not sufficient since the mechanism for preferring one method over another lives in Compiler, and isn't smart enough to make these sorts of decisions.

Show
Alexander Taggart added a comment - My comment from the mailing list: If the test breaks it likely means Numbers.shiftLeft(long,int) was selected over Numbers.shiftLeft(Object,Object). Given that 1N is an Object (one that can exceed the size of a long), the method selection is incorrect, thus the patch is broken.
The suggestion of "simply" modifying paramArgTypeMatch is not sufficient since the mechanism for preferring one method over another lives in Compiler, and isn't smart enough to make these sorts of decisions.
Hide
Christopher Redinger added a comment -

Considering moving this out of Release.next - soliciting comments from Chas.

Show
Christopher Redinger added a comment - Considering moving this out of Release.next - soliciting comments from Chas.
Christopher Redinger made changes -
Waiting On alan.dipert cemerick
Hide
Chas Emerick added a comment -

I'm afraid I don't have any particular insight into the issues involved at this point. I ran into the problem originally noted a while back, and opened the ticket at Rich's suggestion. I'm sorry if the text of the ticket led anyone down unfruitful paths…

Show
Chas Emerick added a comment - I'm afraid I don't have any particular insight into the issues involved at this point. I ran into the problem originally noted a while back, and opened the ticket at Rich's suggestion. I'm sorry if the text of the ticket led anyone down unfruitful paths…
Chas Emerick made changes -
Assignee Chas Emerick [ cemerick ]
Chas Emerick made changes -
Waiting On cemerick
Hide
Luke VanderHart added a comment -

The issues relating to bitshift are moot since the decision was made that bit-shifts are only for 32/64 bit values.

Still a valid issue, but de-prioritized as per Rich.

Show
Luke VanderHart added a comment - The issues relating to bitshift are moot since the decision was made that bit-shifts are only for 32/64 bit values. Still a valid issue, but de-prioritized as per Rich.
Luke VanderHart made changes -
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Release.Next [ 10038 ]
Hide
Alex Ott added a comment -

Modified version of original patch

Show
Alex Ott added a comment - Modified version of original patch
Alex Ott made changes -
Attachment 0001-Add-Big-support-to-Reflector.patch [ 11347 ]
Hide
Andy Fingerhut added a comment -

Alex, would you mind attaching it with a unique file name? I know that JIRA lets us create multiple attachments with the same file name, and I know we can tell them apart by date and the account of the person who uploaded the attachment, but giving them the same name only seems to invite confusion.

Show
Andy Fingerhut added a comment - Alex, would you mind attaching it with a unique file name? I know that JIRA lets us create multiple attachments with the same file name, and I know we can tell them apart by date and the account of the person who uploaded the attachment, but giving them the same name only seems to invite confusion.
Hide
Alex Ott added a comment - - edited

Renamed updated patch to unique name

Show
Alex Ott added a comment - - edited Renamed updated patch to unique name
Alex Ott made changes -
Alex Ott made changes -
Attachment 0001-Add-Big-support-to-Reflector.patch [ 11347 ]
Andy Fingerhut made changes -
Patch Code and Test [ 10002 ]
Alex Miller made changes -
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Backlog [ 10035 ]
Alex Miller made changes -
Fix Version/s Backlog [ 10035 ]

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated: