8005983: JavaAdapterFactory generated proxy classes should take extra constructor arguments at the end
Reviewed-by: lagergren, sundar
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java Thu Jan 10 19:55:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java Thu Jan 10 15:28:05 2013 +0100
@@ -91,7 +91,7 @@
* For every protected or public constructor in the extended class (which is either the original class, or Object when
* an interface is implemented), the adapter class will have one or two public constructors (visibility of protected
* constructors in the extended class is promoted to public). In every case, for every original constructor, a new
- * constructor taking an initial ScriptObject argument followed by original constructor arguments is present on the
+ * constructor taking a trailing ScriptObject argument preceded by original constructor arguments is present on the
* adapter class. When such a constructor is invoked, the passed ScriptObject's member functions are used to implement
* and/or override methods on the original class, dispatched by name. A single JavaScript function will act as the
* implementation for all overloaded methods of the same name. When methods on an adapter instance are invoked, the
@@ -106,17 +106,24 @@
* </p><p>
* For abstract classes or interfaces that only have one abstract method, or have several of them, but all share the
* same name, an additional constructor is provided for every original constructor; this one takes a ScriptFunction as
- * its first argument followed by original constructor arguments. This constructor will use the passed function as the
+ * its last argument preceded by original constructor arguments. This constructor will use the passed function as the
* implementation for all abstract methods. For consistency, any concrete methods sharing the single abstract method
* name will also be overridden by the function. When methods on the adapter instance are invoked, the ScriptFunction is
* invoked with {@code null} as its "this".
- * </p><p>If the superclass has a protected or public default constructor, then a generated constructor that only takes
- * a ScriptFunction is also implicitly used as an automatic conversion whenever a ScriptFunction is passed in an
+ * </p><p>
+ * If the superclass has a protected or public default constructor, then a generated constructor that only takes a
+ * ScriptFunction is also implicitly used as an automatic conversion whenever a ScriptFunction is passed in an
* invocation of any Java method that expects such SAM type.
* </p><p>
* For adapter methods that return values, all the JavaScript-to-Java conversions supported by Nashorn will be in effect
* to coerce the JavaScript function return value to the expected Java return type.
* </p><p>
+ * Since we are adding a trailing argument to the generated constructors in the adapter class, they will never be
+ * declared as variable arity, even if the original constructor in the superclass was declared as variable arity. The
+ * reason we are passing the additional argument at the end of the argument list instead at the front is that the
+ * source-level script expression <code>new X(a, b) { ... }</code> (which is a proprietary syntax extension Nashorn uses
+ * to resemble Java anonymous classes) is actually equivalent to <code>new X(a, b, { ... })</code>.
+ * </p><p>
* You normally don't use this class directly, but rather either create adapters from script using
* {@link NativeJava#extend(Object, Object)}, using the {@code new} operator on abstract classes and interfaces (see
* {@link NativeJava#type(Object, Object)}), or implicitly when passing script functions to Java methods expecting SAM
@@ -504,12 +511,15 @@
* {@link #getHandle(ScriptFunction, MethodType, boolean)} or {@link #getHandle(ScriptObject, String, MethodType,
* boolean)} to obtain the method handles; these methods make sure to add the necessary conversions and arity
* adjustments so that the resulting method handles can be invoked from generated methods using {@code invokeExact}.
- * The constructor that takes a script function will only initialize the abstract methods
- * The constructor will also store the Nashorn global that was current at the constructor invocation time in a
- * field named "global". The generated constructor will be public, regardless of whether the supertype constructor
- * was public or protected.
+ * The constructor that takes a script function will only initialize the methods with the same name as the single
+ * abstract method. The constructor will also store the Nashorn global that was current at the constructor
+ * invocation time in a field named "global". The generated constructor will be public, regardless of whether the
+ * supertype constructor was public or protected. The generated constructor will not be variable arity, even if the
+ * supertype constructor was.
* @param ctor the supertype constructor that is serving as the base for the generated constructor.
- * @param fromFunction true if a
+ * @param fromFunction true if we're generating a constructor that initializes SAM types from a single
+ * ScriptFunction passed to it, false if we're generating a constructor that initializes an arbitrary type from a
+ * ScriptObject passed to it.
*/
private void generateConstructor(final Constructor<?> ctor, final boolean fromFunction) {
final Type originalCtorType = Type.getType(ctor);
@@ -517,23 +527,22 @@
final int argLen = originalArgTypes.length;
final Type[] newArgTypes = new Type[argLen + 1];
- // Insert ScriptFunction|ScriptObject as the frontmost argument to the constructor
+ // Insert ScriptFunction|ScriptObject as the last argument to the constructor
final Type extraArgumentType = fromFunction ? SCRIPT_FUNCTION_TYPE : SCRIPT_OBJECT_TYPE;
- newArgTypes[0] = extraArgumentType;
- System.arraycopy(originalArgTypes, 0, newArgTypes, 1, argLen);
+ newArgTypes[argLen] = extraArgumentType;
+ System.arraycopy(originalArgTypes, 0, newArgTypes, 0, argLen);
// All constructors must be public, even if in the superclass they were protected.
// Existing super constructor <init>(this, args...) triggers generating <init>(this, scriptObj, args...).
- final InstructionAdapter mv = new InstructionAdapter(cw.visitMethod(ACC_PUBLIC | (ctor.isVarArgs() ?
- ACC_VARARGS : 0), INIT, Type.getMethodDescriptor(originalCtorType.getReturnType(), newArgTypes),
- null, null));
+ final InstructionAdapter mv = new InstructionAdapter(cw.visitMethod(ACC_PUBLIC, INIT,
+ Type.getMethodDescriptor(originalCtorType.getReturnType(), newArgTypes), null, null));
mv.visitCode();
- // First, invoke super constructor with shifted arguments. If the form of the constructor we're generating is
- // <init>(this, scriptFn, args...), then we're invoking super.<init>(this, args...).
+ // First, invoke super constructor with original arguments. If the form of the constructor we're generating is
+ // <init>(this, args..., scriptFn), then we're invoking super.<init>(this, args...).
mv.visitVarInsn(ALOAD, 0);
final Class<?>[] argTypes = ctor.getParameterTypes();
- int offset = 2; // First arg is at position 2, after this and scriptFn.
+ int offset = 1; // First arg is at position 1, after this.
for (int i = 0; i < argLen; ++i) {
final Type argType = Type.getType(argTypes[i]);
mv.load(offset, argType);
@@ -553,7 +562,7 @@
// is a deliberate design choice. All other method handles are initialized to null.
mv.visitInsn(ACONST_NULL);
} else {
- mv.visitVarInsn(ALOAD, 1);
+ mv.visitVarInsn(ALOAD, offset);
if(!fromFunction) {
mv.aconst(mi.getName());
}
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java Thu Jan 10 19:55:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java Thu Jan 10 15:28:05 2013 +0100
@@ -85,24 +85,49 @@
@Override
public GuardedInvocation convertToType(final Class<?> sourceType, final Class<?> targetType) throws Exception {
+ final GuardedInvocation gi = convertToTypeNoCast(sourceType, targetType);
+ return gi == null ? null : gi.asType(MH.type(targetType, sourceType));
+ }
+
+ /**
+ * Main part of the implementation of {@link GuardingTypeConverterFactory#convertToType(Class, Class)} that doesn't
+ * care about adapting the method signature; that's done by the invoking method. Returns either a built-in
+ * conversion to primitive (or primitive wrapper) Java types or to String, or a just-in-time generated converter to
+ * a SAM type (if the target type is a SAM type).
+ * @param sourceType the source type
+ * @param targetType the target type
+ * @return a guarded invocation that converts from the source type to the target type.
+ * @throws Exception if something goes wrong
+ */
+ private static GuardedInvocation convertToTypeNoCast(final Class<?> sourceType, final Class<?> targetType) throws Exception {
final MethodHandle mh = JavaArgumentConverters.getConverter(targetType);
- final GuardedInvocation gi;
if (mh != null) {
- gi = new GuardedInvocation(mh, canLinkTypeStatic(sourceType) ? null : IS_NASHORN_OR_UNDEFINED_TYPE);
- } else if (isAutoConvertibleFromFunction(targetType)) {
+ return new GuardedInvocation(mh, canLinkTypeStatic(sourceType) ? null : IS_NASHORN_OR_UNDEFINED_TYPE);
+ }
+ return getSamTypeConverter(sourceType, targetType);
+ }
+
+ /**
+ * Returns a guarded invocation that converts from a source type that is ScriptFunction, or a subclass or a
+ * superclass of it) to a SAM type.
+ * @param sourceType the source type (presumably ScriptFunction or a subclass or a superclass of it)
+ * @param targetType the target type (presumably a SAM type)
+ * @return a guarded invocation that converts from the source type to the target SAM type. null is returned if
+ * either the source type is neither ScriptFunction, nor a subclass, nor a superclass of it, or if the target type
+ * is not a SAM type.
+ * @throws Exception if something goes wrong; generally, if there's an issue with creation of the SAM proxy type
+ * constructor.
+ */
+ private static GuardedInvocation getSamTypeConverter(final Class<?> sourceType, final Class<?> targetType) throws Exception {
+ // If source type is more generic than ScriptFunction class, we'll need to use a guard
+ final boolean isSourceTypeGeneric = sourceType.isAssignableFrom(ScriptFunction.class);
+
+ if ((isSourceTypeGeneric || ScriptFunction.class.isAssignableFrom(sourceType)) && isAutoConvertibleFromFunction(targetType)) {
final MethodHandle ctor = JavaAdapterFactory.getConstructor(ScriptFunction.class, targetType);
- assert ctor != null; // if JavaAdapterFactory.isAutoConvertible() returned true, then ctor must exist.
- if(ScriptFunction.class.isAssignableFrom(sourceType)) {
- gi = new GuardedInvocation(ctor, null);
- } else if(sourceType == Object.class) {
- gi = new GuardedInvocation(ctor, IS_SCRIPT_FUNCTION);
- } else {
- return null;
- }
- } else {
- return null;
+ assert ctor != null; // if isAutoConvertibleFromFunction() returned true, then ctor must exist.
+ return new GuardedInvocation(ctor, isSourceTypeGeneric ? IS_SCRIPT_FUNCTION : null);
}
- return gi.asType(MH.type(targetType, sourceType));
+ return null;
}
private static boolean isAutoConvertibleFromFunction(final Class<?> clazz) {
--- a/nashorn/test/script/sandbox/javaextend.js Thu Jan 10 19:55:38 2013 +0530
+++ b/nashorn/test/script/sandbox/javaextend.js Thu Jan 10 15:28:05 2013 +0100
@@ -98,5 +98,9 @@
// additional constructor arguments (a token). Also demonstrates how can
// you access the Java adapter instance from the script (just store it in the
// scope, in this example, "cwa") to retrieve the token later on.
-var cwa = new (Java.extend(model("ConstructorWithArgument")))(function() { print(cwa.token) }, "cwa-token")
+var cwa = new (Java.extend(model("ConstructorWithArgument")))("cwa-token", function() { print(cwa.token) })
cwa.doSomething()
+
+// Do the same thing with proprietary syntax and object literal
+var cwa2 = new (model("ConstructorWithArgument"))("cwa2-token") { doSomething: function() { print("cwa2-" + cwa2.token ) } }
+cwa2.doSomething()
--- a/nashorn/test/script/sandbox/javaextend.js.EXPECTED Thu Jan 10 19:55:38 2013 +0530
+++ b/nashorn/test/script/sandbox/javaextend.js.EXPECTED Thu Jan 10 15:28:05 2013 +0100
@@ -17,3 +17,4 @@
oo-proto-overridden-toString: override-object
oo-proto-overridden-equals : true
cwa-token
+cwa2-cwa2-token