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

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
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 -

Modified version of original patch

Show
Alex Ott added a comment - Modified version of original patch
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.
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…
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.
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
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.
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.
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

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated: