Bug report: JavaScript arrays are not coerced into Java arrays when calling Java functions that expect arrays

[Tal Liron]

It works fine! Terrific, I'm glad this got in. My patch seems clumsy 
now... I still have a lot to learn about Nashorn's conversion path. To 
my defense, the code was changing underneath my eyes while I was writing 
the patch. ;)

And re-reading your email now, I do understand what you mean regarding 
varargs, and I think it is acceptible. From a JavaScript caller's 
perspective, varargs *are* arrays, so you should expect indeed to send 
at least a JavaScript array. Otherwise, I think vararg handling could 
have gotten very murky... how would the caller be able to send an array 
object as the first argument of the array? It actually seems impossible 
to get varargs right. And this is Rhino's behavior, too (I think?). So, 
good call.

On 10/16/2013 12:22 PM, Tal Liron wrote:
> Thanks! I will test this soon. It does sound like your implementation 
> is more powerful.
>
> However, I'm curious about your decision concerning varargs. Why the 
> difference? In fact, an API with varargs was exactly the one causing 
> me the trouble that was the stimulus for my proposed patch...
>
> On 10/16/2013 05:10 AM, Attila Szegedi wrote:
>> Hey Tal,
>>
>> I read through your patch, but ended up implementing this 
>> functionality independently, as I wanted to reuse most of our 
>> existing code and also realized that I had to touch few more pieces - 
>> I committed it in two phases:
>> <http://hg.openjdk.java.net/nashorn/jdk8/nashorn/rev/d155c4a7703c> then
>> <http://hg.openjdk.java.net/nashorn/jdk8/nashorn/rev/64e841576c68>.
>>
>> I ended up reusing most of existing Java.to() machinery. One of 
>> beneficial side effects is that since the algorithm behind Java.to() 
>> converts individual elements recursively using the type conversion 
>> mechanism, conversion to multidimensional arrays Just Works. For 
>> maximum fun, converting a JS Array-of-Arrays to List[] will also do 
>> what you'd expect: convert the first level of JS Array to a Java 
>> array, and any contained Arrays into Java lists :-)
>>
>> Also, isInstanceGuardAlwaysFalse was actually signaling us something 
>> serious, and it took a bit to rework. Namely, in presence of 
>> language-specific conversions to Java array types, it's an 
>> interesting question how do we treat invocation of variable arity 
>> methods when there's a single argument in the vararg array position, 
>> and it can be converted to a Java array. I had to rework the linking 
>> in SingleDynamicMethod for this case quite a bit, but the end result 
>> is that if you pass a JS array there, it'll be converted and used as 
>> the vararg array, which is likely the expected behavior (see tests in 
>> the second patch). Also, conversion from a JavaScript array to List 
>> or Deque also works automatically - those are very cheap to create 
>> live wrappers.
>>
>> One minor thing is that for purposes of auto-converting JS arrays in 
>> vararg position, I decided that the automatic conversion will only 
>> work for actual Array instances, and not arbitrary array-like 
>> objects, whether they have Array as their prototype or not. Those can 
>> still be converted using explicit Java.to().
>>
>> In any case, thank you for your contribution; it certainly acted as a 
>> stimulus.
>>
>> Cheers,
>>   Attila.
>>
>> On Oct 11, 2013, at 12:59 PM, Tal Liron <tal.liron at threecrickets.com 
>> <mailto:tal.liron at threecrickets.com>> wrote:
>>
>>> Attached is my working patch. I've signed the developer agreement 
>>> and sent it to the Oracle as required.
>>>
>>> I've marked all my changes with a "// TAL" comment so you can easily 
>>> find them.
>>>
>>> Here is the explanation:
>>>
>>> JavaArgumentConverters.java: I've added new converters for all 
>>> primitive array types, as well as Object[] and String[]. For 
>>> NativeArrays, it would convert all the individual elements. For 
>>> other object types, it would attempt to create a one-element array, 
>>> assuming that element is convertible. I added a few utility methods, 
>>> too, to help with this. I also took the liberty to make a change to 
>>> toString: it will fallback to Object.toString. (I have a feeling 
>>> some of you will not like this change, as it's not directly related 
>>> to the feature, but I see no harm in it, and it much helped my 
>>> testing. In fact, it's even Java's behavior when working with 
>>> non-string objects. Without this change, JavaScript programmers 
>>> would still have to work hard to set up arrays before sending them 
>>> to Java.)
>>>
>>> NashornPrimitiveLinker.java: canLinkTypeStatic now recognizes all 
>>> the supported primitive array types. In compareConversion, I've also 
>>> added a check to prefer conversion to arrays over conversion to 
>>> primitives. Without this, an ambiguous exception could be thrown in 
>>> cases where two similar method signatures exist. (For example, 
>>> java.lang.Runtime.execute has a version that accepts a string array 
>>> and another that accepts a string.) I think this can be further 
>>> improved to prefer Object[] over String[] if there is such 
>>> ambiguity. What I need help with from you guys is in 
>>> getGuardedInvocation: I currently return null for NativeArray, 
>>> because I'm not sure how to get an appropriate guard. Currently 
>>> primitiveLookup only supports non-array primitive types. I think 
>>> what happens in this case is that a default guard is provided, but 
>>> I'm not entirely sure.
>>>
>>> Guards.java: I've added extra checks to avoid the 
>>> isInstanceGuardAlwaysFalse log warning for array conversions.
>>>
>>>
>>> On 10/09/2013 10:28 PM, Jim Laskey wrote:
>>>> If you can get the changes to me by Tuesday Oct 15th, I'll take a 
>>>> look.  No guarantees, but if the changes are small, are correct, 
>>>> and do not restrict future enhancements in this area then I'll run 
>>>> the changes up the approval chain.
>>>>
>>>>
>>>
>>> <arrays.txt>
>>
>