Painless: Add PainlessConstructor (#32447)

PainlessMethod was being used as both a method and a constructor, and while there are 
similarities, there are also some major differences. This allows the reflection objects to be 
stored reducing the number of other pieces of data stored in a PainlessMethod as they are 
now redundant. This temporarily increases some of the code in FunctionRef and 
PainlessDocGenerator as they now differentiate between constructors and methods, BUT 
is also makes the code more maintainable because there aren't checks in several places 
anymore to differentiate.

modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java

@@ -368,7 +368,7 @@ private static MethodHandle lookupReferenceInternal(PainlessLookup painlessLooku
              ref = new FunctionRef(clazz, interfaceMethod, call, handle.type(), captures.length);
          } else {
              // whitelist lookup
-             ref = new FunctionRef(painlessLookup, clazz, type, call, captures.length);
+             ref = FunctionRef.resolveFromLookup(painlessLookup, clazz, type, call, captures.length);
          }
          final CallSite callSite = LambdaBootstrap.lambdaBootstrap(
              methodHandlesLookup,

modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java

@@ -20,13 +20,16 @@
 package org.elasticsearch.painless;
 
 import org.elasticsearch.painless.lookup.PainlessClass;
+import org.elasticsearch.painless.lookup.PainlessConstructor;
 import org.elasticsearch.painless.lookup.PainlessLookup;
 import org.elasticsearch.painless.lookup.PainlessLookupUtility;
 import org.elasticsearch.painless.lookup.PainlessMethod;
 import org.objectweb.asm.Type;
 
 import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Modifier;
+import java.util.List;
 
 import static org.elasticsearch.painless.WriterConstants.CLASS_NAME;
 import static org.objectweb.asm.Opcodes.H_INVOKEINTERFACE;
@@ -59,8 +62,10 @@ public class FunctionRef {
 
     /** interface method */
     public final PainlessMethod interfaceMethod;
-    /** delegate method */
-    public final PainlessMethod delegateMethod;
+    /** delegate method type parameters */
+    public final List<Class<?>> delegateTypeParameters;
+    /** delegate method return type */
+    public final Class<?> delegateReturnType;
 
     /** factory method type descriptor */
     public final String factoryDescriptor;
@@ -80,9 +85,47 @@ public class FunctionRef {
      * @param call the right hand side of a method reference expression
      * @param numCaptures number of captured arguments
      */
-    public FunctionRef(PainlessLookup painlessLookup, Class<?> expected, String type, String call, int numCaptures) {
-        this(expected, painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod,
-                lookup(painlessLookup, expected, type, call, numCaptures > 0), numCaptures);
+    public static FunctionRef resolveFromLookup(
+            PainlessLookup painlessLookup, Class<?> expected, String type, String call, int numCaptures) {
+
+        if ("new".equals(call)) {
+            return new FunctionRef(expected, painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod,
+                    lookup(painlessLookup, expected, type), numCaptures);
+        } else {
+            return new FunctionRef(expected, painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod,
+                    lookup(painlessLookup, expected, type, call, numCaptures > 0), numCaptures);
+        }
+    }
+
+    /**
+     * Creates a new FunctionRef (already resolved)
+     * @param expected functional interface type to implement
+     * @param interfaceMethod functional interface method
+     * @param delegateConstructor implementation constructor
+     * @param numCaptures number of captured arguments
+     */
+    public FunctionRef(Class<?> expected, PainlessMethod interfaceMethod, PainlessConstructor delegateConstructor, int numCaptures) {
+        Constructor<?> javaConstructor = delegateConstructor.javaConstructor;
+        MethodType delegateMethodType = delegateConstructor.methodType;
+
+        interfaceMethodName = interfaceMethod.name;
+        factoryMethodType = MethodType.methodType(expected,
+                delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount()));
+        interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1);
+
+        delegateClassName = javaConstructor.getDeclaringClass().getName();
+        isDelegateInterface = false;
+        delegateInvokeType = H_NEWINVOKESPECIAL;
+        delegateMethodName = PainlessLookupUtility.CONSTRUCTOR_NAME;
+        this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures);
+
+        this.interfaceMethod = interfaceMethod;
+        delegateTypeParameters = delegateConstructor.typeParameters;
+        delegateReturnType = void.class;
+
+        factoryDescriptor = factoryMethodType.toMethodDescriptorString();
+        interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString());
+        delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString());
     }
 
     /**
@@ -112,9 +155,7 @@ public FunctionRef(Class<?> expected, PainlessMethod interfaceMethod, PainlessMe
             isDelegateInterface = delegateMethod.target.isInterface();
         }
 
-        if ("<init>".equals(delegateMethod.name)) {
-            delegateInvokeType = H_NEWINVOKESPECIAL;
-        } else if (Modifier.isStatic(delegateMethod.modifiers)) {
+        if (Modifier.isStatic(delegateMethod.modifiers)) {
             delegateInvokeType = H_INVOKESTATIC;
         } else if (delegateMethod.target.isInterface()) {
             delegateInvokeType = H_INVOKEINTERFACE;
@@ -126,7 +167,8 @@ public FunctionRef(Class<?> expected, PainlessMethod interfaceMethod, PainlessMe
         this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures);
 
         this.interfaceMethod = interfaceMethod;
-        this.delegateMethod = delegateMethod;
+        delegateTypeParameters = delegateMethod.arguments;
+        delegateReturnType = delegateMethod.rtn;
 
         factoryDescriptor = factoryMethodType.toMethodDescriptorString();
         interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString());
@@ -151,13 +193,37 @@ public FunctionRef(Class<?> expected,
         isDelegateInterface = false;
 
         this.interfaceMethod = null;
-        delegateMethod = null;
+        delegateTypeParameters = null;
+        delegateReturnType = null;
 
         factoryDescriptor = null;
         interfaceType = null;
         delegateType = null;
     }
 
+    /**
+     * Looks up {@code type} from the whitelist, and returns a matching constructor.
+     */
+    private static PainlessConstructor lookup(PainlessLookup painlessLookup, Class<?> expected, String type) {
+        // check its really a functional interface
+        // for e.g. Comparable
+        PainlessMethod method = painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod;
+        if (method == null) {
+            throw new IllegalArgumentException("Cannot convert function reference [" + type + "::new] " +
+                    "to [" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "], not a functional interface");
+        }
+
+        // lookup requested constructor
+        PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(painlessLookup.getJavaClassFromPainlessType(type));
+        PainlessConstructor impl = struct.constructors.get(PainlessLookupUtility.buildPainlessConstructorKey(method.arguments.size()));
+
+        if (impl == null) {
+            throw new IllegalArgumentException("Unknown reference [" + type + "::new] matching [" + expected + "]");
+        }
+
+        return impl;
+    }
+
     /**
      * Looks up {@code type::call} from the whitelist, and returns a matching method.
      */
