Mercurial > hg > openjdk > jigsaw > nashorn
changeset 80:5ead5333fa59
8006943: Fix order of function method arguments to be (callee, thisObject)
Reviewed-by: jlaskey, lagergren
line wrap: on
line diff
--- a/src/jdk/nashorn/internal/codegen/Attr.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/codegen/Attr.java Sat Feb 09 16:58:48 2013 +0100 @@ -237,8 +237,8 @@ functionNode.setFrame(functionNode.pushFrame()); + initCallee(functionNode); initThis(functionNode); - initCallee(functionNode); if (functionNode.isVarArg()) { initVarArg(functionNode); }
--- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java Sat Feb 09 16:58:48 2013 +0100 @@ -650,16 +650,16 @@ final String signature = new FunctionSignature(true, callee.needsCallee(), callee.getReturnType(), isVarArg ? null : callee.getParameters()).toString(); + if (callee.needsCallee()) { + new FunctionObjectCreator(CodeGenerator.this, callee).makeObject(method); + } + if (callee.isStrictMode()) { // self is undefined method.loadUndefined(Type.OBJECT); } else { // get global from scope (which is the self) globalInstance(); } - if (callee.needsCallee()) { // TODO: always true - new FunctionObjectCreator(CodeGenerator.this, callee).makeObject(method); // TODO: if callee not needed, function object is used only to pass scope (could be optimized). if neither the scope nor the function object is needed by the callee, we can pass null instead. - } - loadArgs(args, signature, isVarArg, argCount); method.invokeStatic(callee.getCompileUnit().getUnitClassName(), callee.getName(), signature); assert method.peekType().equals(callee.getReturnType()) : method.peekType() + " != " + callee.getReturnType(); @@ -1646,8 +1646,8 @@ final Class<?> rtype = fn.getReturnType().getTypeClass(); final boolean needsArguments = fn.needsArguments(); final Class<?>[] ptypes = needsArguments ? - new Class<?>[] {Object.class, ScriptFunction.class, ScriptObject.class, Object.class} : - new Class<?>[] {Object.class, ScriptFunction.class, ScriptObject.class}; + new Class<?>[] {ScriptFunction.class, Object.class, ScriptObject.class, Object.class} : + new Class<?>[] {ScriptFunction.class, Object.class, ScriptObject.class}; setCurrentCompileUnit(splitCompileUnit); splitNode.setCompileUnit(splitCompileUnit); @@ -1669,12 +1669,12 @@ splitNode.setMethodEmitter(method); final MethodEmitter caller = splitNode.getCaller(); - caller.loadThis(); if(fn.needsCallee()) { caller.loadCallee(); } else { caller.loadNull(); } + caller.loadThis(); caller.loadScope(); if (needsArguments) { caller.loadArguments();
--- a/src/jdk/nashorn/internal/codegen/CompilerConstants.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/codegen/CompilerConstants.java Sat Feb 09 16:58:48 2013 +0100 @@ -82,8 +82,12 @@ /** method name for Java method that is script entry point */ RUN_SCRIPT("runScript"), - /** this name and slot */ - THIS("this", 0), + /** + * "this" name symbol for a parameter representing ECMAScript "this" in static methods that are compiled + * representations of ECMAScript functions. It is not assigned a slot, as its position in the method signature is + * dependent on other factors (most notably, callee can precede it). + */ + THIS("this"), /** this debugger symbol */ THIS_DEBUGGER("__this__"), @@ -95,7 +99,7 @@ SCRIPT_RETURN("__return__"), /** the callee value variable when necessary */ - CALLEE("__callee__", ScriptFunction.class, 1), + CALLEE("__callee__", ScriptFunction.class), /** the varargs variable when necessary */ VARARGS("__varargs__"), @@ -127,6 +131,9 @@ /** prefix for regexps */ REGEX_PREFIX("$regex"), + /** "this" used in non-static Java methods; always in slot 0 */ + JAVA_THIS("this", 0), + /** init scope */ INIT_SCOPE("$scope", 2),
--- a/src/jdk/nashorn/internal/codegen/FunctionSignature.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/codegen/FunctionSignature.java Sat Feb 09 16:58:48 2013 +0100 @@ -25,9 +25,6 @@ package jdk.nashorn.internal.codegen; -import static jdk.nashorn.internal.codegen.CompilerConstants.CALLEE; -import static jdk.nashorn.internal.codegen.CompilerConstants.THIS; - import java.util.List; import jdk.nashorn.internal.codegen.types.Type; import jdk.nashorn.internal.ir.FunctionNode; @@ -99,34 +96,31 @@ count = isVarArg ? 1 : argTypes.length; } - int first = 0; - + if (hasCallee) { + count++; + } if (hasSelf) { count++; - first++; - } - if (hasCallee) { - count++; - first++; } paramTypes = new Type[count]; - if (hasSelf) { - paramTypes[THIS.slot()] = Type.OBJECT; + int next = 0; + if (hasCallee) { + paramTypes[next++] = Type.typeFor(ScriptFunction.class); } - if (hasCallee) { - paramTypes[CALLEE.slot()] = Type.typeFor(ScriptFunction.class); + + if (hasSelf) { + paramTypes[next++] = Type.OBJECT; } if (isVarArg) { - paramTypes[first] = Type.OBJECT_ARRAY; + paramTypes[next] = Type.OBJECT_ARRAY; } else if (argTypes != null) { - for (int i = first, j = 0; i < count; i++, j++) { - paramTypes[i] = argTypes[j]; - if (paramTypes[i].isObject()) { - paramTypes[i] = Type.OBJECT; //TODO: for now, turn java/lang/String into java/lang/Object as we aren't as specific. - } + for (int j = 0; next < count;) { + final Type type = argTypes[j++]; + // TODO: for now, turn java/lang/String into java/lang/Object as we aren't as specific. + paramTypes[next++] = type.isObject() ? Type.OBJECT : type; } } else { assert false : "isVarArgs cannot be false when argTypes are null";
--- a/src/jdk/nashorn/internal/codegen/MethodEmitter.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/codegen/MethodEmitter.java Sat Feb 09 16:58:48 2013 +0100 @@ -831,7 +831,8 @@ if (symbol.hasSlot()) { final int slot = symbol.getSlot(); debug("load symbol", symbol.getName(), " slot=", slot); - pushType(symbol.getSymbolType().load(method, slot)); + final Type type = symbol.getSymbolType().load(method, slot); + pushType(type == Type.OBJECT && symbol.isThis() ? Type.THIS : type); } else if (symbol.isParam()) { assert !symbol.isScope(); assert functionNode.isVarArg() : "Non-vararg functions have slotted parameters"; @@ -865,10 +866,21 @@ */ public MethodEmitter load(final Type type, final int slot) { debug("explicit load", type, slot); - pushType(type.load(method, slot)); + final Type loadType = type.load(method, slot); + pushType(loadType == Type.OBJECT && isThisSlot(slot) ? Type.THIS : loadType); return this; } + private boolean isThisSlot(final int slot) { + if(functionNode == null) { + return slot == CompilerConstants.JAVA_THIS.slot(); + } + final int thisSlot = functionNode.getThisNode().getSymbol().getSlot(); + assert !functionNode.needsCallee() || thisSlot == 1; // needsCallee -> thisSlot == 1 + assert functionNode.needsCallee() || thisSlot == 0; // !needsCallee -> thisSlot == 0 + return slot == thisSlot; + } + /** * Push the this object to the stack. * @@ -949,10 +961,11 @@ * @return the method emitter */ public MethodEmitter loadCallee() { - debug("load callee " + functionNode.getCalleeNode().getSymbol()); - assert functionNode.getCalleeNode().getSymbol().getSlot() != 0 : "callee has wrong slot " + functionNode.getCalleeNode().getSymbol().getSlot() + " in " + functionNode.getName(); + final Symbol calleeSymbol = functionNode.getCalleeNode().getSymbol(); + debug("load callee ", calleeSymbol); + assert calleeSymbol.getSlot() == 0 : "callee has wrong slot " + calleeSymbol.getSlot() + " in " + functionNode.getName(); - return load(functionNode.getCalleeNode().getSymbol()); + return load(calleeSymbol); } /**
--- a/src/jdk/nashorn/internal/codegen/objects/ObjectClassGenerator.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/codegen/objects/ObjectClassGenerator.java Sat Feb 09 16:58:48 2013 +0100 @@ -29,9 +29,9 @@ import static jdk.nashorn.internal.codegen.CompilerConstants.ALLOCATE; import static jdk.nashorn.internal.codegen.CompilerConstants.INIT_ARGUMENTS; import static jdk.nashorn.internal.codegen.CompilerConstants.INIT_SCOPE; +import static jdk.nashorn.internal.codegen.CompilerConstants.JAVA_THIS; import static jdk.nashorn.internal.codegen.CompilerConstants.JS_OBJECT_PREFIX; import static jdk.nashorn.internal.codegen.CompilerConstants.MAP; -import static jdk.nashorn.internal.codegen.CompilerConstants.THIS; import static jdk.nashorn.internal.codegen.CompilerConstants.className; import static jdk.nashorn.internal.codegen.CompilerConstants.constructorNoLookup; import static jdk.nashorn.internal.runtime.linker.Lookup.MH; @@ -251,7 +251,7 @@ // always initialize fields to undefined, even with --dual-fields. Then it's ok to // remember things like "widest set type" in properties, and if it's object, don't // add any special "return undefined" getters, saving an invalidation - init.load(Type.OBJECT, THIS.slot()); + init.load(Type.OBJECT, JAVA_THIS.slot()); init.loadUndefined(Type.OBJECT); final Iterator<String> iter = fieldNames.iterator(); @@ -387,7 +387,7 @@ private static MethodEmitter newInitMethod(final ClassEmitter classEmitter) { final MethodEmitter init = classEmitter.init(PropertyMap.class); init.begin(); - init.load(Type.OBJECT, THIS.slot()); + init.load(Type.OBJECT, JAVA_THIS.slot()); init.load(Type.OBJECT, MAP.slot()); init.invoke(constructorNoLookup(ScriptObject.class, PropertyMap.class)); @@ -402,7 +402,7 @@ private static MethodEmitter newInitScopeMethod(final ClassEmitter classEmitter) { final MethodEmitter init = classEmitter.init(PropertyMap.class, ScriptObject.class); init.begin(); - init.load(Type.OBJECT, THIS.slot()); + init.load(Type.OBJECT, JAVA_THIS.slot()); init.load(Type.OBJECT, MAP.slot()); init.load(Type.OBJECT, INIT_SCOPE.slot()); init.invoke(constructorNoLookup(FunctionScope.class, PropertyMap.class, ScriptObject.class)); @@ -418,7 +418,7 @@ private static MethodEmitter newInitScopeWithArgumentsMethod(final ClassEmitter classEmitter) { final MethodEmitter init = classEmitter.init(PropertyMap.class, ScriptObject.class, Object.class); init.begin(); - init.load(Type.OBJECT, THIS.slot()); + init.load(Type.OBJECT, JAVA_THIS.slot()); init.load(Type.OBJECT, MAP.slot()); init.load(Type.OBJECT, INIT_SCOPE.slot()); init.load(Type.OBJECT, INIT_ARGUMENTS.slot()); @@ -436,7 +436,7 @@ private static void newEmptyInit(final ClassEmitter classEmitter, final String className) { final MethodEmitter emptyInit = classEmitter.init(); emptyInit.begin(); - emptyInit.load(Type.OBJECT, THIS.slot()); + emptyInit.load(Type.OBJECT, JAVA_THIS.slot()); emptyInit.loadNull(); emptyInit.invoke(constructorNoLookup(className, PropertyMap.class)); emptyInit.returnVoid();
--- a/src/jdk/nashorn/internal/codegen/types/ObjectType.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/codegen/types/ObjectType.java Sat Feb 09 16:58:48 2013 +0100 @@ -74,11 +74,6 @@ public Type load(final MethodVisitor method, final int slot) { assert slot != -1; method.visitVarInsn(ALOAD, slot); - - if (slot == CompilerConstants.THIS.slot()) { - return Type.THIS; - } - return Type.OBJECT; }
--- a/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java Sat Feb 09 16:58:48 2013 +0100 @@ -30,13 +30,12 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; - -import jdk.nashorn.internal.runtime.ScriptFunctionData; import jdk.nashorn.internal.codegen.objects.FunctionObjectCreator; import jdk.nashorn.internal.runtime.GlobalFunctions; import jdk.nashorn.internal.runtime.Property; import jdk.nashorn.internal.runtime.PropertyMap; import jdk.nashorn.internal.runtime.ScriptFunction; +import jdk.nashorn.internal.runtime.ScriptFunctionData; import jdk.nashorn.internal.runtime.ScriptObject; import jdk.nashorn.internal.runtime.ScriptRuntime; import jdk.nashorn.internal.runtime.linker.Lookup;
--- a/src/jdk/nashorn/internal/runtime/Context.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/runtime/Context.java Sat Feb 09 16:58:48 2013 +0100 @@ -886,8 +886,8 @@ RUN_SCRIPT.tag(), MH.type( Object.class, - Object.class, - ScriptFunction.class)); + ScriptFunction.class, + Object.class)); boolean strict;
--- a/src/jdk/nashorn/internal/runtime/ScriptFunction.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/runtime/ScriptFunction.java Sat Feb 09 16:58:48 2013 +0100 @@ -210,7 +210,7 @@ if (data.isVarArg()) { if (data.needsCallee()) { - return invoker.invokeExact(selfObj, this, args); + return invoker.invokeExact(this, selfObj, args); } return invoker.invokeExact(selfObj, args); } @@ -219,15 +219,15 @@ if (data.needsCallee()) { switch (paramCount) { case 2: - return invoker.invokeExact(selfObj, this); + return invoker.invokeExact(this, selfObj); case 3: - return invoker.invokeExact(selfObj, this, getArg(args, 0)); + return invoker.invokeExact(this, selfObj, getArg(args, 0)); case 4: - return invoker.invokeExact(selfObj, this, getArg(args, 0), getArg(args, 1)); + return invoker.invokeExact(this, selfObj, getArg(args, 0), getArg(args, 1)); case 5: - return invoker.invokeExact(selfObj, this, getArg(args, 0), getArg(args, 1), getArg(args, 2)); + return invoker.invokeExact(this, selfObj, getArg(args, 0), getArg(args, 1), getArg(args, 2)); default: - return invoker.invokeWithArguments(withArguments(selfObj, this, paramCount, args)); + return invoker.invokeWithArguments(withArguments(selfObj, paramCount, args)); } } @@ -241,7 +241,7 @@ case 4: return invoker.invokeExact(selfObj, getArg(args, 0), getArg(args, 1), getArg(args, 2)); default: - return invoker.invokeWithArguments(withArguments(selfObj, null, paramCount, args)); + return invoker.invokeWithArguments(withArguments(selfObj, paramCount, args)); } } @@ -264,7 +264,7 @@ final MethodHandle constructor = data.getGenericConstructor(); if (data.isVarArg()) { if (data.needsCallee()) { - return constructor.invokeExact(self, this, args); + return constructor.invokeExact(this, self, args); } return constructor.invokeExact(self, args); } @@ -273,15 +273,15 @@ if (data.needsCallee()) { switch (paramCount) { case 2: - return constructor.invokeExact(self, this); + return constructor.invokeExact(this, self); case 3: - return constructor.invokeExact(self, this, getArg(args, 0)); + return constructor.invokeExact(this, self, getArg(args, 0)); case 4: - return constructor.invokeExact(self, this, getArg(args, 0), getArg(args, 1)); + return constructor.invokeExact(this, self, getArg(args, 0), getArg(args, 1)); case 5: - return constructor.invokeExact(self, this, getArg(args, 0), getArg(args, 1), getArg(args, 2)); + return constructor.invokeExact(this, self, getArg(args, 0), getArg(args, 1), getArg(args, 2)); default: - return constructor.invokeWithArguments(withArguments(self, this, args)); + return constructor.invokeWithArguments(withArguments(self, args)); } } @@ -295,31 +295,30 @@ case 4: return constructor.invokeExact(self, getArg(args, 0), getArg(args, 1), getArg(args, 2)); default: - return constructor.invokeWithArguments(withArguments(self, null, args)); + return constructor.invokeWithArguments(withArguments(self, args)); } } - private static Object[] withArguments(final Object self, final ScriptFunction function, final Object... args) { - return withArguments(self, function, args.length + (function == null ? 1 : 2), args); // + 2 to include self and function + private Object[] withArguments(final Object self, final Object[] args) { + return withArguments(self, args.length + (data.needsCallee() ? 2 : 1), args); } - private static Object[] withArguments(final Object self, final ScriptFunction function, final int paramCount, final Object... args) { - final Object[] finalArgs = new Object[paramCount]; + private Object[] withArguments(final Object self, final int argCount, final Object[] args) { + final Object[] finalArgs = new Object[argCount]; - finalArgs[0] = self; - int nextArg = 1; - if (function != null) { - finalArgs[nextArg++] = function; + int nextArg = 0; + if (data.needsCallee()) { + finalArgs[nextArg++] = this; } + finalArgs[nextArg++] = self; - //don't add more args that there is paramcount in the handle (including self) - final int maxArgs = Math.min(args.length, paramCount - (function == null ? 1 : 2)); - for (int i = 0; i < maxArgs;) { + // Don't add more args that there is argCount in the handle (including self and callee). + for (int i = 0; i < args.length && nextArg < argCount;) { finalArgs[nextArg++] = args[i++]; } - //if we have fewer params than paramcount, pad undefined - while (nextArg < paramCount) { + // If we have fewer args than argCount, pad with undefined. + while (nextArg < argCount) { finalArgs[nextArg++] = UNDEFINED; } @@ -512,10 +511,18 @@ * @return bound invoke handle */ public final MethodHandle getBoundInvokeHandle(final ScriptObject self) { - final MethodHandle bound = MH.bindTo(getInvokeHandle(), self); - return data.needsCallee() ? MH.bindTo(bound, this) : bound; + return MH.bindTo(bindToCalleeIfNeeded(getInvokeHandle()), self); } + /** + * Bind the method handle to this {@code ScriptFunction} instance if it needs a callee parameter. If this function's + * method handles don't have a callee parameter, the handle is returned unchanged. + * @param methodHandle the method handle to potentially bind to this function instance. + * @return the potentially bound method handle + */ + private MethodHandle bindToCalleeIfNeeded(final MethodHandle methodHandle) { + return data.needsCallee() ? MH.bindTo(methodHandle, this) : methodHandle; + } /** * Get the construct handle - the most generic (and if no specializations are in place, only) constructor @@ -524,7 +531,7 @@ * @param type type for wanted constructor * @return construct handle */ - public final MethodHandle getConstructHandle(final MethodType type) { + private final MethodHandle getConstructHandle(final MethodType type) { return candidateWithLowestWeight(type, getConstructHandle(), data.getConstructSpecializations()); } @@ -537,8 +544,8 @@ } /** - * Set a method handle to the constructor for this function - * @param constructHandle constructor handle + * Set a method handle to the constructor for this function. + * @param constructHandle constructor handle. Can be null to prevent the function from being used as a constructor. */ public final void setConstructHandle(final MethodHandle constructHandle) { data.setConstructor(constructHandle); @@ -674,7 +681,11 @@ @Override protected GuardedInvocation findNewMethod(final CallSiteDescriptor desc) { + // Call site type is (callee, args...) - dyn:new doesn't specify a "this", it's our job to allocate it final MethodType type = desc.getMethodType(); + + // Constructor arguments are either (callee, this, args...) or (this, args...), depending on whether it needs a + // callee or not. MethodHandle constructor = getConstructHandle(type); if (constructor == null) { @@ -682,18 +693,30 @@ return null; } - final MethodType ctorType = constructor.type(); + // If it was (callee, this, args...), permute it to (this, callee, args...). We're doing this because having + // "this" in the first argument position is what allows the elegant folded composition of + // (newFilter x constructor x allocator) further down below in the code. + constructor = swapCalleeAndThis(constructor); - // guard against primitive constructor return types - constructor = MH.asType(constructor, constructor.type().changeReturnType(Object.class)); - - // apply new filter - final Class<?>[] ctorArgs = ctorType.dropParameterTypes(0, 1).parameterArray(); // drop self + final MethodType ctorType = constructor.type(); + // Drop constructor "this", so it's also captured as "allocation" parameter of newFilter after we fold the + // constructor into newFilter. + // (this, [callee, ]args...) => ([callee, ]args...) + final Class<?>[] ctorArgs = ctorType.dropParameterTypes(0, 1).parameterArray(); + // Fold constructor into newFilter that replaces the return value from the constructor with the originally + // allocated value when the originally allocated value is a primitive. + // (result, this, [callee, ]args...) x (this, [callee, ]args...) => (this, [callee, ]args...) MethodHandle handle = MH.foldArguments(MH.dropArguments(NEWFILTER, 2, ctorArgs), constructor); + // allocate() takes a ScriptFunction and returns a newly allocated ScriptObject... if (data.needsCallee()) { + // ...we either fold it into the previous composition, if we need both the ScriptFunction callee object and + // the newly allocated object in the arguments, so (this, callee, args...) x (callee) => (callee, args...), + // or... handle = MH.foldArguments(handle, ALLOCATE); } else { + // ...replace the ScriptFunction argument with the newly allocated object, if it doesn't need the callee + // (this, args...) filter (callee) => (callee, args...) handle = MH.filterArguments(handle, 0, ALLOCATE); } @@ -701,6 +724,30 @@ return new GuardedInvocation(filterIn, null, NashornGuards.getFunctionGuard(this)); } + /** + * If this function's method handles need a callee parameter, swap the order of first two arguments for the passed + * method handle. If this function's method handles don't need a callee parameter, returns the original method + * handle unchanged. + * @param mh a method handle with order of arguments {@code (callee, this, args...)} + * @return a method handle with order of arguments {@code (this, callee, args...)} + */ + private MethodHandle swapCalleeAndThis(final MethodHandle mh) { + if (!data.needsCallee()) { + return mh; + } + final MethodType type = mh.type(); + assert type.parameterType(0) == ScriptFunction.class; + assert type.parameterType(1) == Object.class; + final MethodType newType = type.changeParameterType(0, Object.class).changeParameterType(1, ScriptFunction.class); + final int[] reorder = new int[type.parameterCount()]; + reorder[0] = 1; + assert reorder[1] == 0; + for (int i = 2; i < reorder.length; ++i) { + reorder[i] = i; + } + return MethodHandles.permuteArguments(mh, newType, reorder); + } + @SuppressWarnings("unused") private static Object newFilter(final Object result, final Object allocation) { return (result instanceof ScriptObject || !JSType.isPrimitive(result))? result : allocation; @@ -716,7 +763,7 @@ /** * dyn:call call site signature: (callee, thiz, [args...]) - * generated method signature: (thiz, callee, [args...]) + * generated method signature: (callee, thiz, [args...]) * * cases: * (a) method has callee parameter @@ -735,8 +782,10 @@ final MethodHandle collector = MH.asCollector(ScriptRuntime.APPLY.methodHandle(), Object[].class, type.parameterCount() - 2); - return new GuardedInvocation(collector, - desc.getMethodType().parameterType(0) == ScriptFunction.class ? null : NashornGuards.getScriptFunctionGuard()); + // If call site is statically typed to take a ScriptFunction, we don't need a guard, otherwise we need a + // generic "is this a ScriptFunction?" guard. + return new GuardedInvocation(collector, ScriptFunction.class.isAssignableFrom(desc.getMethodType().parameterType(0)) + ? null : NashornGuards.getScriptFunctionGuard()); } MethodHandle boundHandle; @@ -745,21 +794,15 @@ if (data.needsCallee()) { final MethodHandle callHandle = getBestSpecializedInvokeHandle(type); - if(NashornCallSiteDescriptor.isScope(desc)) { - // (this, callee, args...) => (callee, args...) => (callee, [this], args...) - boundHandle = MH.bindTo(callHandle, needsWrappedThis() ? Context.getGlobalTrusted() : ScriptRuntime.UNDEFINED); + if (NashornCallSiteDescriptor.isScope(desc)) { + // Make a handle that drops the passed "this" argument and substitutes either Global or Undefined + // (callee, this, args...) => (callee, args...) + boundHandle = MH.insertArguments(callHandle, 1, needsWrappedThis() ? Context.getGlobalTrusted() : ScriptRuntime.UNDEFINED); + // (callee, args...) => (callee, [this], args...) boundHandle = MH.dropArguments(boundHandle, 1, Object.class); } else { - // (this, callee, args...) permute => (callee, this, args...) which is what we get in - final MethodType oldType = callHandle.type(); - final int[] reorder = new int[oldType.parameterCount()]; - for (int i = 2; i < reorder.length; i++) { - reorder[i] = i; - } - reorder[0] = 1; - assert reorder[1] == 0; - final MethodType newType = oldType.changeParameterType(0, oldType.parameterType(1)).changeParameterType(1, oldType.parameterType(0)); - boundHandle = MethodHandles.permuteArguments(callHandle, newType, reorder); + // It's already (callee, this, args...), just what we need + boundHandle = callHandle; // For non-strict functions, check whether this-object is primitive type. // If so add a to-object-wrapper argument filter. @@ -775,10 +818,14 @@ } else { final MethodHandle callHandle = getBestSpecializedInvokeHandle(type.dropParameterTypes(0, 1)); - if(NashornCallSiteDescriptor.isScope(desc)) { + if (NashornCallSiteDescriptor.isScope(desc)) { + // Make a handle that drops the passed "this" argument and substitutes either Global or Undefined + // (this, args...) => (args...) boundHandle = MH.bindTo(callHandle, needsWrappedThis() ? Context.getGlobalTrusted() : ScriptRuntime.UNDEFINED); + // (args...) => ([callee], [this], args...) boundHandle = MH.dropArguments(boundHandle, 0, Object.class, Object.class); } else { + // (this, args...) => ([callee], this, args...) boundHandle = MH.dropArguments(callHandle, 0, Object.class); } } @@ -793,21 +840,11 @@ * These don't want a callee parameter, so bind that. Name binding is optional. */ MethodHandle getCallMethodHandle(final MethodType type, final String bindName) { - MethodHandle methodHandle = getBestSpecializedInvokeHandle(type); + return pairArguments(bindToNameIfNeeded(bindToCalleeIfNeeded(getBestSpecializedInvokeHandle(type)), bindName), type); + } - if (bindName != null) { - if (data.needsCallee()) { - methodHandle = MH.insertArguments(methodHandle, 1, this, bindName); - } else { - methodHandle = MH.insertArguments(methodHandle, 1, bindName); - } - } else { - if (data.needsCallee()) { - methodHandle = MH.insertArguments(methodHandle, 1, this); - } - } - - return pairArguments(methodHandle, type); + private static MethodHandle bindToNameIfNeeded(final MethodHandle methodHandle, final String bindName) { + return bindName == null ? methodHandle : MH.insertArguments(methodHandle, 1, bindName); } /**
--- a/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java Fri Feb 08 09:19:38 2013 -0400 +++ b/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java Sat Feb 09 16:58:48 2013 +0100 @@ -25,15 +25,14 @@ package jdk.nashorn.internal.runtime; +import static jdk.nashorn.internal.runtime.linker.Lookup.MH; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodType; import jdk.nashorn.internal.ir.FunctionNode; import jdk.nashorn.internal.parser.Token; import jdk.nashorn.internal.parser.TokenType; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodType; - -import static jdk.nashorn.internal.runtime.linker.Lookup.MH; - /** * A container for data needed to instantiate a specific {@link ScriptFunction} at runtime. * Instances of this class are created during codegen and stored in script classes' @@ -129,19 +128,19 @@ * constructor or not if the method handle's first argument is boolean. */ this.invoker = MH.insertArguments(methodHandle, 0, false); - this.constructor = MH.insertArguments(methodHandle, 0, true); + this.constructor = adaptConstructor(MH.insertArguments(methodHandle, 0, true)); if (specs != null) { this.invokeSpecializations = new MethodHandle[specs.length]; this.constructSpecializations = new MethodHandle[specs.length]; for (int i = 0; i < specs.length; i++) { this.invokeSpecializations[i] = MH.insertArguments(specs[i], 0, false); - this.constructSpecializations[i] = MH.insertArguments(specs[i], 0, true); + this.constructSpecializations[i] = adaptConstructor(MH.insertArguments(specs[i], 0, true)); } } } else { this.invoker = methodHandle; - this.constructor = methodHandle; + this.constructor = adaptConstructor(methodHandle); this.invokeSpecializations = specs; this.constructSpecializations = specs; } @@ -353,7 +352,7 @@ // and they're set when first called, so we enforce set-once here. if (this.invoker == null) { this.invoker = invoker; - this.constructor = invoker; + this.constructor = adaptConstructor(invoker); this.allocator = allocator; } } @@ -408,9 +407,9 @@ return false; } if(type.parameterType(0) == boolean.class) { - return len > 2 && type.parameterType(2) == ScriptFunction.class; + return len > 1 && type.parameterType(1) == ScriptFunction.class; } - return len > 1 && type.parameterType(1) == ScriptFunction.class; + return type.parameterType(0) == ScriptFunction.class; } /** @@ -434,9 +433,29 @@ newType = newType.changeParameterType(type.parameterCount() - 1, Object[].class); } if (needsCallee()) { - newType = newType.changeParameterType(1, ScriptFunction.class); + newType = newType.changeParameterType(0, ScriptFunction.class); } return type.equals(newType) ? handle : handle.asType(newType); } + /** + * Adapts a method handle to conform to requirements of a constructor. Right now this consists of making sure its + * return value is {@code Object}. We might consider moving the caller-this argument swap here too from + * {@link ScriptFunction#findNewMethod(org.dynalang.dynalink.CallSiteDescriptor)}. + * @param ctorHandle the constructor method handle + * @return adapted constructor method handle + */ + private static MethodHandle adaptConstructor(final MethodHandle ctorHandle) { + return changeReturnTypeToObject(ctorHandle); + } + + /** + * Adapts the method handle so its return type is {@code Object}. If the handle's return type is already + * {@code Object}, the handle is returned unchanged. + * @param mh the handle to adapt + * @return the adapted handle + */ + private static MethodHandle changeReturnTypeToObject(final MethodHandle mh) { + return MH.asType(mh, mh.type().changeReturnType(Object.class)); + } }