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