[CLJCLR-49] ArraySeq.createFromObject broken on arrays of value types Created: 14/Nov/14 Updated: 14/Nov/14
|Reporter:||Ramsey Nasser||Assignee:||David Miller|
Calling seq on CLR arrays eventually comes to ArraySeq.createFromObject, which handles arrays of primitive and reference types correctly, but throws an exception when called with an array of value types. Value types have a TypeCode of Object, just like reference types, so they both fall into the branch on line 91 of ArraySeq, which attempts an object prefix cast. Casting an array of value types to an array of reference types is not allowed, so this will throw an exception.
Original Arcadia issue: https://github.com/arcadia-unity/Arcadia/issues/75. The example case makes use of Unity types, but the same should hold for any CLR environment.
|Comment by Ramsey Nasser [ 14/Nov/14 1:19 PM ]|
The provided patch fixes the problem by leveraging CLR generics and the already present TypedArraySeq class for value types. The following changes were needed
1. TypedArraySeq was made public and non-abstract, and given generic implementations for its previously abstract methods
|Comment by Ramsey Nasser [ 14/Nov/14 2:41 PM ]|
The only odd part of the patch is the ConvertNum implementation, which right now just casts to the generic type, mirroring what ArraySeq_object does. Not sure what else to do there, but I have other questions about ConvertNum.
TypedArraySeq could conceivably replace all the hard coded ArraySeq_* subclasses. This would result in much smaller (~65% smaller), simpler code. They seem to all do the exact same thing, except in the ConvertNum method, which brings up a few questions. Why is ConvertNum there, and why is it so concerned with numbers? It is only used in the IList.IndexOf implementation, where input is run through a Util.IsNumeric check before converting to a number via ConvertNum, and only then looking up the index. This means that IndexOf will always return -1 for anything that is not numeric. Is there a reason for this behavior, or is it a bug? As usual I'm not aware of the farther reaching implications of these implementation details.
If it is a bug, then I propose we replace ConvertNum with ConvertToType or just Convert that converts a value to the generic type of the TypedArraySeq and remove the Util.IsNumeric check from IndexOf. I can provide this patch.