Finish forcing method name in DynamicMethod constructors.

* Add name path to all JavaMethod. * Modify generation to generate name constructor on invokers. * Use DynamicMethod.Version annotation to indicate new constructor * Add threadlocal out-of-band mechanism for old constructors.
jruby · Feb 12, 2018 · 02dbb67 · 02dbb67
1 parent 7cc201a
commit 02dbb67
Showing 10 changed files with 261 additions and 57 deletions.
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -1067,7 +1067,7 @@ public final boolean defineAnnotatedMethod(String name, List<JavaMethodDescripto
             return defineAnnotatedMethod(name, desc, methodFactory);
         }
 
-        DynamicMethod dynamicMethod = methodFactory.getAnnotatedMethod(this, methods);
+        DynamicMethod dynamicMethod = methodFactory.getAnnotatedMethod(this, methods, name);
         define(this, desc, name, dynamicMethod);
 
         return true;
@@ -1079,7 +1079,7 @@ public final boolean defineAnnotatedMethod(Method method, MethodFactory methodFa
         if (jrubyMethod == null) return false;
 
         JavaMethodDescriptor desc = new JavaMethodDescriptor(method);
-        DynamicMethod dynamicMethod = methodFactory.getAnnotatedMethod(this, desc);
+        DynamicMethod dynamicMethod = methodFactory.getAnnotatedMethod(this, desc, method.getName());
         define(this, desc, method.getName(), dynamicMethod);
 
         return true;
@@ -1090,7 +1090,7 @@ public final boolean defineAnnotatedMethod(String name, JavaMethodDescriptor des
 
         if (jrubyMethod == null) return false;
 
-        DynamicMethod dynamicMethod = methodFactory.getAnnotatedMethod(this, desc);
+        DynamicMethod dynamicMethod = methodFactory.getAnnotatedMethod(this, desc, name);
         define(this, desc, name, dynamicMethod);
 
         return true;

diff --git a/core/src/main/java/org/jruby/anno/AnnotationBinder.java b/core/src/main/java/org/jruby/anno/AnnotationBinder.java
@@ -318,7 +318,8 @@ public void processMethodDeclaration(ExecutableElement method) {
                     anno.frame());
             String implClass = anno.meta() ? "singletonClass" : "cls";
 
-            out.println("        javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility() + ");");
+            String baseName = getBaseName(anno.name(), method);
+            out.println("        javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility() + ", \"" + baseName + "\");");
             out.println("        populateMethod(javaMethod, " +
                     +AnnotationHelper.getArityValue(anno, actualRequired) + ", \""
                     + method.getSimpleName() + "\", "
@@ -364,7 +365,8 @@ public void processMethodDeclarationMulti(ExecutableElement method) {
                     anno.frame());
             String implClass = anno.meta() ? "singletonClass" : "cls";
 
-            out.println("        javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility() + ");");
+            String baseName = getBaseName(anno.name(), method);
+            out.println("        javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility() + ", \"" + baseName + "\");");
             out.println("        populateMethod(javaMethod, " +
                     "-1, \"" +
                     method.getSimpleName() + "\", " +
@@ -466,12 +468,10 @@ private void generateMethodAddCalls(ExecutableElement md, final boolean meta, fi
 
     private void defineMethodOnClass(String methodVar, String classVar, final String[] names, final String[] aliases,
         ExecutableElement md) {
-        CharSequence baseName;
+        CharSequence baseName = getBaseName(names, md);
         if (names.length == 0) {
-            baseName = md.getSimpleName();
             out.println("        " + classVar + ".addMethodAtBootTimeOnly(\"" + baseName + "\", " + methodVar + ");");
         } else {
-            baseName = names[0];
             for (String name : names) {
                 out.println("        " + classVar + ".addMethodAtBootTimeOnly(\"" + name + "\", " + methodVar + ");");
             }
@@ -483,4 +483,12 @@ private void defineMethodOnClass(String methodVar, String classVar, final String
             }
         }
     }
+
+    private String getBaseName(String[] names, ExecutableElement md) {
+        if (names.length == 0) {
+            return md.getSimpleName().toString();
+        } else {
+            return names[0];
+        }
+    }
 }
diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/DynamicMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/DynamicMethod.java
@@ -30,6 +30,10 @@
 
 package org.jruby.internal.runtime.methods;
 
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import org.jruby.MetaClass;
@@ -72,6 +76,11 @@ public abstract class DynamicMethod {
     private static final int BUILTIN_FLAG = 0x1;
     private static final int NOTIMPL_FLAG = 0x2;
 
+    @Retention(RetentionPolicy.RUNTIME)
+    public @interface Version {
+        int version = 0;
+    }
+
     /**
      * Base constructor for dynamic method handles with names.
      *
@@ -82,7 +91,9 @@ public abstract class DynamicMethod {
      */
     protected DynamicMethod(RubyModule implementationClass, Visibility visibility, String name) {
         assert implementationClass != null;
-        assert name != null;
+        if (name == null) {
+            name = "null";
+        }
         this.name = name;
         init(implementationClass, visibility);
     }
@@ -93,10 +104,10 @@ protected DynamicMethod(RubyModule implementationClass, Visibility visibility, S
      */
     protected DynamicMethod(String name) {
         this.visibility = (byte) Visibility.PUBLIC.ordinal();
+        if (name == null) {
+            name = "null";
+        }
         this.name = name;
-//        assert (this instanceof UndefinedMethod ||
-//                this instanceof CompiledMethod ||
-//                this instanceof );
     }
 
     protected void init(RubyModule implementationClass, Visibility visibility) {

diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/InvocationMethodFactory.java b/core/src/main/java/org/jruby/internal/runtime/methods/InvocationMethodFactory.java
@@ -46,6 +46,7 @@
 import org.jruby.util.CodegenUtils;
 import org.jruby.util.log.Logger;
 import org.jruby.util.log.LoggerFactory;
+import org.objectweb.asm.AnnotationVisitor;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.Label;
@@ -122,7 +123,7 @@ public class InvocationMethodFactory extends MethodFactory implements Opcodes {
             params(ThreadContext.class, IRubyObject.class, RubyModule.class, String.class, IRubyObject.class, IRubyObject.class, IRubyObject.class));
 
     /** The super constructor signature for Java-based method handles. */
-    private final static String JAVA_SUPER_SIG = sig(Void.TYPE, params(RubyModule.class, Visibility.class));
+    private final static String JAVA_SUPER_SIG = sig(Void.TYPE, params(RubyModule.class, Visibility.class, String.class));
 
     /** The lvar index of "this" */
     public static final int THIS_INDEX = 0;
@@ -187,15 +188,15 @@ public InvocationMethodFactory(ClassLoader classLoader) {
     //    return signature.isFixed() && signature.required() <= 3;
     //}
 
-    private static final Class[] RubyModule_and_Visibility = new Class[]{ RubyModule.class, Visibility.class };
+    private static final Class[] RubyModule_and_Visibility_and_Name = new Class[]{ RubyModule.class, Visibility.class, String.class };
 
     /**
      * Use code generation to provide a method handle based on an annotated Java
      * method.
      *
      * @see org.jruby.runtime.MethodFactory#getAnnotatedMethod
      */
-    public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, List<JavaMethodDescriptor> descs) {
+    public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, List<JavaMethodDescriptor> descs, String name) {
         JavaMethodDescriptor desc1 = descs.get(0);
         final JRubyMethod anno = desc1.anno;
         final String javaMethodName = desc1.name;
@@ -208,7 +209,7 @@ public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, List<Jav
             DescriptorInfo info = new DescriptorInfo(descs);
             if (DEBUG) LOG.debug(" min: " + info.getMin() + ", max: " + info.getMax());
 
-            JavaMethod ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility).newInstance(implementationClass, anno.visibility());
+            JavaMethod ic = constructJavaMethod(implementationClass, desc1, name, c);
 
             TypePopulator.populateMethod(
                     ic,
@@ -318,13 +319,12 @@ private static Class determineSuperclass(DescriptorInfo info) {
      *
      * @see org.jruby.runtime.MethodFactory#getAnnotatedMethod
      */
-    public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMethodDescriptor desc) {
+    public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMethodDescriptor desc, String name) {
         String javaMethodName = desc.name;
 
         try {
             Class c = getAnnotatedMethodClass(Collections.singletonList(desc));
-
-            JavaMethod ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility).newInstance(implementationClass, desc.anno.visibility());
+            JavaMethod ic = constructJavaMethod(implementationClass, desc, name, c);
 
             TypePopulator.populateMethod(
                     ic,
@@ -343,6 +343,27 @@ public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMeth
         }
     }
 
+    public JavaMethod constructJavaMethod(RubyModule implementationClass, JavaMethodDescriptor desc, String name, Class c) throws InstantiationException, IllegalAccessException, java.lang.reflect.InvocationTargetException, NoSuchMethodException {
+        // In order to support older versions of generated JavaMethod invokers, we check for the Version
+        // annotation to be present and > 0. If absent, we use a thread local to allow the deprecated constructor
+        // to still provide a final method name.
+        DynamicMethod.Version version = (DynamicMethod.Version) c.getAnnotation(DynamicMethod.Version.class);
+        JavaMethod ic;
+        if (version == null) {
+            // Old constructor with no name, use thread-local to pass it.
+            JavaMethod.NAME_PASSER.set(name);
+            try {
+                ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility).newInstance(implementationClass, desc.anno.visibility());
+            } finally {
+                JavaMethod.NAME_PASSER.remove();
+            }
+        } else {
+            // New constructor with name.
+            ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility_and_Name).newInstance(implementationClass, desc.anno.visibility(), name);
+        }
+        return ic;
+    }
+
     /**
      * Emit code to check the arity of a call to a Java-based method.
      *
@@ -417,9 +438,14 @@ private static ClassWriter createJavaMethodCtor(String namePath, String sup, Str
         String sourceFile = namePath.substring(namePath.lastIndexOf('/') + 1) + ".gen";
         cw.visit(RubyInstanceConfig.JAVA_VERSION, ACC_PUBLIC + ACC_SUPER, namePath, null, sup, null);
         cw.visitSource(sourceFile, null);
+
+        AnnotationVisitor av = cw.visitAnnotation(ci(DynamicMethod.Version.class), true);
+        av.visit("version", 0);
+        av.visitEnd();
+
         SkinnyMethodAdapter mv = new SkinnyMethodAdapter(cw, ACC_PUBLIC, "<init>", JAVA_SUPER_SIG, null, null);
         mv.start();
-        mv.aloadMany(0, 1, 2);
+        mv.aloadMany(0, 1, 2, 3);
         mv.invokespecial(sup, "<init>", JAVA_SUPER_SIG);
         mv.aload(0);
         mv.ldc(parameterDesc);
@@ -818,4 +844,7 @@ private static void invokeCReturnTrace(SkinnyMethodAdapter method, int traceBool
         method.aload(4); // invokedName
         method.invokevirtual(p(JavaMethod.class), "returnTrace", sig(void.class, ThreadContext.class, boolean.class, String.class));
     }
+
+    @Deprecated
+    private static final Class[] RubyModule_and_Visibility = new Class[]{ RubyModule.class, Visibility.class };
 }
diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/InvokeDynamicMethodFactory.java b/core/src/main/java/org/jruby/internal/runtime/methods/InvokeDynamicMethodFactory.java
@@ -72,15 +72,15 @@ public InvokeDynamicMethodFactory(ClassLoader classLoader) {
     }
 
     @Override
-    public DynamicMethod getAnnotatedMethod(final RubyModule implementationClass, final List<JavaMethodDescriptor> descs) {
+    public DynamicMethod getAnnotatedMethod(final RubyModule implementationClass, final List<JavaMethodDescriptor> descs, String name) {
         JavaMethodDescriptor desc1 = descs.get(0);
         DescriptorInfo info = new DescriptorInfo(desc1);
 
         if (desc1.anno.frame()) {
             // super logic does not work yet because we need to take impl class
             // and method name from the DynamicMethod#call call, so punt to
             // generated class for now
-            return super.getAnnotatedMethod(implementationClass, descs);
+            return super.getAnnotatedMethod(implementationClass, descs, name);
         }
 
         if (!Modifier.isPublic(desc1.getDeclaringClass().getModifiers())) {
@@ -315,8 +315,8 @@ public MethodHandle call() throws Exception {
      * @see org.jruby.runtime.MethodFactory#getAnnotatedMethod
      */
     @Override
-    public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMethodDescriptor desc) {
-        return getAnnotatedMethod(implementationClass, Collections.singletonList(desc));
+    public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMethodDescriptor desc, String name) {
+        return getAnnotatedMethod(implementationClass, Collections.singletonList(desc), name);
     }
 
     public static final Signature VARIABLE_ARITY_SIGNATURE = Signature

diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/JavaMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/JavaMethod.java
diff --git a/core/src/main/java/org/jruby/java/proxies/JavaInterfaceTemplate.java b/core/src/main/java/org/jruby/java/proxies/JavaInterfaceTemplate.java
@@ -376,8 +376,8 @@ else if ( args.length == 1 && args[0] instanceof RubyBoolean ) {
 
         final Class<?> ifaceClass = JavaClass.getJavaClass(context, ((RubyModule) self));
         if ( methodNames == null ) {
-            final BlockInterfaceImpl.ConcreteMethod implMethod = ifaceImpl.getConcreteMethod();
             for ( Method method : ifaceClass.getMethods() ) {
+                BlockInterfaceImpl.ConcreteMethod implMethod = ifaceImpl.getConcreteMethod(method.getName());
                 if ( method.isBridge() || method.isSynthetic() ) continue;
                 if ( Modifier.isStatic( method.getModifiers() ) ) continue;
                 // override default methods (by default) - users should pass down method names or impl(false) { ... }
@@ -386,10 +386,10 @@ else if ( args.length == 1 && args[0] instanceof RubyBoolean ) {
             }
         }
         else {
-            final BlockInterfaceImpl.ConcreteMethod implMethod = ifaceImpl.getConcreteMethod();
             final Method[] decMethods = ifaceClass.getDeclaredMethods();
             loop: for ( IRubyObject methodName : methodNames ) {
                 final String name = methodName.toString();
+                final BlockInterfaceImpl.ConcreteMethod implMethod = ifaceImpl.getConcreteMethod(name);
                 for ( int i = 0; i < decMethods.length; i++ ) {
                     final Method method = decMethods[i];
                     if ( method.isBridge() || method.isSynthetic() ) continue;
@@ -414,7 +414,7 @@ private static final class BlockInterfaceImpl extends JavaMethod {
         private final Block implBlock;
 
         BlockInterfaceImpl(final RubyClass implClass, final Block implBlock, final IRubyObject[] methodNames) {
-            super(implClass, Visibility.PUBLIC);
+            super(implClass, Visibility.PUBLIC, "method_missing");
             this.implBlock = implBlock; this.methodNames = methodNames;
         }
 
@@ -460,12 +460,12 @@ public final IRubyObject call(ThreadContext context, IRubyObject self, RubyModul
 
         public DynamicMethod dup() { return this; }
 
-        final ConcreteMethod getConcreteMethod() { return new ConcreteMethod(); }
+        final ConcreteMethod getConcreteMethod(String name) { return new ConcreteMethod(name); }
 
         private final class ConcreteMethod extends JavaMethod {
 
-            ConcreteMethod() {
-                super(BlockInterfaceImpl.this.implementationClass, Visibility.PUBLIC);
+            ConcreteMethod(String name) {
+                super(BlockInterfaceImpl.this.implementationClass, Visibility.PUBLIC, name);
             }
 
             @Override

diff --git a/core/src/main/java/org/jruby/javasupport/Java.java b/core/src/main/java/org/jruby/javasupport/Java.java
@@ -1120,12 +1120,12 @@ public DynamicMethod dup() {
             return this;
         }
 
-        final ConcreteMethod getConcreteMethod() { return new ConcreteMethod(); }
+        final ConcreteMethod getConcreteMethod(String name) { return new ConcreteMethod(name); }
 
         final class ConcreteMethod extends org.jruby.internal.runtime.methods.JavaMethod {
 
-            ConcreteMethod() {
-                super(ProcToInterface.this.implementationClass, Visibility.PUBLIC);
+            ConcreteMethod(String name) {
+                super(ProcToInterface.this.implementationClass, Visibility.PUBLIC, name);
             }
 
             @Override

diff --git a/core/src/main/java/org/jruby/javasupport/JavaUtil.java b/core/src/main/java/org/jruby/javasupport/JavaUtil.java
@@ -251,9 +251,9 @@ public static <T> T convertProcToInterface(ThreadContext context, RubyBasicObjec
             singletonClass.addMethod("method_missing", procToIface);
             // similar to Iface.impl { ... } - bind interface method(s) to avoid Java-Ruby conflicts
             // ... e.g. calling a Ruby implemented Predicate#test should not dispatch to Kernel#test
-            final Java.ProcToInterface.ConcreteMethod implMethod = procToIface.getConcreteMethod();
             // getMethods for interface returns all methods (including ones from super-interfaces)
             for ( Method method : targetType.getMethods() ) {
+                Java.ProcToInterface.ConcreteMethod implMethod = procToIface.getConcreteMethod(method.getName());
                 if ( Modifier.isAbstract(method.getModifiers()) ) {
                     singletonClass.addMethodInternal(method.getName(), implMethod);
                 }

diff --git a/core/src/main/java/org/jruby/runtime/MethodFactory.java b/core/src/main/java/org/jruby/runtime/MethodFactory.java
@@ -77,7 +77,7 @@ public static MethodFactory createFactory(ClassLoader classLoader) {
      * will be bound.
      * @return A method handle for the target object.
      */
-    public abstract DynamicMethod getAnnotatedMethod(RubyModule implementationClass, List<JavaMethodDescriptor> desc);
+    public abstract DynamicMethod getAnnotatedMethod(RubyModule implementationClass, List<JavaMethodDescriptor> desc, String name);
 
     /**
      * Based on an annotated Java method object, generate a method handle using
@@ -90,5 +90,5 @@ public static MethodFactory createFactory(ClassLoader classLoader) {
      * @param desc A JavaMethodDescriptor describing the target method
      * @return A method handle for the target object.
      */
-    public abstract DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMethodDescriptor desc);
+    public abstract DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMethodDescriptor desc, String name);
 }