8011065: Problems when script implements an interface with variadic methods
Reviewed-by: jlaskey, hannesw, sundar
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Apr 22 19:57:57 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Tue Apr 23 12:52:29 2013 +0200
@@ -664,7 +664,7 @@
final int useCount = symbol.getUseCount();
// Threshold for generating shared scope callsite is lower for fast scope symbols because we know
- // we can dial in the correct scope. However, we als need to enable it for non-fast scopes to
+ // we can dial in the correct scope. However, we also need to enable it for non-fast scopes to
// support huge scripts like mandreel.js.
if (callNode.isEval()) {
evalCall(node, flags);
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java Mon Apr 22 19:57:57 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java Tue Apr 23 12:52:29 2013 +0200
@@ -28,6 +28,7 @@
import static jdk.nashorn.internal.codegen.CompilerConstants.staticCall;
import static jdk.nashorn.internal.codegen.CompilerConstants.virtualCall;
import static jdk.nashorn.internal.codegen.CompilerConstants.virtualCallNoLookup;
+import static jdk.nashorn.internal.lookup.Lookup.MH;
import static jdk.nashorn.internal.runtime.ECMAErrors.referenceError;
import static jdk.nashorn.internal.runtime.ECMAErrors.typeError;
import static jdk.nashorn.internal.runtime.PropertyDescriptor.CONFIGURABLE;
@@ -39,7 +40,6 @@
import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
import static jdk.nashorn.internal.runtime.arrays.ArrayIndex.getArrayIndexNoThrow;
import static jdk.nashorn.internal.runtime.arrays.ArrayIndex.isValidArrayIndex;
-import static jdk.nashorn.internal.lookup.Lookup.MH;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
@@ -61,12 +61,13 @@
import jdk.internal.dynalink.support.CallSiteDescriptorFactory;
import jdk.nashorn.internal.codegen.CompilerConstants.Call;
import jdk.nashorn.internal.codegen.ObjectClassGenerator;
+import jdk.nashorn.internal.lookup.Lookup;
+import jdk.nashorn.internal.lookup.MethodHandleFactory;
import jdk.nashorn.internal.objects.AccessorPropertyDescriptor;
import jdk.nashorn.internal.objects.DataPropertyDescriptor;
import jdk.nashorn.internal.runtime.arrays.ArrayData;
import jdk.nashorn.internal.runtime.linker.Bootstrap;
-import jdk.nashorn.internal.lookup.Lookup;
-import jdk.nashorn.internal.lookup.MethodHandleFactory;
+import jdk.nashorn.internal.runtime.linker.LinkerCallSite;
import jdk.nashorn.internal.runtime.linker.NashornCallSiteDescriptor;
import jdk.nashorn.internal.runtime.linker.NashornGuards;
@@ -2139,8 +2140,11 @@
*
* Make sure arguments are paired correctly.
* @param methodHandle MethodHandle to adjust.
- * @param callType MethodType of caller.
- * @param callerVarArg true if the caller is vararg, false otherwise, null if it should be inferred.
+ * @param callType MethodType of the call site.
+ * @param callerVarArg true if the caller is vararg, false otherwise, null if it should be inferred from the
+ * {@code callType}; basically, if the last parameter type of the call site is an array, it'll be considered a
+ * variable arity call site. These are ordinarily rare; Nashorn code generator creates variable arity call sites
+ * when the call has more than {@link LinkerCallSite#ARGLIMIT} parameters.
*
* @return method handle with adjusted arguments
*/
@@ -2155,7 +2159,7 @@
final int callCount = callType.parameterCount();
final boolean isCalleeVarArg = parameterCount > 0 && methodType.parameterType(parameterCount - 1).isArray();
- final boolean isCallerVarArg = callerVarArg != null ? callerVarArg.booleanValue() : (callCount > 1 &&
+ final boolean isCallerVarArg = callerVarArg != null ? callerVarArg.booleanValue() : (callCount > 0 &&
callType.parameterType(callCount - 1).isArray());
if (callCount < parameterCount) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java Mon Apr 22 19:57:57 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java Tue Apr 23 12:52:29 2013 +0200
@@ -127,9 +127,9 @@
private static final Type METHOD_TYPE_TYPE = Type.getType(MethodType.class);
private static final Type METHOD_HANDLE_TYPE = Type.getType(MethodHandle.class);
private static final String GET_HANDLE_OBJECT_DESCRIPTOR = Type.getMethodDescriptor(METHOD_HANDLE_TYPE,
- OBJECT_TYPE, STRING_TYPE, METHOD_TYPE_TYPE, Type.BOOLEAN_TYPE);
+ OBJECT_TYPE, STRING_TYPE, METHOD_TYPE_TYPE);
private static final String GET_HANDLE_FUNCTION_DESCRIPTOR = Type.getMethodDescriptor(METHOD_HANDLE_TYPE,
- SCRIPT_FUNCTION_TYPE, METHOD_TYPE_TYPE, Type.BOOLEAN_TYPE);
+ SCRIPT_FUNCTION_TYPE, METHOD_TYPE_TYPE);
private static final String GET_CLASS_INITIALIZER_DESCRIPTOR = Type.getMethodDescriptor(SCRIPT_OBJECT_TYPE);
private static final Type RUNTIME_EXCEPTION_TYPE = Type.getType(RuntimeException.class);
private static final Type THROWABLE_TYPE = Type.getType(Throwable.class);
@@ -315,7 +315,6 @@
mv.dup();
mv.aconst(mi.getName());
mv.aconst(Type.getMethodType(mi.type.toMethodDescriptorString()));
- mv.iconst(mi.method.isVarArgs() ? 1 : 0);
mv.invokestatic(SERVICES_CLASS_TYPE_NAME, "getHandle", GET_HANDLE_OBJECT_DESCRIPTOR);
mv.putstatic(generatedClassName, mi.methodHandleClassFieldName, METHOD_HANDLE_TYPE_DESCRIPTOR);
}
@@ -459,7 +458,6 @@
mv.aconst(mi.getName());
}
mv.aconst(Type.getMethodType(mi.type.toMethodDescriptorString()));
- mv.iconst(mi.method.isVarArgs() ? 1 : 0);
mv.invokestatic(SERVICES_CLASS_TYPE_NAME, "getHandle", getHandleDescriptor);
}
mv.putfield(generatedClassName, mi.methodHandleInstanceFieldName, METHOD_HANDLE_TYPE_DESCRIPTOR);
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterServices.java Mon Apr 22 19:57:57 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterServices.java Tue Apr 23 12:52:29 2013 +0200
@@ -50,12 +50,11 @@
* handles for their abstract method implementations.
* @param fn the script function
* @param type the method type it has to conform to
- * @param varArg if the Java method for which the function is being adapted is a variable arity method
* @return the appropriately adapted method handle for invoking the script function.
*/
- public static MethodHandle getHandle(final ScriptFunction fn, final MethodType type, final boolean varArg) {
+ public static MethodHandle getHandle(final ScriptFunction fn, final MethodType type) {
// JS "this" will be null for SAMs
- return adaptHandle(fn.getBoundInvokeHandle(null), type, varArg);
+ return adaptHandle(fn.getBoundInvokeHandle(null), type);
}
/**
@@ -66,12 +65,11 @@
* @param obj the script obj
* @param name the name of the property that contains the function
* @param type the method type it has to conform to
- * @param varArg if the Java method for which the function is being adapted is a variable arity method
* @return the appropriately adapted method handle for invoking the script function, or null if the value of the
* property is either null or undefined, or "toString" was requested as the name, but the object doesn't directly
* define it but just inherits it through prototype.
*/
- public static MethodHandle getHandle(final Object obj, final String name, final MethodType type, final boolean varArg) {
+ public static MethodHandle getHandle(final Object obj, final String name, final MethodType type) {
if (! (obj instanceof ScriptObject)) {
throw typeError("not.an.object", ScriptRuntime.safeToString(obj));
}
@@ -84,7 +82,7 @@
final Object fnObj = sobj.get(name);
if (fnObj instanceof ScriptFunction) {
- return adaptHandle(((ScriptFunction)fnObj).getBoundInvokeHandle(sobj), type, varArg);
+ return adaptHandle(((ScriptFunction)fnObj).getBoundInvokeHandle(sobj), type);
} else if(fnObj == null || fnObj instanceof Undefined) {
return null;
} else {
@@ -108,7 +106,7 @@
classOverrides.set(overrides);
}
- private static MethodHandle adaptHandle(final MethodHandle handle, final MethodType type, final boolean varArg) {
- return Bootstrap.getLinkerServices().asType(ScriptObject.pairArguments(handle, type, varArg), type);
+ private static MethodHandle adaptHandle(final MethodHandle handle, final MethodType type) {
+ return Bootstrap.getLinkerServices().asType(ScriptObject.pairArguments(handle, type, false), type);
}
}
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Mon Apr 22 19:57:57 2013 +0530
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Tue Apr 23 12:52:29 2013 +0200
@@ -906,4 +906,26 @@
fail(se.getMessage());
}
}
+
+ @Test
+ /**
+ * Tests whether invocation of a JavaScript method through a variable arity Java method will pass the vararg array.
+ * Both non-vararg and vararg JavaScript methods are tested.
+ * @throws ScriptException
+ */
+ public void variableArityInterfaceTest() throws ScriptException {
+ final ScriptEngineManager m = new ScriptEngineManager();
+ final ScriptEngine e = m.getEngineByName("nashorn");
+ e.eval(
+ "function test1(i, strings) {" +
+ " return 'i == ' + i + ', strings instanceof java.lang.String[] == ' + (strings instanceof Java.type('java.lang.String[]')) + ', strings == ' + java.util.Arrays.toString(strings)" +
+ "}" +
+ "function test2() {" +
+ " return 'arguments[0] == ' + arguments[0] + ', arguments[1] instanceof java.lang.String[] == ' + (arguments[1] instanceof Java.type('java.lang.String[]')) + ', arguments[1] == ' + java.util.Arrays.toString(arguments[1])" +
+ "}"
+ );
+ final VariableArityTestInterface itf = ((Invocable)e).getInterface(VariableArityTestInterface.class);
+ Assert.assertEquals(itf.test1(42, "a", "b"), "i == 42, strings instanceof java.lang.String[] == true, strings == [a, b]");
+ Assert.assertEquals(itf.test2(44, "c", "d", "e"), "arguments[0] == 44, arguments[1] instanceof java.lang.String[] == true, arguments[1] == [c, d, e]");
+ }
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/VariableArityTestInterface.java Tue Apr 23 12:52:29 2013 +0200
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.nashorn.api.scripting;
+
+public interface VariableArityTestInterface {
+ public String test1(int i, String... strings);
+ public String test2(int i, String... strings);
+}