Clojure

Implicit casting behaviour of into-array differs from <primitive>-array

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.3
  • Fix Version/s: Release 1.6
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Incomplete

Description

Current patch: byte-short-array-ctors.diff

The behavior of byte-array and short-array is inconsistent with the other <type>-array functions and with the into-array function when invoked with types other than byte or short. All of the other cases upcast to Number, then extract the primitive value, allowing this operation to succeed (assuming the value is in range). byte-array and short-array throw a ClassCastException.

Example:

base64v3a=> (into-array Byte/TYPE [1 2 3 4])  ;; int to byte ok
#<byte[] [B@5ee04fd>
base64v3a=> (byte-array [1 2 3 4])  ;; int to byte NOT ok!! 
ClassCastException java.lang.Long cannot be cast to java.lang.Byte
  clojure.lang.Numbers.byte_array (Numbers.java:1418)
base64v3a=> (long-array [1 2 3 4])  ;; int to long ok
#<long[] [J@3f9f4d1d>

into-array (via RT.seqToTypedArray) and the other <type>-array functions all upcast to Number (via Numbers.<type>_array}}), then obtain the proper primitive value. Numbers.byte-array and Numbers.short-array do casts directly to Byte and Short (yielding the ClassCastException).

The attached patch makes the Byte and Short cases match the other types and the into-array behavior. Tests are included. The submitter is a contributor.

Activity

Descending order - Click to sort in ascending order
Hide
Permalink
Alex Miller added a comment -

Sending back to Karsten for the requested patch updates.

Show
Alex Miller added a comment - Sending back to Karsten for the requested patch updates.
Hide
Permalink
Alex Miller added a comment - - edited

I think the patch approach is valid. However, the patch does not cover the same problem in the 2-arity version of byte_array and short_array (when a size is supplied). Please update the patch to include a fix in the 2-arity version of byte_array and short_array and tests for the same. Ex: (byte-array 1 [1]).

Also, there is a whitespace issue in the patch - please just use spaces! 

/Users/alex/work/code/clojure/.git/rebase-apply/patch:24: space before tab in indent.
	    	ret[i] = ((Number)s.first()).byteValue();
warning: 1 line adds whitespace errors.

Re Stuart's comments, I don't think that's in the scope of this ticket or solution.

Show
Alex Miller added a comment - - edited I think the patch approach is valid. However, the patch does not cover the same problem in the 2-arity version of byte_array and short_array (when a size is supplied). Please update the patch to include a fix in the 2-arity version of byte_array and short_array and tests for the same. Ex: (byte-array 1 [1]). Also, there is a whitespace issue in the patch - please just use spaces! 
/Users/alex/work/code/clojure/.git/rebase-apply/patch:24: space before tab in indent.
	    	ret[i] = ((Number)s.first()).byteValue();
warning: 1 line adds whitespace errors.
Re Stuart's comments, I don't think that's in the scope of this ticket or solution.
Hide
Permalink
Stuart Halloway added a comment -

I agree that all the numeric array constructors should work the same way, but I am not sue we should adopt long-array's approach, which allows coercion from float to integer types.

Show
Stuart Halloway added a comment - I agree that all the numeric array constructors should work the same way, but I am not sue we should adopt long-array's approach, which allows coercion from float to integer types.
Hide
Permalink
Stuart Halloway added a comment -

While investigating this, I noticed that long-array coerces across type, e.g.

(seq (long-array [1.0]))
=> (1)

Is this what we want? I don't think so.

Show
Stuart Halloway added a comment - While investigating this, I noticed that long-array coerces across type, e.g. 
(seq (long-array [1.0]))
=> (1)
Is this what we want? I don't think so.
Hide
Permalink
Karsten Schmidt added a comment -

Sorry Andy, I used Atlassian SourceTree to create the patch and assumed it's in standard git format... I've removed the old one and attach a (hopefully) properly formatted one. Apologies, contributing-beginners-luck!

Show
Karsten Schmidt added a comment - Sorry Andy, I used Atlassian SourceTree to create the patch and assumed it's in standard git format... I've removed the old one and attach a (hopefully) properly formatted one. Apologies, contributing-beginners-luck!
Hide
Permalink
Andy Fingerhut added a comment -

Karsten, patches should be in the format created by the "git format-patch" command as described in the instructions under the heading "Development" on this page: http://dev.clojure.org/display/design/JIRA+workflow

Show
Andy Fingerhut added a comment - Karsten, patches should be in the format created by the "git format-patch" command as described in the instructions under the heading "Development" on this page: http://dev.clojure.org/display/design/JIRA+workflow
Hide
Permalink
Andy Fingerhut added a comment -

Voting on a ticket (click the "Vote" link under the "People" heading while viewing the ticket on JIRA) may help draw attention to it, too.

Show
Andy Fingerhut added a comment - Voting on a ticket (click the "Vote" link under the "People" heading while viewing the ticket on JIRA) may help draw attention to it, too.
Hide
Permalink
Karsten Schmidt added a comment -

I'd like to bump up this issue, since it'd be great if all the (xxx-array) factory functions have the same expected behavior (i.e. not throw an exception, especially not if the values are in range). The attached patch is changing the behaviour for byte-array & short-array to the same pattern used as for int/long/float/double and therefore will not throw an exception for valid (even if overflown) numbers. You can find more discussion in this thread on:

https://groups.google.com/forum/?hl=en&fromgroups=#!topic/clojure/KyQrbph-zqo

Show
Karsten Schmidt added a comment - I'd like to bump up this issue, since it'd be great if all the (xxx-array) factory functions have the same expected behavior (i.e. not throw an exception, especially not if the values are in range). The attached patch is changing the behaviour for byte-array & short-array to the same pattern used as for int/long/float/double and therefore will not throw an exception for valid (even if overflown) numbers. You can find more discussion in this thread on: https://groups.google.com/forum/?hl=en&fromgroups=#!topic/clojure/KyQrbph-zqo
Hide
Permalink
Andy Fingerhut added a comment -

Some more details from Alexandar Taggart: This is not a duplicate of CLJ-678. CLJ-766 was created precisely due to the fact the the behavior of *-array is not consistent with the post-678 version of into-array.

Show
Andy Fingerhut added a comment - Some more details from Alexandar Taggart: This is not a duplicate of CLJ-678. CLJ-766 was created precisely due to the fact the the behavior of *-array is not consistent with the post-678 version of into-array.
Hide
Permalink
Alexander Taggart added a comment -

See CLJ-678.

Show
Alexander Taggart added a comment - See CLJ-678.

People

Vote (3)
Watch (6)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.