User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9) Gecko/2008052912 Firefox/3.0
Build Identifier: Rhino 1.7R1
If i try to run follow code:
var layRegE00 = java.lang.reflect.Array.newInstance(java.lang.Object, [17,4]);
i get the exception:
Cannot convert 17,4 to java.lang.Integer.
this code works in version 1.6R1 of Rhino.
Steps to Reproduce:
I guess this might have to do with the patch from bug 382340. I just tried and it is broken with 1.6R6, which is the first release containing that patch. FWIW, invoking that method varargs style works:
java.lang.reflect.Array.newInstance(java.lang.Object, 17, 4);
I found the problem. The java.lang.reflect.Array.newInstance() method taking an int array as second argument has changed to varargs signature in Java 6, and NativeJavaMethod.preferSignature() currently always prefers non-varargs methods over varargs methods. Thus, the method with int as second argument is called on Java 6, while on Java 5 the right method in chosen.
I guess we shouldn't blindly prefer non-varargs methods over varargs ones. Removing that code results in the varargs method being called with an array object, and the non-varargs one with a number argument.
Apart from that, I wonder if it is right that NativeJavaObject.canConvert() returns true for Scriptable -> java.lang.Number conversion. Is there a scenario where this conversion actually works - and makes sense?
Created attachment 351536 [details] [diff] [review]
patch and test case
This patch removes the code that always prefers non-varargs methods over varargs ones. From the original comment, my feeling is that the author added that code without a stringent cause such as actual code requiring it, and the testcase shows the new code works in this case. The comment is updated to point to this bug.
Committed to HEAD:
new revision: 1.64; previous revision: 1.63
new revision: 188.8.131.52; previous revision: 1.1
Probably should go also to Rhino1_7R2_BRANCH, although there's a slight chance something else might break.
This change caused a regression (see bug 496585). I'm working on a fix that will preserve this case as well.
I committed my fix to bug 496585. That fix includes a new regression test for this bug: testsrc/doctests/467396.doctest.