@@ -174,27 +240,22 @@ private static PainlessMethod lookup(PainlessLookup painlessLookup, Class<?> exp
         // lookup requested method
         PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(painlessLookup.getJavaClassFromPainlessType(type));
         final PainlessMethod impl;
-        // ctor ref
-        if ("new".equals(call)) {
-            impl = struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("<init>", method.arguments.size()));
-        } else {
-            // look for a static impl first
-            PainlessMethod staticImpl =
-                    struct.staticMethods.get(PainlessLookupUtility.buildPainlessMethodKey(call, method.arguments.size()));
-            if (staticImpl == null) {
-                // otherwise a virtual impl
-                final int arity;
-                if (receiverCaptured) {
-                    // receiver captured
-                    arity = method.arguments.size();
-                } else {
-                    // receiver passed
-                    arity = method.arguments.size() - 1;
-                }
-                impl = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey(call, arity));
+        // look for a static impl first
+        PainlessMethod staticImpl =
+                struct.staticMethods.get(PainlessLookupUtility.buildPainlessMethodKey(call, method.arguments.size()));
+        if (staticImpl == null) {
+            // otherwise a virtual impl
+            final int arity;
+            if (receiverCaptured) {
+                // receiver captured
+                arity = method.arguments.size();
             } else {
-                impl = staticImpl;
+                // receiver passed
+                arity = method.arguments.size() - 1;
             }
+            impl = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey(call, arity));
+        } else {
+            impl = staticImpl;
         }
         if (impl == null) {
             throw new IllegalArgumentException("Unknown reference [" + type + "::" + call + "] matching " +

modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java

@@ -24,7 +24,8 @@
 import java.util.Map;
 
 public final class PainlessClass {
-    public final Map<String, PainlessMethod> constructors;
+    public final Map<String, PainlessConstructor> constructors;
+
     public final Map<String, PainlessMethod> staticMethods;
     public final Map<String, PainlessMethod> methods;
 
@@ -36,13 +37,14 @@ public final class PainlessClass {
 
     public final PainlessMethod functionalMethod;
 
-    PainlessClass(Map<String, PainlessMethod> constructors,
+    PainlessClass(Map<String, PainlessConstructor> constructors,
             Map<String, PainlessMethod> staticMethods, Map<String, PainlessMethod> methods,
             Map<String, PainlessField> staticFields, Map<String, PainlessField> fields,
             Map<String, MethodHandle> getterMethodHandles, Map<String, MethodHandle> setterMethodHandles,
             PainlessMethod functionalMethod) {
 
         this.constructors = Collections.unmodifiableMap(constructors);
+
         this.staticMethods = Collections.unmodifiableMap(staticMethods);
         this.methods = Collections.unmodifiableMap(methods);
 

modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java

@@ -24,7 +24,8 @@
 import java.util.Map;
 
 final class PainlessClassBuilder {
-    final Map<String, PainlessMethod> constructors;
+    final Map<String, PainlessConstructor> constructors;
+
     final Map<String, PainlessMethod> staticMethods;
     final Map<String, PainlessMethod> methods;
 
@@ -38,6 +39,7 @@ final class PainlessClassBuilder {
 
     PainlessClassBuilder() {
         constructors = new HashMap<>();
+
         staticMethods = new HashMap<>();
         methods = new HashMap<>();
 

modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java

@@ -0,0 +1,39 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.painless.lookup;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.util.List;
+
+public class PainlessConstructor {
+    public final Constructor<?> javaConstructor;
+    public final List<Class<?>> typeParameters;
+    public final MethodHandle methodHandle;
+    public final MethodType methodType;
+
+    PainlessConstructor(Constructor<?> javaConstructor, List<Class<?>> typeParameters, MethodHandle methodHandle, MethodType methodType) {
+        this.javaConstructor = javaConstructor;
+        this.typeParameters = typeParameters;
+        this.methodHandle = methodHandle;
+        this.methodType = methodType;
+    }
+}

modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java

@@ -40,15 +40,47 @@
 import java.util.Objects;
 import java.util.regex.Pattern;
 
-import static org.elasticsearch.painless.lookup.PainlessLookupUtility.CONSTRUCTOR_NAME;
 import static org.elasticsearch.painless.lookup.PainlessLookupUtility.DEF_CLASS_NAME;
+import static org.elasticsearch.painless.lookup.PainlessLookupUtility.buildPainlessConstructorKey;
 import static org.elasticsearch.painless.lookup.PainlessLookupUtility.buildPainlessFieldKey;
 import static org.elasticsearch.painless.lookup.PainlessLookupUtility.buildPainlessMethodKey;
 import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typeToCanonicalTypeName;
 import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typeToJavaType;
 import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typesToCanonicalTypeNames;
 
-public class PainlessLookupBuilder {
+public final class PainlessLookupBuilder {
+
+    private static class PainlessConstructorCacheKey {
+
+        private final Class<?> targetType;
+        private final List<Class<?>> typeParameters;
+
+        private PainlessConstructorCacheKey(Class<?> targetType, List<Class<?>> typeParameters) {
+            this.targetType = targetType;
+            this.typeParameters = Collections.unmodifiableList(typeParameters);
+        }
+
+        @Override
+        public boolean equals(Object object) {
+            if (this == object) {
+                return true;
+            }
+
+            if (object == null || getClass() != object.getClass()) {
+                return false;
+            }
+
+            PainlessConstructorCacheKey that = (PainlessConstructorCacheKey)object;
+
+            return Objects.equals(targetType, that.targetType) &&
+                   Objects.equals(typeParameters, that.typeParameters);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(targetType, typeParameters);
+        }
+    }
 
     private static class PainlessMethodCacheKey {
 
@@ -120,8 +152,9 @@ public int hashCode() {
         }
     }
 
-    private static final Map<PainlessMethodCacheKey, PainlessMethod> painlessMethodCache = new HashMap<>();
-    private static final Map<PainlessFieldCacheKey,  PainlessField>  painlessFieldCache  = new HashMap<>();
+    private static final Map<PainlessConstructorCacheKey, PainlessConstructor> painlessConstuctorCache = new HashMap<>();
+    private static final Map<PainlessMethodCacheKey,      PainlessMethod>      painlessMethodCache     = new HashMap<>();
+    private static final Map<PainlessFieldCacheKey,       PainlessField>       painlessFieldCache      = new HashMap<>();
 
     private static final Pattern CLASS_NAME_PATTERN  = Pattern.compile("^[_a-zA-Z][._a-zA-Z0-9]*$");
     private static final Pattern METHOD_NAME_PATTERN = Pattern.compile("^[_a-zA-Z][_a-zA-Z0-9]*$");
@@ -343,11 +376,10 @@ public void addPainlessConstructor(Class<?> targetClass, List<Class<?>> typePara
                     "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", nsme);
         }
 
-        String painlessMethodKey = buildPainlessMethodKey(CONSTRUCTOR_NAME, typeParametersSize);
-        PainlessMethod painlessConstructor = painlessClassBuilder.constructors.get(painlessMethodKey);
+        String painlessConstructorKey = buildPainlessConstructorKey(typeParametersSize);
+        PainlessConstructor painlessConstructor = painlessClassBuilder.constructors.get(painlessConstructorKey);
 
         if (painlessConstructor == null) {
-            org.objectweb.asm.commons.Method asmConstructor = org.objectweb.asm.commons.Method.getMethod(javaConstructor);
             MethodHandle methodHandle;
 
             try {
@@ -359,17 +391,16 @@ public void addPainlessConstructor(Class<?> targetClass, List<Class<?>> typePara
 
             MethodType methodType = methodHandle.type();
 
-            painlessConstructor = painlessMethodCache.computeIfAbsent(
-                    new PainlessMethodCacheKey(targetClass, CONSTRUCTOR_NAME, typeParameters),
-                    key -> new PainlessMethod(CONSTRUCTOR_NAME, targetClass, null, void.class, typeParameters,
-                                              asmConstructor, javaConstructor.getModifiers(), methodHandle, methodType)
+            painlessConstructor = painlessConstuctorCache.computeIfAbsent(
+                    new PainlessConstructorCacheKey(targetClass, typeParameters),
+                    key -> new PainlessConstructor(javaConstructor, typeParameters, methodHandle, methodType)
             );
 
-            painlessClassBuilder.constructors.put(painlessMethodKey, painlessConstructor);
-        } else if (painlessConstructor.arguments.equals(typeParameters) == false){
+            painlessClassBuilder.constructors.put(painlessConstructorKey, painlessConstructor);
+        } else if (painlessConstructor.typeParameters.equals(typeParameters) == false){
             throw new IllegalArgumentException("cannot have constructors " +
                     "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] and " +
-                    "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(painlessConstructor.arguments) + "] " +
+                    "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(painlessConstructor.typeParameters) + "] " +
                     "with the same arity and different type parameters");
         }
     }

modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java

@@ -336,6 +336,13 @@ public static boolean isConstantType(Class<?> type) {
                type == String.class;
     }
 
+    /**
+     * Constructs a painless constructor key used to lookup painless constructors from a painless class.
+     */
+    public static String buildPainlessConstructorKey(int constructorArity) {
+        return CONSTRUCTOR_NAME + "/" + constructorArity;
+    }
+
     /**
      * Constructs a painless method key used to lookup painless methods from a painless class.
      */