8005983: JavaAdapterFactory generated proxy classes should take extra constructor arguments at the end
authorattila
Thu, 10 Jan 2013 15:28:05 +0100
changeset 16167 d99db3541813
parent 16166 b30784442c47
child 16168 f0c208287983
8005983: JavaAdapterFactory generated proxy classes should take extra constructor arguments at the end Reviewed-by: lagergren, sundar
nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java
nashorn/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java
nashorn/test/script/sandbox/javaextend.js
nashorn/test/script/sandbox/javaextend.js.EXPECTED
--- 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