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 <[email protected]> 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>

Reply via email to