From 858f89e27771c53b40575b5ace48d134902fa3a9 Mon Sep 17 00:00:00 2001 From: attila Date: Tue, 23 Apr 2013 12:52:29 +0200 Subject: [PATCH] 8011065: Problems when script implements an interface with variadic methods Reviewed-by: jlaskey, hannesw, sundar --- .../internal/codegen/CodeGenerator.java | 2 +- .../internal/runtime/ScriptObject.java | 16 ++++++---- .../linker/JavaAdapterBytecodeGenerator.java | 6 ++-- .../runtime/linker/JavaAdapterServices.java | 14 ++++----- .../api/scripting/ScriptEngineTest.java | 22 +++++++++++++ .../scripting/VariableArityTestInterface.java | 31 +++++++++++++++++++ 6 files changed, 72 insertions(+), 19 deletions(-) create mode 100644 test/src/jdk/nashorn/api/scripting/VariableArityTestInterface.java diff --git a/src/jdk/nashorn/internal/codegen/CodeGenerator.java b/src/jdk/nashorn/internal/codegen/CodeGenerator.java index 15631ade..8814bc7b 100644 --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java @@ -664,7 +664,7 @@ final class CodeGenerator extends NodeOperatorVisitor { 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); diff --git a/src/jdk/nashorn/internal/runtime/ScriptObject.java b/src/jdk/nashorn/internal/runtime/ScriptObject.java index 5cc34194..e30ae0b1 100644 --- a/src/jdk/nashorn/internal/runtime/ScriptObject.java +++ b/src/jdk/nashorn/internal/runtime/ScriptObject.java @@ -28,6 +28,7 @@ package jdk.nashorn.internal.runtime; 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.PropertyDescriptor.WRITABLE; 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.linker.LinkRequest; 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 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr * * 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 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr 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) { diff --git a/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java b/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java index ea47878a..37786b1e 100644 --- a/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java +++ b/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java @@ -127,9 +127,9 @@ final class JavaAdapterBytecodeGenerator extends JavaAdapterGeneratorBase { 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 @@ final class JavaAdapterBytecodeGenerator extends JavaAdapterGeneratorBase { 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 @@ final class JavaAdapterBytecodeGenerator extends JavaAdapterGeneratorBase { 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); diff --git a/src/jdk/nashorn/internal/runtime/linker/JavaAdapterServices.java b/src/jdk/nashorn/internal/runtime/linker/JavaAdapterServices.java index b7a64629..eb890b9b 100644 --- a/src/jdk/nashorn/internal/runtime/linker/JavaAdapterServices.java +++ b/src/jdk/nashorn/internal/runtime/linker/JavaAdapterServices.java @@ -50,12 +50,11 @@ public class JavaAdapterServices { * 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 @@ public class JavaAdapterServices { * @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 @@ public class JavaAdapterServices { 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 @@ public class JavaAdapterServices { 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); } } diff --git a/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java b/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java index 48277aa8..918d1f05 100644 --- a/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java +++ b/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java @@ -906,4 +906,26 @@ public class ScriptEngineTest { 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]"); + } } diff --git a/test/src/jdk/nashorn/api/scripting/VariableArityTestInterface.java b/test/src/jdk/nashorn/api/scripting/VariableArityTestInterface.java new file mode 100644 index 00000000..d3904522 --- /dev/null +++ b/test/src/jdk/nashorn/api/scripting/VariableArityTestInterface.java @@ -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); +} -- GitLab