<< Back to previous view

[CLJ-766] Implicit casting behaviour of into-array differs from <primitive>-array Created: 01/Apr/11  Updated: 03/May/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.3
Fix Version/s: Release 1.6

Type: Defect Priority: Minor
Reporter: Alexander Taggart Assignee: Karsten Schmidt
Resolution: Unresolved Votes: 3
Labels: None

Attachments: File byte-short-array-ctors.diff    
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.

 Comments   
Comment by Alexander Taggart [ 02/Apr/11 2:04 PM ]

See CLJ-678.

Comment by Andy Fingerhut [ 28/May/12 6:45 PM ]

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.

Comment by Karsten Schmidt [ 02/Mar/13 5:11 PM ]

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

Comment by Andy Fingerhut [ 03/Mar/13 8:11 AM ]

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.

Comment by Andy Fingerhut [ 04/Mar/13 1:08 PM ]

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

Comment by Karsten Schmidt [ 04/Mar/13 2:05 PM ]

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!

Comment by Stuart Halloway [ 30/Mar/13 8:52 AM ]

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.

Comment by Stuart Halloway [ 30/Mar/13 8:53 AM ]

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.

Comment by Alex Miller [ 03/May/13 8:31 PM ]

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.

Comment by Alex Miller [ 03/May/13 8:32 PM ]

Sending back to Karsten for the requested patch updates.

Generated at Tue Jun 18 01:32:36 CDT 2013 using JIRA 4.4#649-r158309.