Clojure

Cannot reduce over short[] arrays

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Duplicate
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:

Description

Reducing over a short array currently causes an error:

(reduce + (seq (short-array 10)))
=> ClassCastException [S cannot be cast to [Ljava.lang.Object; clojure.core.protocols/fn--6037 (protocols.clj:126)

This appears to occur because ArraySeq is assumed by protocols.clj to contain an Object[] array in the ".array" field, when in fact it is a short[] array.

Proposed solution to to create ArraySeq_short (analogous to the other primitive types ArraySeq_long etc.) to handle short arrays.

Activity

Hide
Mike Anderson added a comment -

Fix for CLJ-1306

Show
Mike Anderson added a comment - Fix for CLJ-1306
Hide
Alex Miller added a comment -

This was also discovered in CLJ-1200 and the patch for that issue includes ArraySeq_short as you propose. It is expected that CLJ-1200 will be included in 1.6.

Show
Alex Miller added a comment - This was also discovered in CLJ-1200 and the patch for that issue includes ArraySeq_short as you propose. It is expected that CLJ-1200 will be included in 1.6.
Hide
Mike Anderson added a comment -

OK, thanks Alex!

Just to be sure: Can you confirm that a fix will definitely go into 1.6? This is a defect, and as such should have a higher priority than CLJ-1200 (which appears to be presented as an performance enhancement patch).

My patch also includes a regression test which I think could helpfully be included in the test suite.

Show
Mike Anderson added a comment - OK, thanks Alex! Just to be sure: Can you confirm that a fix will definitely go into 1.6? This is a defect, and as such should have a higher priority than CLJ-1200 (which appears to be presented as an performance enhancement patch). My patch also includes a regression test which I think could helpfully be included in the test suite.
Hide
Alex Miller added a comment -

Yes, I fully expect CLJ-1200 to be included and talked to Rich about it as recently as yesterday. I could split things out of that patch and pull both of these in separately. That would be objectively better but definitely more work to do all the ticket, patch, and screening work so I'd rather not. If you want to attach just the regression test to 1200, I think we could include it that way as 1200 hasn't been screened yet. Stuart Sierra is planning to screen it in the next few days.

Show
Alex Miller added a comment - Yes, I fully expect CLJ-1200 to be included and talked to Rich about it as recently as yesterday. I could split things out of that patch and pull both of these in separately. That would be objectively better but definitely more work to do all the ticket, patch, and screening work so I'd rather not. If you want to attach just the regression test to 1200, I think we could include it that way as 1200 hasn't been screened yet. Stuart Sierra is planning to screen it in the next few days.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: