From 1419751abcc6e9dd475ec776cd3ea7ebe415f236 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Thu, 26 Jul 2018 13:22:10 -0700 Subject: [PATCH 1/7] Parially added constructor. --- .../elasticsearch/painless/FunctionRef.java | 76 +++++++++++++++++-- .../painless/lookup/PainlessClass.java | 6 +- .../painless/lookup/PainlessClassBuilder.java | 4 +- .../painless/lookup/PainlessConstructor.java | 36 +++++++++ .../lookup/PainlessLookupBuilder.java | 56 ++++++++++---- .../painless/lookup/PainlessMethod.java | 48 +----------- .../painless/node/ECapturingFunctionRef.java | 4 +- .../painless/node/EListInit.java | 10 ++- .../elasticsearch/painless/node/EMapInit.java | 10 ++- .../elasticsearch/painless/node/ENewObj.java | 18 +++-- 10 files changed, 179 insertions(+), 89 deletions(-) create mode 100644 modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java index aa72724b93029..aff367fc62318 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java @@ -20,6 +20,7 @@ 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; @@ -27,6 +28,7 @@ import java.lang.invoke.MethodType; 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 +61,10 @@ public class FunctionRef { /** interface method */ public final PainlessMethod interfaceMethod; - /** delegate method */ - public final PainlessMethod delegateMethod; + /** delegate method type parameters */ + public final List> delegateTypeParameters; + /** delegate method return type */ + public final Class delegateReturnType; /** factory method type descriptor */ public final String factoryDescriptor; @@ -93,12 +97,12 @@ public FunctionRef(PainlessLookup painlessLookup, Class expected, String type * @param numCaptures number of captured arguments */ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMethod delegateMethod, int numCaptures) { - MethodType delegateMethodType = delegateMethod.getMethodType(); + MethodType delegateMethodType = getMethodType(delegateMethod); interfaceMethodName = interfaceMethod.name; factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.getMethodType().dropParameterTypes(0, 1); + interfaceMethodType = getMethodType(delegateMethod).dropParameterTypes(0, 1); // the Painless$Script class can be inferred if owner is null if (delegateMethod.target == null) { @@ -126,7 +130,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()); @@ -142,7 +147,7 @@ public FunctionRef(Class expected, interfaceMethodName = interfaceMethod.name; factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.getMethodType().dropParameterTypes(0, 1); + interfaceMethodType = getMethodType(interfaceMethod).dropParameterTypes(0, 1); delegateClassName = CLASS_NAME; delegateInvokeType = H_INVOKESTATIC; @@ -151,7 +156,8 @@ public FunctionRef(Class expected, isDelegateInterface = false; this.interfaceMethod = null; - delegateMethod = null; + delegateTypeParameters = null; + delegateReturnType = null; factoryDescriptor = null; interfaceType = null; @@ -176,7 +182,7 @@ private static PainlessMethod lookup(PainlessLookup painlessLookup, Class exp final PainlessMethod impl; // ctor ref if ("new".equals(call)) { - impl = struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("", method.arguments.size())); + impl = null;//TODO: struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("", method.arguments.size())); } else { // look for a static impl first PainlessMethod staticImpl = @@ -202,4 +208,58 @@ private static PainlessMethod lookup(PainlessLookup painlessLookup, Class exp } return impl; } + + /** + * Returns MethodType for this constructor. + */ + private MethodType getMethodType(PainlessConstructor painlessConstructor) { + // constructor: returns the owner class + final Class params[] = new Class[painlessConstructor.typeParameters.size()]; + for (int i = 0; i < painlessConstructor.typeParameters.size(); i++) { + params[i] = PainlessLookupUtility.typeToJavaType(painlessConstructor.typeParameters.get(i)); + } + Class returnValue = painlessConstructor.javaConstructor.getDeclaringClass(); + return MethodType.methodType(returnValue, params); + } + + /** + * Returns MethodType for this method. + *

+ * This works even for user-defined Methods (where the MethodHandle is null). + */ + private MethodType getMethodType(PainlessMethod painlessMethod) { + // we have a methodhandle already (e.g. whitelisted class) + // just return its type + if (painlessMethod.handle != null) { + return painlessMethod.handle.type(); + } + // otherwise compute it + final Class params[]; + final Class returnValue; + if (painlessMethod.augmentation != null) { + // static method disguised as virtual/interface method + params = new Class[1 + painlessMethod.arguments.size()]; + params[0] = painlessMethod.augmentation; + for (int i = 0; i < painlessMethod.arguments.size(); i++) { + params[i + 1] = PainlessLookupUtility.typeToJavaType(painlessMethod.arguments.get(i)); + } + returnValue = PainlessLookupUtility.typeToJavaType(painlessMethod.rtn); + } else if (Modifier.isStatic(painlessMethod.modifiers)) { + // static method: straightforward copy + params = new Class[painlessMethod.arguments.size()]; + for (int i = 0; i < painlessMethod.arguments.size(); i++) { + params[i] = PainlessLookupUtility.typeToJavaType(painlessMethod.arguments.get(i)); + } + returnValue = PainlessLookupUtility.typeToJavaType(painlessMethod.rtn); + } else { + // virtual/interface method: add receiver class + params = new Class[1 + painlessMethod.arguments.size()]; + params[0] = painlessMethod.target; + for (int i = 0; i < painlessMethod.arguments.size(); i++) { + params[i + 1] = PainlessLookupUtility.typeToJavaType(painlessMethod.arguments.get(i)); + } + returnValue = PainlessLookupUtility.typeToJavaType(painlessMethod.rtn); + } + return MethodType.methodType(returnValue, params); + } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java index 24dcf0ebdba87..a10413acb8498 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java +++ b/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 constructors; + public final Map constructors; + public final Map staticMethods; public final Map methods; @@ -36,13 +37,14 @@ public final class PainlessClass { public final PainlessMethod functionalMethod; - PainlessClass(Map constructors, + PainlessClass(Map constructors, Map staticMethods, Map methods, Map staticFields, Map fields, Map getterMethodHandles, Map setterMethodHandles, PainlessMethod functionalMethod) { this.constructors = Collections.unmodifiableMap(constructors); + this.staticMethods = Collections.unmodifiableMap(staticMethods); this.methods = Collections.unmodifiableMap(methods); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java index 2f41ed5dca8f1..9cc4e0fdbb7fe 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java +++ b/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 constructors; + final Map constructors; + final Map staticMethods; final Map methods; @@ -38,6 +39,7 @@ final class PainlessClassBuilder { PainlessClassBuilder() { constructors = new HashMap<>(); + staticMethods = new HashMap<>(); methods = new HashMap<>(); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java new file mode 100644 index 0000000000000..cf59ca0725265 --- /dev/null +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java @@ -0,0 +1,36 @@ +/* + * 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.reflect.Constructor; +import java.util.List; + +public class PainlessConstructor { + public final Constructor javaConstructor; + public final List> typeParameters; + public final MethodHandle methodHandle; + + PainlessConstructor(Constructor javaConstructor, List> typeParameters, MethodHandle methodHandle) { + this.javaConstructor = javaConstructor; + this.typeParameters = typeParameters; + this.methodHandle = methodHandle; + } +} diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 3675cc7cd0f20..392c6acc5aa84 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -47,7 +47,39 @@ 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> typeParameters; + + private PainlessConstructorCacheKey(Class targetType, List> 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; + } + + PainlessMethodCacheKey that = (PainlessMethodCacheKey)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 { @@ -119,8 +151,9 @@ public int hashCode() { } } - private static final Map painlessMethodCache = new HashMap<>(); - private static final Map painlessFieldCache = new HashMap<>(); + private static final Map painlessConstuctorCache = new HashMap<>(); + private static final Map painlessMethodCache = new HashMap<>(); + private static final Map 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]*$"); @@ -342,11 +375,9 @@ public void addPainlessConstructor(Class targetClass, List> typePara "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", nsme); } - String painlessMethodKey = buildPainlessMethodKey(CONSTRUCTOR_NAME, typeParametersSize); - PainlessMethod painlessConstructor = painlessClassBuilder.constructors.get(painlessMethodKey); + PainlessConstructor painlessConstructor = painlessClassBuilder.constructors.get(typeParametersSize); if (painlessConstructor == null) { - org.objectweb.asm.commons.Method asmConstructor = org.objectweb.asm.commons.Method.getMethod(javaConstructor); MethodHandle methodHandle; try { @@ -356,17 +387,16 @@ public void addPainlessConstructor(Class targetClass, List> typePara "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", iae); } - painlessConstructor = painlessMethodCache.computeIfAbsent( - new PainlessMethodCacheKey(targetClass, CONSTRUCTOR_NAME, typeParameters), - key -> new PainlessMethod(CONSTRUCTOR_NAME, targetClass, null, void.class, typeParameters, - asmConstructor, javaConstructor.getModifiers(), methodHandle) + painlessConstructor = painlessConstuctorCache.computeIfAbsent( + new PainlessConstructorCacheKey(targetClass, typeParameters), + key -> new PainlessConstructor(javaConstructor, typeParameters, methodHandle) ); - painlessClassBuilder.constructors.put(painlessMethodKey, painlessConstructor); - } else if (painlessConstructor.arguments.equals(typeParameters) == false){ + painlessClassBuilder.constructors.put(typeParametersSize, 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"); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java index 3321de94a267f..adeeb31b3accd 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java @@ -51,53 +51,7 @@ public PainlessMethod(String name, Class target, Class augmentation, Class this.handle = handle; } - /** - * Returns MethodType for this method. - *

- * This works even for user-defined Methods (where the MethodHandle is null). - */ - public MethodType getMethodType() { - // we have a methodhandle already (e.g. whitelisted class) - // just return its type - if (handle != null) { - return handle.type(); - } - // otherwise compute it - final Class params[]; - final Class returnValue; - if (augmentation != null) { - // static method disguised as virtual/interface method - params = new Class[1 + arguments.size()]; - params[0] = augmentation; - for (int i = 0; i < arguments.size(); i++) { - params[i + 1] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = PainlessLookupUtility.typeToJavaType(rtn); - } else if (Modifier.isStatic(modifiers)) { - // static method: straightforward copy - params = new Class[arguments.size()]; - for (int i = 0; i < arguments.size(); i++) { - params[i] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = PainlessLookupUtility.typeToJavaType(rtn); - } else if ("".equals(name)) { - // constructor: returns the owner class - params = new Class[arguments.size()]; - for (int i = 0; i < arguments.size(); i++) { - params[i] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = target; - } else { - // virtual/interface method: add receiver class - params = new Class[1 + arguments.size()]; - params[0] = target; - for (int i = 0; i < arguments.size(); i++) { - params[i + 1] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = PainlessLookupUtility.typeToJavaType(rtn); - } - return MethodType.methodType(returnValue, params); - } + public void write(MethodWriter writer) { final org.objectweb.asm.Type type; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java index 7b35bc1b48ee5..986cb0def68dd 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java @@ -83,12 +83,12 @@ void analyze(Locals locals) { // check casts between the interface method and the delegate method are legal for (int i = 0; i < ref.interfaceMethod.arguments.size(); ++i) { Class from = ref.interfaceMethod.arguments.get(i); - Class to = ref.delegateMethod.arguments.get(i); + Class to = ref.delegateTypeParameters.get(i); AnalyzerCaster.getLegalCast(location, from, to, false, true); } if (ref.interfaceMethod.rtn != void.class) { - AnalyzerCaster.getLegalCast(location, ref.delegateMethod.rtn, ref.interfaceMethod.rtn, false, true); + AnalyzerCaster.getLegalCast(location, ref.delegateReturnType, ref.interfaceMethod.rtn, false, true); } } catch (IllegalArgumentException e) { throw createError(e); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java index e0af653d2098a..1dec938adeb50 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java @@ -23,10 +23,12 @@ import org.elasticsearch.painless.Locals; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; +import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.painless.lookup.PainlessMethod; import org.elasticsearch.painless.lookup.def; import org.objectweb.asm.Type; +import org.objectweb.asm.commons.Method; import java.util.ArrayList; import java.util.List; @@ -38,7 +40,7 @@ public final class EListInit extends AExpression { private final List values; - private PainlessMethod constructor = null; + private PainlessConstructor constructor = null; private PainlessMethod method = null; public EListInit(Location location, List values) { @@ -62,8 +64,7 @@ void analyze(Locals locals) { actual = ArrayList.class; - constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors - .get(PainlessLookupUtility.buildPainlessMethodKey("", 0)); + constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get(0); if (constructor == null) { throw createError(new IllegalStateException("Illegal tree structure.")); @@ -92,7 +93,8 @@ void write(MethodWriter writer, Globals globals) { writer.newInstance(MethodWriter.getType(actual)); writer.dup(); - writer.invokeConstructor(Type.getType(constructor.target), constructor.method); + writer.invokeConstructor( + Type.getType(constructor.javaConstructor.getDeclaringClass()), Method.getMethod(constructor.javaConstructor)); for (AExpression value : values) { writer.dup(); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java index d81f08dc3cc54..5013cc302c5b5 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java @@ -23,10 +23,12 @@ import org.elasticsearch.painless.Locals; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; +import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.painless.lookup.PainlessMethod; import org.elasticsearch.painless.lookup.def; import org.objectweb.asm.Type; +import org.objectweb.asm.commons.Method; import java.util.HashMap; import java.util.List; @@ -39,7 +41,7 @@ public final class EMapInit extends AExpression { private final List keys; private final List values; - private PainlessMethod constructor = null; + private PainlessConstructor constructor = null; private PainlessMethod method = null; public EMapInit(Location location, List keys, List values) { @@ -68,8 +70,7 @@ void analyze(Locals locals) { actual = HashMap.class; - constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors - .get(PainlessLookupUtility.buildPainlessMethodKey("", 0)); + constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get(0); if (constructor == null) { throw createError(new IllegalStateException("Illegal tree structure.")); @@ -111,7 +112,8 @@ void write(MethodWriter writer, Globals globals) { writer.newInstance(MethodWriter.getType(actual)); writer.dup(); - writer.invokeConstructor(Type.getType(constructor.target), constructor.method); + writer.invokeConstructor( + Type.getType(constructor.javaConstructor.getDeclaringClass()), Method.getMethod(constructor.javaConstructor)); for (int index = 0; index < keys.size(); ++index) { AExpression key = keys.get(index); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java index f092a17c9fc4a..f9d882f39efec 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java @@ -24,9 +24,10 @@ import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; import org.elasticsearch.painless.lookup.PainlessClass; +import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessLookupUtility; -import org.elasticsearch.painless.lookup.PainlessMethod; import org.objectweb.asm.Type; +import org.objectweb.asm.commons.Method; import java.util.List; import java.util.Objects; @@ -40,7 +41,7 @@ public final class ENewObj extends AExpression { private final String type; private final List arguments; - private PainlessMethod constructor; + private PainlessConstructor constructor; public ENewObj(Location location, String type, List arguments) { super(location); @@ -65,16 +66,16 @@ void analyze(Locals locals) { } PainlessClass struct = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual); - constructor = struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("", arguments.size())); + constructor = struct.constructors.get(arguments.size()); if (constructor != null) { - Class[] types = new Class[constructor.arguments.size()]; - constructor.arguments.toArray(types); + Class[] types = new Class[constructor.typeParameters.size()]; + constructor.typeParameters.toArray(types); - if (constructor.arguments.size() != arguments.size()) { + if (constructor.typeParameters.size() != arguments.size()) { throw createError(new IllegalArgumentException( "When calling constructor on type [" + PainlessLookupUtility.typeToCanonicalTypeName(actual) + "] " + - "expected [" + constructor.arguments.size() + "] arguments, but found [" + arguments.size() + "].")); + "expected [" + constructor.typeParameters.size() + "] arguments, but found [" + arguments.size() + "].")); } for (int argument = 0; argument < arguments.size(); ++argument) { @@ -107,7 +108,8 @@ void write(MethodWriter writer, Globals globals) { argument.write(writer, globals); } - writer.invokeConstructor(Type.getType(constructor.target), constructor.method); + writer.invokeConstructor( + Type.getType(constructor.javaConstructor.getDeclaringClass()), Method.getMethod(constructor.javaConstructor)); } @Override From 6469598db78a22dadcc53159c8b1a9e76ffc85d3 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 27 Jul 2018 09:43:31 -0700 Subject: [PATCH 2/7] Add method type to method. --- .../elasticsearch/painless/FunctionRef.java | 6 +-- .../lookup/PainlessLookupBuilder.java | 13 +++-- .../painless/lookup/PainlessMethod.java | 54 ++----------------- .../painless/node/SFunction.java | 4 +- 4 files changed, 20 insertions(+), 57 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java index aa72724b93029..7b7e8d25f392d 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java @@ -93,12 +93,12 @@ public FunctionRef(PainlessLookup painlessLookup, Class expected, String type * @param numCaptures number of captured arguments */ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMethod delegateMethod, int numCaptures) { - MethodType delegateMethodType = delegateMethod.getMethodType(); + MethodType delegateMethodType = delegateMethod.methodType; interfaceMethodName = interfaceMethod.name; factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.getMethodType().dropParameterTypes(0, 1); + interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); // the Painless$Script class can be inferred if owner is null if (delegateMethod.target == null) { @@ -142,7 +142,7 @@ public FunctionRef(Class expected, interfaceMethodName = interfaceMethod.name; factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.getMethodType().dropParameterTypes(0, 1); + interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); delegateClassName = CLASS_NAME; delegateInvokeType = H_INVOKESTATIC; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 3675cc7cd0f20..ba48fbea734cf 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -27,6 +27,7 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; @@ -356,10 +357,12 @@ public void addPainlessConstructor(Class targetClass, List> typePara "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", iae); } + 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) + asmConstructor, javaConstructor.getModifiers(), methodHandle, methodType) ); painlessClassBuilder.constructors.put(painlessMethodKey, painlessConstructor); @@ -516,10 +519,12 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, Str "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", iae); } + MethodType methodType = methodHandle.type(); + painlessMethod = painlessMethodCache.computeIfAbsent( new PainlessMethodCacheKey(targetClass, methodName, typeParameters), key -> new PainlessMethod(methodName, targetClass, null, returnType, - typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle)); + typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle, methodType)); painlessClassBuilder.staticMethods.put(painlessMethodKey, painlessMethod); } else if ((painlessMethod.name.equals(methodName) && painlessMethod.rtn == returnType && @@ -557,10 +562,12 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, Str } } + MethodType methodType = methodHandle.type(); + painlessMethod = painlessMethodCache.computeIfAbsent( new PainlessMethodCacheKey(targetClass, methodName, typeParameters), key -> new PainlessMethod(methodName, targetClass, augmentedClass, returnType, - typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle)); + typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle, methodType)); painlessClassBuilder.methods.put(painlessMethodKey, painlessMethod); } else if ((painlessMethod.name.equals(methodName) && painlessMethod.rtn == returnType && diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java index 3321de94a267f..904cf06907fde 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java @@ -38,9 +38,10 @@ public class PainlessMethod { public final org.objectweb.asm.commons.Method method; public final int modifiers; public final MethodHandle handle; + public final MethodType methodType; public PainlessMethod(String name, Class target, Class augmentation, Class rtn, List> arguments, - org.objectweb.asm.commons.Method method, int modifiers, MethodHandle handle) { + org.objectweb.asm.commons.Method method, int modifiers, MethodHandle handle, MethodType methodType) { this.name = name; this.augmentation = augmentation; this.target = target; @@ -49,54 +50,7 @@ public PainlessMethod(String name, Class target, Class augmentation, Class this.method = method; this.modifiers = modifiers; this.handle = handle; - } - - /** - * Returns MethodType for this method. - *

- * This works even for user-defined Methods (where the MethodHandle is null). - */ - public MethodType getMethodType() { - // we have a methodhandle already (e.g. whitelisted class) - // just return its type - if (handle != null) { - return handle.type(); - } - // otherwise compute it - final Class params[]; - final Class returnValue; - if (augmentation != null) { - // static method disguised as virtual/interface method - params = new Class[1 + arguments.size()]; - params[0] = augmentation; - for (int i = 0; i < arguments.size(); i++) { - params[i + 1] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = PainlessLookupUtility.typeToJavaType(rtn); - } else if (Modifier.isStatic(modifiers)) { - // static method: straightforward copy - params = new Class[arguments.size()]; - for (int i = 0; i < arguments.size(); i++) { - params[i] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = PainlessLookupUtility.typeToJavaType(rtn); - } else if ("".equals(name)) { - // constructor: returns the owner class - params = new Class[arguments.size()]; - for (int i = 0; i < arguments.size(); i++) { - params[i] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = target; - } else { - // virtual/interface method: add receiver class - params = new Class[1 + arguments.size()]; - params[0] = target; - for (int i = 0; i < arguments.size(); i++) { - params[i + 1] = PainlessLookupUtility.typeToJavaType(arguments.get(i)); - } - returnValue = PainlessLookupUtility.typeToJavaType(rtn); - } - return MethodType.methodType(returnValue, params); + this.methodType = methodType; } public void write(MethodWriter writer) { @@ -118,7 +72,7 @@ public void write(MethodWriter writer) { // method since java 8 did not check, but java 9 and 10 do if (Modifier.isInterface(clazz.getModifiers())) { writer.visitMethodInsn(Opcodes.INVOKESTATIC, - type.getInternalName(), name, getMethodType().toMethodDescriptorString(), true); + type.getInternalName(), name, methodType.toMethodDescriptorString(), true); } else { writer.invokeStatic(type, method); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java index 7c243e296c7e3..600ca95504ac6 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java @@ -145,9 +145,11 @@ void generateSignature(PainlessLookup painlessLookup) { } } + int modifiers = Modifier.STATIC | Modifier.PRIVATE; org.objectweb.asm.commons.Method method = new org.objectweb.asm.commons.Method(name, MethodType.methodType( PainlessLookupUtility.typeToJavaType(rtnType), paramClasses).toMethodDescriptorString()); - this.method = new PainlessMethod(name, null, null, rtnType, paramTypes, method, Modifier.STATIC | Modifier.PRIVATE, null); + MethodType methodType = MethodType.methodType(PainlessLookupUtility.typeToJavaType(rtnType), paramClasses); + this.method = new PainlessMethod(name, null, null, rtnType, paramTypes, method, modifiers, null, methodType); } @Override From 641c383af51da6389b76b866dd59a66a8041110a Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 27 Jul 2018 10:42:59 -0700 Subject: [PATCH 3/7] Add PainlessConstructor. --- .../java/org/elasticsearch/painless/Def.java | 2 +- .../elasticsearch/painless/FunctionRef.java | 104 +++++++++++++----- .../lookup/PainlessLookupBuilder.java | 9 +- .../lookup/PainlessLookupUtility.java | 7 ++ .../painless/node/ECapturingFunctionRef.java | 2 +- .../painless/node/EFunctionRef.java | 2 +- .../painless/PainlessDocGenerator.java | 99 +++++++++++++++-- 7 files changed, 184 insertions(+), 41 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index 8a90f53b4fdfa..863f973a5c2f3 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java +++ b/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.getFunctionRef(painlessLookup, clazz, type, call, captures.length); } final CallSite callSite = LambdaBootstrap.lambdaBootstrap( methodHandlesLookup, diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java index a3ec943e78a88..2752b046f8e24 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java @@ -20,12 +20,14 @@ 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; @@ -83,9 +85,45 @@ 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 getFunctionRef(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()); } /** @@ -115,9 +153,7 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMe isDelegateInterface = delegateMethod.target.isInterface(); } - if ("".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; @@ -163,6 +199,29 @@ public FunctionRef(Class expected, 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. */ @@ -179,27 +238,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 = null;//TODO: struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("", 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 " + diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 1ea98c50fc67c..28ee228601a22 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -40,8 +40,8 @@ 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; @@ -70,7 +70,7 @@ public boolean equals(Object object) { return false; } - PainlessMethodCacheKey that = (PainlessMethodCacheKey)object; + PainlessConstructorCacheKey that = (PainlessConstructorCacheKey)object; return Objects.equals(targetType, that.targetType) && Objects.equals(typeParameters, that.typeParameters); @@ -376,7 +376,8 @@ public void addPainlessConstructor(Class targetClass, List> typePara "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", nsme); } - PainlessConstructor painlessConstructor = painlessClassBuilder.constructors.get(typeParametersSize); + int painlessConstructorKey = buildPainlessConstructorKey(typeParametersSize); + PainlessConstructor painlessConstructor = painlessClassBuilder.constructors.get(painlessConstructorKey); if (painlessConstructor == null) { MethodHandle methodHandle; @@ -395,7 +396,7 @@ public void addPainlessConstructor(Class targetClass, List> typePara key -> new PainlessConstructor(javaConstructor, typeParameters, methodHandle, methodType) ); - painlessClassBuilder.constructors.put(typeParametersSize, painlessConstructor); + painlessClassBuilder.constructors.put(painlessConstructorKey, painlessConstructor); } else if (painlessConstructor.typeParameters.equals(typeParameters) == false){ throw new IllegalArgumentException("cannot have constructors " + "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] and " + diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java index 86d3f87663867..7a4c951235e13 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java +++ b/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 int buildPainlessConstructorKey(int constructorArity) { + return constructorArity; + } + /** * Constructs a painless method key used to lookup painless methods from a painless class. */ diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java index 986cb0def68dd..50b685c1ca52f 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java @@ -77,7 +77,7 @@ void analyze(Locals locals) { // static case if (captured.clazz != def.class) { try { - ref = new FunctionRef(locals.getPainlessLookup(), expected, + ref = FunctionRef.getFunctionRef(locals.getPainlessLookup(), expected, PainlessLookupUtility.typeToCanonicalTypeName(captured.clazz), call, 1); // check casts between the interface method and the delegate method are legal diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java index d787db5d41c92..437fb84b79e2f 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java @@ -90,7 +90,7 @@ void analyze(Locals locals) { } } else { // whitelist lookup - ref = new FunctionRef(locals.getPainlessLookup(), expected, type, call, 0); + ref = FunctionRef.getFunctionRef(locals.getPainlessLookup(), expected, type, call, 0); } } catch (IllegalArgumentException e) { diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java index ff0d423117564..654291543017f 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.painless.lookup.PainlessClass; +import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessField; import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.lookup.PainlessLookupBuilder; @@ -57,7 +58,8 @@ public class PainlessDocGenerator { private static final Logger logger = ESLoggerFactory.getLogger(PainlessDocGenerator.class); private static final Comparator FIELD_NAME = comparing(f -> f.name); private static final Comparator METHOD_NAME = comparing(m -> m.name); - private static final Comparator NUMBER_OF_ARGS = comparing(m -> m.arguments.size()); + private static final Comparator METHOD_NUMBER_OF_PARAMS = comparing(m -> m.arguments.size()); + private static final Comparator CONSTRUCTOR_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size()); public static void main(String[] args) throws IOException { Path apiRootPath = PathUtils.get(args[0]); @@ -103,12 +105,15 @@ public static void main(String[] args) throws IOException { Consumer documentField = field -> PainlessDocGenerator.documentField(typeStream, field); Consumer documentMethod = method -> PainlessDocGenerator.documentMethod(typeStream, method); + Consumer documentConstructor = + constructor -> PainlessDocGenerator.documentConstructor(typeStream, constructor); struct.staticFields.values().stream().sorted(FIELD_NAME).forEach(documentField); struct.fields.values().stream().sorted(FIELD_NAME).forEach(documentField); - struct.staticMethods.values().stream().sorted(METHOD_NAME.thenComparing(NUMBER_OF_ARGS)).forEach(documentMethod); - struct.constructors.values().stream().sorted(NUMBER_OF_ARGS).forEach(documentMethod); + struct.staticMethods.values().stream().sorted( + METHOD_NAME.thenComparing(METHOD_NUMBER_OF_PARAMS)).forEach(documentMethod); + struct.constructors.values().stream().sorted(CONSTRUCTOR_NUMBER_OF_PARAMS).forEach(documentConstructor); Map> inherited = new TreeMap<>(); - struct.methods.values().stream().sorted(METHOD_NAME.thenComparing(NUMBER_OF_ARGS)).forEach(method -> { + struct.methods.values().stream().sorted(METHOD_NAME.thenComparing(METHOD_NUMBER_OF_PARAMS)).forEach(method -> { if (method.target == clazz) { documentMethod(typeStream, method); } else { @@ -164,6 +169,41 @@ private static void documentField(PrintStream stream, PainlessField field) { stream.println(); } + /** + * Document a constructor. + */ + private static void documentConstructor(PrintStream stream, PainlessConstructor constructor) { + stream.print("* ++[["); + emitAnchor(stream, constructor); + stream.print("]]"); + + String javadocRoot = javadocRoot(constructor.javaConstructor.getDeclaringClass()); + emitJavadocLink(stream, javadocRoot, constructor); + stream.print('['); + + stream.print(constructorName(constructor)); + + stream.print("]("); + boolean first = true; + for (Class arg : constructor.typeParameters) { + if (first) { + first = false; + } else { + stream.print(", "); + } + emitType(stream, arg); + } + stream.print(")++"); + + if (javadocRoot.equals("java8")) { + stream.print(" ("); + emitJavadocLink(stream, "java9", constructor); + stream.print("[java 9])"); + } + + stream.println(); + } + /** * Document a method. */ @@ -176,10 +216,8 @@ private static void documentMethod(PrintStream stream, PainlessMethod method) { stream.print("static "); } - if (false == method.name.equals("")) { - emitType(stream, method.rtn); - stream.print(' '); - } + emitType(stream, method.rtn); + stream.print(' '); String javadocRoot = javadocRoot(method); emitJavadocLink(stream, javadocRoot, method); @@ -216,6 +254,17 @@ private static void emitAnchor(PrintStream stream, Class clazz) { stream.print(PainlessLookupUtility.typeToCanonicalTypeName(clazz).replace('.', '-')); } + /** + * Anchor text for a {@link PainlessConstructor}. + */ + private static void emitAnchor(PrintStream stream, PainlessConstructor constructor) { + emitAnchor(stream, constructor.javaConstructor.getDeclaringClass()); + stream.print('-'); + stream.print(constructorName(constructor)); + stream.print('-'); + stream.print(constructor.typeParameters.size()); + } + /** * Anchor text for a {@link PainlessMethod}. */ @@ -236,8 +285,12 @@ private static void emitAnchor(PrintStream stream, PainlessField field) { stream.print(field.name); } + private static String constructorName(PainlessConstructor constructor) { + return PainlessLookupUtility.typeToCanonicalTypeName(constructor.javaConstructor.getDeclaringClass()); + } + private static String methodName(PainlessMethod method) { - return method.name.equals("") ? PainlessLookupUtility.typeToCanonicalTypeName(method.target) : method.name; + return PainlessLookupUtility.typeToCanonicalTypeName(method.target); } /** @@ -269,6 +322,34 @@ private static void emitStruct(PrintStream stream, Class clazz) { } } + /** + * Emit an external link to Javadoc for a {@link PainlessMethod}. + * + * @param root name of the root uri variable + */ + private static void emitJavadocLink(PrintStream stream, String root, PainlessConstructor constructor) { + stream.print("link:{"); + stream.print(root); + stream.print("-javadoc}/"); + stream.print(classUrlPath(constructor.javaConstructor.getDeclaringClass())); + stream.print(".html#"); + stream.print(constructorName(constructor)); + stream.print("%2D"); + boolean first = true; + for (Class clazz: constructor.typeParameters) { + if (first) { + first = false; + } else { + stream.print("%2D"); + } + stream.print(clazz.getName()); + if (clazz.isArray()) { + stream.print(":A"); + } + } + stream.print("%2D"); + } + /** * Emit an external link to Javadoc for a {@link PainlessMethod}. * From 5b449d891cbb4f949c8c4d189cd6e7771900dd90 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 27 Jul 2018 14:17:59 -0700 Subject: [PATCH 4/7] Clean up method. --- .../java/org/elasticsearch/painless/Def.java | 15 +++-- .../elasticsearch/painless/FunctionRef.java | 67 +++++++++++++------ .../org/elasticsearch/painless/Locals.java | 42 +++++++++--- .../elasticsearch/painless/MethodWriter.java | 24 +++++++ .../lookup/PainlessLookupBuilder.java | 35 +++++----- .../painless/lookup/PainlessMethod.java | 67 ++++--------------- .../painless/node/ECallLocal.java | 11 +-- .../painless/node/ECapturingFunctionRef.java | 8 +-- .../painless/node/EFunctionRef.java | 14 ++-- .../elasticsearch/painless/node/ELambda.java | 23 ++++--- .../painless/node/EListInit.java | 2 +- .../elasticsearch/painless/node/EMapInit.java | 2 +- .../painless/node/PSubCallInvoke.java | 8 +-- .../painless/node/PSubListShortcut.java | 25 +++---- .../painless/node/PSubMapShortcut.java | 32 ++++----- .../painless/node/PSubShortcut.java | 24 +++---- .../painless/node/SFunction.java | 31 ++++----- .../elasticsearch/painless/node/SSource.java | 12 ++-- .../painless/node/SSubEachIterable.java | 2 +- .../painless/PainlessDocGenerator.java | 32 ++++----- 20 files changed, 248 insertions(+), 228 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index 863f973a5c2f3..e4e4414afb9c5 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java @@ -238,7 +238,7 @@ static MethodHandle lookupMethod(PainlessLookup painlessLookup, MethodHandles.Lo int numArguments = callSiteType.parameterCount(); // simple case: no lambdas if (recipeString.isEmpty()) { - return lookupMethodInternal(painlessLookup, receiverClass, name, numArguments - 1).handle; + return lookupMethodInternal(painlessLookup, receiverClass, name, numArguments - 1).methodHandle; } // convert recipe string to a bitset for convenience (the code below should be refactored...) @@ -262,7 +262,7 @@ static MethodHandle lookupMethod(PainlessLookup painlessLookup, MethodHandles.Lo // lookup the method with the proper arity, then we know everything (e.g. interface types of parameters). // based on these we can finally link any remaining lambdas that were deferred. PainlessMethod method = lookupMethodInternal(painlessLookup, receiverClass, name, arity); - MethodHandle handle = method.handle; + MethodHandle handle = method.methodHandle; int replaced = 0; upTo = 1; @@ -281,7 +281,7 @@ static MethodHandle lookupMethod(PainlessLookup painlessLookup, MethodHandles.Lo captures[capture] = callSiteType.parameterType(i + 1 + capture); } MethodHandle filter; - Class interfaceType = method.arguments.get(i - 1 - replaced); + Class interfaceType = method.typeParameters.get(i - 1 - replaced); if (signature.charAt(0) == 'S') { // the implementation is strongly typed, now that we know the interface type, // we have everything. @@ -331,10 +331,11 @@ static MethodHandle lookupReference(PainlessLookup painlessLookup, MethodHandles if (interfaceMethod == null) { throw new IllegalArgumentException("Class [" + interfaceClass + "] is not a functional interface"); } - int arity = interfaceMethod.arguments.size(); + int arity = interfaceMethod.typeParameters.size(); PainlessMethod implMethod = lookupMethodInternal(painlessLookup, receiverClass, name, arity); return lookupReferenceInternal(painlessLookup, methodHandlesLookup, interfaceType, - PainlessLookupUtility.typeToCanonicalTypeName(implMethod.target), implMethod.name, receiverClass); + PainlessLookupUtility.typeToCanonicalTypeName(implMethod.javaMethod.getDeclaringClass()), + implMethod.javaMethod.getName(), receiverClass); } /** Returns a method handle to an implementation of clazz, given method reference signature. */ @@ -349,7 +350,7 @@ private static MethodHandle lookupReferenceInternal(PainlessLookup painlessLooku throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] " + "to [" + PainlessLookupUtility.typeToCanonicalTypeName(clazz) + "], not a functional interface"); } - int arity = interfaceMethod.arguments.size() + captures.length; + int arity = interfaceMethod.typeParameters.size() + captures.length; final MethodHandle handle; try { MethodHandle accessor = methodHandlesLookup.findStaticGetter(methodHandlesLookup.lookupClass(), @@ -360,7 +361,7 @@ private static MethodHandle lookupReferenceInternal(PainlessLookup painlessLooku // is it a synthetic method? If we generated the method ourselves, be more helpful. It can only fail // because the arity does not match the expected interface type. if (call.contains("$")) { - throw new IllegalArgumentException("Incorrect number of parameters for [" + interfaceMethod.name + + throw new IllegalArgumentException("Incorrect number of parameters for [" + interfaceMethod.javaMethod.getName() + "] in [" + clazz + "]"); } throw new IllegalArgumentException("Unknown call [" + call + "] with [" + arity + "] arguments."); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java index 2752b046f8e24..dff5ff90cf22b 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java @@ -19,6 +19,7 @@ package org.elasticsearch.painless; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.lookup.PainlessClass; import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessLookup; @@ -106,7 +107,7 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessCo Constructor javaConstructor = delegateConstructor.javaConstructor; MethodType delegateMethodType = delegateConstructor.methodType; - interfaceMethodName = interfaceMethod.name; + interfaceMethodName = interfaceMethod.javaMethod.getName(); factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); @@ -136,37 +137,59 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessCo public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMethod delegateMethod, int numCaptures) { MethodType delegateMethodType = delegateMethod.methodType; - interfaceMethodName = interfaceMethod.name; + interfaceMethodName = interfaceMethod.javaMethod.getName(); factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); - // the Painless$Script class can be inferred if owner is null - if (delegateMethod.target == null) { - delegateClassName = CLASS_NAME; - isDelegateInterface = false; - } else if (delegateMethod.augmentation != null) { - delegateClassName = delegateMethod.augmentation.getName(); - isDelegateInterface = delegateMethod.augmentation.isInterface(); - } else { - delegateClassName = delegateMethod.target.getName(); - isDelegateInterface = delegateMethod.target.isInterface(); - } + delegateClassName = delegateMethod.javaMethod.getName(); + isDelegateInterface = delegateMethod.javaMethod.getDeclaringClass().isInterface(); - if (Modifier.isStatic(delegateMethod.modifiers)) { + if (Modifier.isStatic(delegateMethod.javaMethod.getModifiers())) { delegateInvokeType = H_INVOKESTATIC; - } else if (delegateMethod.target.isInterface()) { + } else if (delegateMethod.javaMethod.getDeclaringClass().isInterface()) { delegateInvokeType = H_INVOKEINTERFACE; } else { delegateInvokeType = H_INVOKEVIRTUAL; } + delegateMethodName = delegateMethod.javaMethod.getName(); + this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); + + this.interfaceMethod = interfaceMethod; + delegateTypeParameters = delegateMethod.typeParameters; + delegateReturnType = delegateMethod.returnType; + + factoryDescriptor = factoryMethodType.toMethodDescriptorString(); + interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); + delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); + } + + /** + * Creates a new FunctionRef (already resolved) + * @param expected functional interface type to implement + * @param interfaceMethod functional interface method + * @param delegateMethod implementation method + * @param numCaptures number of captured arguments + */ + public FunctionRef(Class expected, PainlessMethod interfaceMethod, LocalMethod delegateMethod, int numCaptures) { + MethodType delegateMethodType = delegateMethod.methodType; + + interfaceMethodName = interfaceMethod.javaMethod.getName(); + factoryMethodType = MethodType.methodType(expected, + delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); + interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); + + delegateClassName = CLASS_NAME; + isDelegateInterface = false; + delegateInvokeType = H_INVOKESTATIC; + delegateMethodName = delegateMethod.name; this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); this.interfaceMethod = interfaceMethod; - delegateTypeParameters = delegateMethod.arguments; - delegateReturnType = delegateMethod.rtn; + delegateTypeParameters = delegateMethod.typeParameters; + delegateReturnType = delegateMethod.returnType; factoryDescriptor = factoryMethodType.toMethodDescriptorString(); interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); @@ -179,7 +202,7 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMe */ public FunctionRef(Class expected, PainlessMethod interfaceMethod, String delegateMethodName, MethodType delegateMethodType, int numCaptures) { - interfaceMethodName = interfaceMethod.name; + interfaceMethodName = interfaceMethod.javaMethod.getName(); factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); @@ -213,7 +236,7 @@ private static PainlessConstructor lookup(PainlessLookup painlessLookup, Class exp final PainlessMethod impl; // look for a static impl first PainlessMethod staticImpl = - struct.staticMethods.get(PainlessLookupUtility.buildPainlessMethodKey(call, method.arguments.size())); + struct.staticMethods.get(PainlessLookupUtility.buildPainlessMethodKey(call, method.typeParameters.size())); if (staticImpl == null) { // otherwise a virtual impl final int arity; if (receiverCaptured) { // receiver captured - arity = method.arguments.size(); + arity = method.typeParameters.size(); } else { // receiver passed - arity = method.arguments.size() - 1; + arity = method.typeParameters.size() - 1; } impl = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey(call, arity)); } else { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java index 804f6aa2b689c..f2c7e02c637c1 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java @@ -22,8 +22,8 @@ import org.elasticsearch.painless.ScriptClassInfo.MethodArgument; import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.lookup.PainlessLookupUtility; -import org.elasticsearch.painless.lookup.PainlessMethod; +import java.lang.invoke.MethodType; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -38,6 +38,30 @@ */ public final class Locals { + /** + * Constructs a local method key used to lookup local methods from a painless class. + */ + public static String buildLocalMethodKey(String methodName, int methodArity) { + return methodName + "/" + methodArity; + } + + /** + * Stores information about methods directly callable on the generated script class. + */ + public static class LocalMethod { + public final String name; + public final Class returnType; + public final List> typeParameters; + public final MethodType methodType; + + public LocalMethod(String name, Class returnType, List> typeParameters, MethodType methodType) { + this.name = name; + this.returnType = returnType; + this.typeParameters = typeParameters; + this.methodType = methodType; + } + } + /** Reserved word: loop counter */ public static final String LOOP = "#loop"; /** Reserved word: unused */ @@ -110,9 +134,9 @@ public static Locals newMainMethodScope(ScriptClassInfo scriptClassInfo, Locals } /** Creates a new program scope: the list of methods. It is the parent for all methods */ - public static Locals newProgramScope(PainlessLookup painlessLookup, Collection methods) { + public static Locals newProgramScope(PainlessLookup painlessLookup, Collection methods) { Locals locals = new Locals(null, painlessLookup, null, null); - for (PainlessMethod method : methods) { + for (LocalMethod method : methods) { locals.addMethod(method); } return locals; @@ -143,8 +167,8 @@ public Variable getVariable(Location location, String name) { } /** Looks up a method. Returns null if the method does not exist. */ - public PainlessMethod getMethod(String key) { - PainlessMethod method = lookupMethod(key); + public LocalMethod getMethod(String key) { + LocalMethod method = lookupMethod(key); if (method != null) { return method; } @@ -199,7 +223,7 @@ public PainlessLookup getPainlessLookup() { // variable name -> variable private Map variables; // method name+arity -> methods - private Map methods; + private Map methods; /** * Create a new Locals @@ -237,7 +261,7 @@ private Variable lookupVariable(Location location, String name) { } /** Looks up a method at this scope only. Returns null if the method does not exist. */ - private PainlessMethod lookupMethod(String key) { + private LocalMethod lookupMethod(String key) { if (methods == null) { return null; } @@ -256,11 +280,11 @@ private Variable defineVariable(Location location, Class type, String name, b return variable; } - private void addMethod(PainlessMethod method) { + private void addMethod(LocalMethod method) { if (methods == null) { methods = new HashMap<>(); } - methods.put(PainlessLookupUtility.buildPainlessMethodKey(method.name, method.arguments.size()), method); + methods.put(buildLocalMethodKey(method.name, method.typeParameters.size()), method); // TODO: check result } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java index c339e7bfb2613..72435562a3bd0 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java @@ -20,6 +20,7 @@ package org.elasticsearch.painless; import org.elasticsearch.painless.lookup.PainlessCast; +import org.elasticsearch.painless.lookup.PainlessMethod; import org.elasticsearch.painless.lookup.def; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.Label; @@ -28,6 +29,7 @@ import org.objectweb.asm.commons.GeneratorAdapter; import org.objectweb.asm.commons.Method; +import java.lang.reflect.Modifier; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; @@ -415,4 +417,26 @@ public void invokeDefCall(String name, Type methodType, int flavor, Object... pa System.arraycopy(params, 0, args, 2, params.length); invokeDynamic(name, methodType.getDescriptor(), DEF_BOOTSTRAP_HANDLE, args); } + + public void invokeMethodCall(PainlessMethod painlessMethod) { + Type type = Type.getType(painlessMethod.javaMethod.getDeclaringClass()); + Method method = Method.getMethod(painlessMethod.javaMethod); + + if (Modifier.isStatic(painlessMethod.javaMethod.getModifiers())) { + // invokeStatic assumes that the owner class is not an interface, so this is a + // special case for interfaces where the interface method boolean needs to be set to + // true to reference the appropriate class constant when calling a static interface + // method since java 8 did not check, but java 9 and 10 do + if (painlessMethod.javaMethod.getDeclaringClass().isInterface()) { + visitMethodInsn(Opcodes.INVOKESTATIC, type.getInternalName(), + painlessMethod.javaMethod.getName(), painlessMethod.methodType.toMethodDescriptorString(), true); + } else { + invokeStatic(type, method); + } + } else if (painlessMethod.javaMethod.getDeclaringClass().isInterface()) { + invokeInterface(type, method); + } else { + invokeVirtual(type, method); + } + } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 28ee228601a22..12581e27c9701 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -540,7 +540,6 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, Str PainlessMethod painlessMethod = painlessClassBuilder.staticMethods.get(painlessMethodKey); if (painlessMethod == null) { - org.objectweb.asm.commons.Method asmMethod = org.objectweb.asm.commons.Method.getMethod(javaMethod); MethodHandle methodHandle; try { @@ -554,19 +553,17 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, Str painlessMethod = painlessMethodCache.computeIfAbsent( new PainlessMethodCacheKey(targetClass, methodName, typeParameters), - key -> new PainlessMethod(methodName, targetClass, null, returnType, - typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle, methodType)); + key -> new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType)); painlessClassBuilder.staticMethods.put(painlessMethodKey, painlessMethod); - } else if ((painlessMethod.name.equals(methodName) && painlessMethod.rtn == returnType && - painlessMethod.arguments.equals(typeParameters)) == false) { + } else if (painlessMethod.returnType == returnType && painlessMethod.typeParameters.equals(typeParameters) == false) { throw new IllegalArgumentException("cannot have static methods " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + "[" + typeToCanonicalTypeName(returnType) + "], " + typesToCanonicalTypeNames(typeParameters) + "] and " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + - "[" + typeToCanonicalTypeName(painlessMethod.rtn) + "], " + - typesToCanonicalTypeNames(painlessMethod.arguments) + "] " + + "[" + typeToCanonicalTypeName(painlessMethod.returnType) + "], " + + typesToCanonicalTypeNames(painlessMethod.typeParameters) + "] " + "with the same arity and different return type or type parameters"); } } else { @@ -597,19 +594,17 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, Str painlessMethod = painlessMethodCache.computeIfAbsent( new PainlessMethodCacheKey(targetClass, methodName, typeParameters), - key -> new PainlessMethod(methodName, targetClass, augmentedClass, returnType, - typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle, methodType)); + key -> new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType)); painlessClassBuilder.methods.put(painlessMethodKey, painlessMethod); - } else if ((painlessMethod.name.equals(methodName) && painlessMethod.rtn == returnType && - painlessMethod.arguments.equals(typeParameters)) == false) { + } else if (painlessMethod.returnType == returnType && painlessMethod.typeParameters.equals(typeParameters) == false) { throw new IllegalArgumentException("cannot have methods " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + "[" + typeToCanonicalTypeName(returnType) + "], " + typesToCanonicalTypeNames(typeParameters) + "] and " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + - "[" + typeToCanonicalTypeName(painlessMethod.rtn) + "], " + - typesToCanonicalTypeNames(painlessMethod.arguments) + "] " + + "[" + typeToCanonicalTypeName(painlessMethod.returnType) + "], " + + typesToCanonicalTypeNames(painlessMethod.typeParameters) + "] " + "with the same arity and different return type or type parameters"); } } @@ -806,8 +801,8 @@ private void copyPainlessClassMembers(Class originalClass, Class targetCla PainlessMethod newPainlessMethod = painlessMethodEntry.getValue(); PainlessMethod existingPainlessMethod = targetPainlessClassBuilder.methods.get(painlessMethodKey); - if (existingPainlessMethod == null || existingPainlessMethod.target != newPainlessMethod.target && - existingPainlessMethod.target.isAssignableFrom(newPainlessMethod.target)) { + if (existingPainlessMethod == null || existingPainlessMethod.targetClass != newPainlessMethod.targetClass && + existingPainlessMethod.targetClass.isAssignableFrom(newPainlessMethod.targetClass)) { targetPainlessClassBuilder.methods.put(painlessMethodKey, newPainlessMethod); } } @@ -832,21 +827,21 @@ private void cacheRuntimeHandles() { private void cacheRuntimeHandles(PainlessClassBuilder painlessClassBuilder) { for (PainlessMethod painlessMethod : painlessClassBuilder.methods.values()) { - String methodName = painlessMethod.name; - int typeParametersSize = painlessMethod.arguments.size(); + String methodName = painlessMethod.javaMethod.getName(); + int typeParametersSize = painlessMethod.typeParameters.size(); if (typeParametersSize == 0 && methodName.startsWith("get") && methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) { painlessClassBuilder.getterMethodHandles.putIfAbsent( - Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4), painlessMethod.handle); + Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4), painlessMethod.methodHandle); } else if (typeParametersSize == 0 && methodName.startsWith("is") && methodName.length() > 2 && Character.isUpperCase(methodName.charAt(2))) { painlessClassBuilder.getterMethodHandles.putIfAbsent( - Character.toLowerCase(methodName.charAt(2)) + methodName.substring(3), painlessMethod.handle); + Character.toLowerCase(methodName.charAt(2)) + methodName.substring(3), painlessMethod.methodHandle); } else if (typeParametersSize == 1 && methodName.startsWith("set") && methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) { painlessClassBuilder.setterMethodHandles.putIfAbsent( - Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4), painlessMethod.handle); + Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4), painlessMethod.methodHandle); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java index 904cf06907fde..9dd143a402865 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java @@ -19,67 +19,28 @@ package org.elasticsearch.painless.lookup; -import org.elasticsearch.painless.MethodWriter; -import org.objectweb.asm.Opcodes; -import org.objectweb.asm.Type; - import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodType; -import java.lang.reflect.Modifier; +import java.lang.reflect.Method; import java.util.Collections; import java.util.List; public class PainlessMethod { - public final String name; - public final Class target; - public final Class augmentation; - public final Class rtn; - public final List> arguments; - public final org.objectweb.asm.commons.Method method; - public final int modifiers; - public final MethodHandle handle; + public final Method javaMethod; + public final Class targetClass; + public final Class returnType; + public final List> typeParameters; + public final MethodHandle methodHandle; public final MethodType methodType; - public PainlessMethod(String name, Class target, Class augmentation, Class rtn, List> arguments, - org.objectweb.asm.commons.Method method, int modifiers, MethodHandle handle, MethodType methodType) { - this.name = name; - this.augmentation = augmentation; - this.target = target; - this.rtn = rtn; - this.arguments = Collections.unmodifiableList(arguments); - this.method = method; - this.modifiers = modifiers; - this.handle = handle; - this.methodType = methodType; - } + public PainlessMethod(Method javaMethod, Class targetClass, Class returnType, List> typeParameters, + MethodHandle methodHandle, MethodType methodType) { - public void write(MethodWriter writer) { - final org.objectweb.asm.Type type; - final Class clazz; - if (augmentation != null) { - assert Modifier.isStatic(modifiers); - clazz = augmentation; - type = org.objectweb.asm.Type.getType(augmentation); - } else { - clazz = target; - type = Type.getType(target); - } - - if (Modifier.isStatic(modifiers)) { - // invokeStatic assumes that the owner class is not an interface, so this is a - // special case for interfaces where the interface method boolean needs to be set to - // true to reference the appropriate class constant when calling a static interface - // method since java 8 did not check, but java 9 and 10 do - if (Modifier.isInterface(clazz.getModifiers())) { - writer.visitMethodInsn(Opcodes.INVOKESTATIC, - type.getInternalName(), name, methodType.toMethodDescriptorString(), true); - } else { - writer.invokeStatic(type, method); - } - } else if (Modifier.isInterface(clazz.getModifiers())) { - writer.invokeInterface(type, method); - } else { - writer.invokeVirtual(type, method); - } + this.javaMethod = javaMethod; + this.targetClass = targetClass; + this.returnType = returnType; + this.typeParameters = Collections.unmodifiableList(typeParameters); + this.methodHandle = methodHandle; + this.methodType = methodType; } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java index 098c75386e1a6..7605a0c9f7f40 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java @@ -21,10 +21,11 @@ import org.elasticsearch.painless.Globals; import org.elasticsearch.painless.Locals; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; import org.elasticsearch.painless.lookup.PainlessLookupUtility; -import org.elasticsearch.painless.lookup.PainlessMethod; +import org.objectweb.asm.commons.Method; import java.util.List; import java.util.Objects; @@ -40,7 +41,7 @@ public final class ECallLocal extends AExpression { private final String name; private final List arguments; - private PainlessMethod method = null; + private LocalMethod method = null; public ECallLocal(Location location, String name, List arguments) { super(location); @@ -68,14 +69,14 @@ void analyze(Locals locals) { for (int argument = 0; argument < arguments.size(); ++argument) { AExpression expression = arguments.get(argument); - expression.expected = method.arguments.get(argument); + expression.expected = method.typeParameters.get(argument); expression.internal = true; expression.analyze(locals); arguments.set(argument, expression.cast(locals)); } statement = true; - actual = method.rtn; + actual = method.returnType; } @Override @@ -86,7 +87,7 @@ void write(MethodWriter writer, Globals globals) { argument.write(writer, globals); } - writer.invokeStatic(CLASS_TYPE, method.method); + writer.invokeStatic(CLASS_TYPE, new Method(method.name, method.methodType.toMethodDescriptorString())); } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java index 50b685c1ca52f..b0c8ad96e8337 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java @@ -81,14 +81,14 @@ void analyze(Locals locals) { PainlessLookupUtility.typeToCanonicalTypeName(captured.clazz), call, 1); // check casts between the interface method and the delegate method are legal - for (int i = 0; i < ref.interfaceMethod.arguments.size(); ++i) { - Class from = ref.interfaceMethod.arguments.get(i); + for (int i = 0; i < ref.interfaceMethod.typeParameters.size(); ++i) { + Class from = ref.interfaceMethod.typeParameters.get(i); Class to = ref.delegateTypeParameters.get(i); AnalyzerCaster.getLegalCast(location, from, to, false, true); } - if (ref.interfaceMethod.rtn != void.class) { - AnalyzerCaster.getLegalCast(location, ref.delegateReturnType, ref.interfaceMethod.rtn, false, true); + if (ref.interfaceMethod.returnType != void.class) { + AnalyzerCaster.getLegalCast(location, ref.delegateReturnType, ref.interfaceMethod.returnType, false, true); } } catch (IllegalArgumentException e) { throw createError(e); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java index 437fb84b79e2f..91a92aa7e7f6c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java @@ -23,6 +23,7 @@ import org.elasticsearch.painless.FunctionRef; import org.elasticsearch.painless.Globals; import org.elasticsearch.painless.Locals; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; import org.elasticsearch.painless.lookup.PainlessLookupUtility; @@ -70,8 +71,7 @@ void analyze(Locals locals) { throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] " + "to [" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "], not a functional interface"); } - PainlessMethod delegateMethod = - locals.getMethod(PainlessLookupUtility.buildPainlessMethodKey(call, interfaceMethod.arguments.size())); + LocalMethod delegateMethod = locals.getMethod(Locals.buildLocalMethodKey(call, interfaceMethod.typeParameters.size())); if (delegateMethod == null) { throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] " + "to [" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "], function not found"); @@ -79,14 +79,14 @@ void analyze(Locals locals) { ref = new FunctionRef(expected, interfaceMethod, delegateMethod, 0); // check casts between the interface method and the delegate method are legal - for (int i = 0; i < interfaceMethod.arguments.size(); ++i) { - Class from = interfaceMethod.arguments.get(i); - Class to = delegateMethod.arguments.get(i); + for (int i = 0; i < interfaceMethod.typeParameters.size(); ++i) { + Class from = interfaceMethod.typeParameters.get(i); + Class to = delegateMethod.typeParameters.get(i); AnalyzerCaster.getLegalCast(location, from, to, false, true); } - if (interfaceMethod.rtn != void.class) { - AnalyzerCaster.getLegalCast(location, delegateMethod.rtn, interfaceMethod.rtn, false, true); + if (interfaceMethod.returnType != void.class) { + AnalyzerCaster.getLegalCast(location, delegateMethod.returnType, interfaceMethod.returnType, false, true); } } else { // whitelist lookup diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java index ab1442be805eb..6fc4a3a648039 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java @@ -23,6 +23,7 @@ import org.elasticsearch.painless.FunctionRef; import org.elasticsearch.painless.Globals; import org.elasticsearch.painless.Locals; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.Locals.Variable; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; @@ -126,21 +127,21 @@ void analyze(Locals locals) { "[" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "], not a functional interface")); } // check arity before we manipulate parameters - if (interfaceMethod.arguments.size() != paramTypeStrs.size()) - throw new IllegalArgumentException("Incorrect number of parameters for [" + interfaceMethod.name + + if (interfaceMethod.typeParameters.size() != paramTypeStrs.size()) + throw new IllegalArgumentException("Incorrect number of parameters for [" + interfaceMethod.javaMethod.getName() + "] in [" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "]"); // for method invocation, its allowed to ignore the return value - if (interfaceMethod.rtn == void.class) { + if (interfaceMethod.returnType == void.class) { returnType = def.class; } else { - returnType = interfaceMethod.rtn; + returnType = interfaceMethod.returnType; } // replace any null types with the actual type actualParamTypeStrs = new ArrayList<>(paramTypeStrs.size()); for (int i = 0; i < paramTypeStrs.size(); i++) { String paramType = paramTypeStrs.get(i); if (paramType == null) { - actualParamTypeStrs.add(PainlessLookupUtility.typeToCanonicalTypeName(interfaceMethod.arguments.get(i))); + actualParamTypeStrs.add(PainlessLookupUtility.typeToCanonicalTypeName(interfaceMethod.typeParameters.get(i))); } else { actualParamTypeStrs.add(paramType); } @@ -183,20 +184,22 @@ void analyze(Locals locals) { } else { defPointer = null; try { - ref = new FunctionRef(expected, interfaceMethod, desugared.method, captures.size()); + LocalMethod localMethod = + new LocalMethod(desugared.name, desugared.returnType, desugared.typeParameters, desugared.methodType); + ref = new FunctionRef(expected, interfaceMethod, localMethod, captures.size()); } catch (IllegalArgumentException e) { throw createError(e); } // check casts between the interface method and the delegate method are legal - for (int i = 0; i < interfaceMethod.arguments.size(); ++i) { - Class from = interfaceMethod.arguments.get(i); + for (int i = 0; i < interfaceMethod.typeParameters.size(); ++i) { + Class from = interfaceMethod.typeParameters.get(i); Class to = desugared.parameters.get(i + captures.size()).clazz; AnalyzerCaster.getLegalCast(location, from, to, false, true); } - if (interfaceMethod.rtn != void.class) { - AnalyzerCaster.getLegalCast(location, desugared.rtnType, interfaceMethod.rtn, false, true); + if (interfaceMethod.returnType != void.class) { + AnalyzerCaster.getLegalCast(location, desugared.returnType, interfaceMethod.returnType, false, true); } actual = expected; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java index 1dec938adeb50..d6b0c592bf10b 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java @@ -99,7 +99,7 @@ void write(MethodWriter writer, Globals globals) { for (AExpression value : values) { writer.dup(); value.write(writer, globals); - method.write(writer); + writer.invokeMethodCall(method); writer.pop(); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java index 5013cc302c5b5..5c3c3e0ad4baa 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java @@ -122,7 +122,7 @@ void write(MethodWriter writer, Globals globals) { writer.dup(); key.write(writer, globals); value.write(writer, globals); - method.write(writer); + writer.invokeMethodCall(method); writer.pop(); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubCallInvoke.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubCallInvoke.java index 237efa61ffa7d..fe2ae52603b57 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubCallInvoke.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubCallInvoke.java @@ -56,14 +56,14 @@ void analyze(Locals locals) { for (int argument = 0; argument < arguments.size(); ++argument) { AExpression expression = arguments.get(argument); - expression.expected = method.arguments.get(argument); + expression.expected = method.typeParameters.get(argument); expression.internal = true; expression.analyze(locals); arguments.set(argument, expression.cast(locals)); } statement = true; - actual = method.rtn; + actual = method.returnType; } @Override @@ -78,11 +78,11 @@ void write(MethodWriter writer, Globals globals) { argument.write(writer, globals); } - method.write(writer); + writer.invokeMethodCall(method); } @Override public String toString() { - return singleLineToStringWithOptionalArgs(arguments, prefix, method.name); + return singleLineToStringWithOptionalArgs(arguments, prefix, method.javaMethod.getName()); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java index 509aad6415349..0738f55c2cf84 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java @@ -62,17 +62,17 @@ void analyze(Locals locals) { getter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("get", 1)); setter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("set", 2)); - if (getter != null && (getter.rtn == void.class || getter.arguments.size() != 1 || - getter.arguments.get(0) != int.class)) { + if (getter != null && (getter.returnType == void.class || getter.typeParameters.size() != 1 || + getter.typeParameters.get(0) != int.class)) { throw createError(new IllegalArgumentException("Illegal list get shortcut for type [" + canonicalClassName + "].")); } - if (setter != null && (setter.arguments.size() != 2 || setter.arguments.get(0) != int.class)) { + if (setter != null && (setter.typeParameters.size() != 2 || setter.typeParameters.get(0) != int.class)) { throw createError(new IllegalArgumentException("Illegal list set shortcut for type [" + canonicalClassName + "].")); } - if (getter != null && setter != null && (!getter.arguments.get(0).equals(setter.arguments.get(0)) - || !getter.rtn.equals(setter.arguments.get(1)))) { + if (getter != null && setter != null && (!getter.typeParameters.get(0).equals(setter.typeParameters.get(0)) + || !getter.returnType.equals(setter.typeParameters.get(1)))) { throw createError(new IllegalArgumentException("Shortcut argument types must match.")); } @@ -81,7 +81,7 @@ void analyze(Locals locals) { index.analyze(locals); index = index.cast(locals); - actual = setter != null ? setter.arguments.get(1) : getter.rtn; + actual = setter != null ? setter.typeParameters.get(1) : getter.returnType; } else { throw createError(new IllegalArgumentException("Illegal list shortcut for type [" + canonicalClassName + "].")); } @@ -119,21 +119,18 @@ void setup(MethodWriter writer, Globals globals) { @Override void load(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); + writer.invokeMethodCall(getter); - getter.write(writer); - - if (getter.rtn == getter.handle.type().returnType()) { - writer.checkCast(MethodWriter.getType(getter.rtn)); + if (getter.returnType == getter.javaMethod.getReturnType()) { + writer.checkCast(MethodWriter.getType(getter.returnType)); } } @Override void store(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); - - setter.write(writer); - - writer.writePop(MethodWriter.getType(setter.rtn).getSize()); + writer.invokeMethodCall(setter); + writer.writePop(MethodWriter.getType(setter.returnType).getSize()); } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java index 2d7f2250c6c38..04ccbc9f534a0 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java @@ -61,25 +61,25 @@ void analyze(Locals locals) { getter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("get", 1)); setter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("put", 2)); - if (getter != null && (getter.rtn == void.class || getter.arguments.size() != 1)) { + if (getter != null && (getter.returnType == void.class || getter.typeParameters.size() != 1)) { throw createError(new IllegalArgumentException("Illegal map get shortcut for type [" + canonicalClassName + "].")); } - if (setter != null && setter.arguments.size() != 2) { + if (setter != null && setter.typeParameters.size() != 2) { throw createError(new IllegalArgumentException("Illegal map set shortcut for type [" + canonicalClassName + "].")); } - if (getter != null && setter != null && - (!getter.arguments.get(0).equals(setter.arguments.get(0)) || !getter.rtn.equals(setter.arguments.get(1)))) { + if (getter != null && setter != null && (!getter.typeParameters.get(0).equals(setter.typeParameters.get(0)) || + !getter.returnType.equals(setter.typeParameters.get(1)))) { throw createError(new IllegalArgumentException("Shortcut argument types must match.")); } if ((read || write) && (!read || getter != null) && (!write || setter != null)) { - index.expected = setter != null ? setter.arguments.get(0) : getter.arguments.get(0); + index.expected = setter != null ? setter.typeParameters.get(0) : getter.typeParameters.get(0); index.analyze(locals); index = index.cast(locals); - actual = setter != null ? setter.arguments.get(1) : getter.rtn; + actual = setter != null ? setter.typeParameters.get(1) : getter.returnType; } else { throw createError(new IllegalArgumentException("Illegal map shortcut for type [" + canonicalClassName + "].")); } @@ -90,11 +90,10 @@ void write(MethodWriter writer, Globals globals) { index.write(writer, globals); writer.writeDebugInfo(location); + writer.invokeMethodCall(getter); - getter.write(writer); - - if (getter.rtn != getter.handle.type().returnType()) { - writer.checkCast(MethodWriter.getType(getter.rtn)); + if (getter.returnType != getter.javaMethod.getReturnType()) { + writer.checkCast(MethodWriter.getType(getter.returnType)); } } @@ -121,21 +120,18 @@ void setup(MethodWriter writer, Globals globals) { @Override void load(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); + writer.invokeMethodCall(getter); - getter.write(writer); - - if (getter.rtn != getter.handle.type().returnType()) { - writer.checkCast(MethodWriter.getType(getter.rtn)); + if (getter.returnType != getter.javaMethod.getReturnType()) { + writer.checkCast(MethodWriter.getType(getter.returnType)); } } @Override void store(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); - - setter.write(writer); - - writer.writePop(MethodWriter.getType(setter.rtn).getSize()); + writer.invokeMethodCall(setter); + writer.writePop(MethodWriter.getType(setter.returnType).getSize()); } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubShortcut.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubShortcut.java index eb5668c554c20..1697566047798 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubShortcut.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubShortcut.java @@ -53,22 +53,22 @@ void extractVariables(Set variables) { @Override void analyze(Locals locals) { - if (getter != null && (getter.rtn == void.class || !getter.arguments.isEmpty())) { + if (getter != null && (getter.returnType == void.class || !getter.typeParameters.isEmpty())) { throw createError(new IllegalArgumentException( "Illegal get shortcut on field [" + value + "] for type [" + type + "].")); } - if (setter != null && (setter.rtn != void.class || setter.arguments.size() != 1)) { + if (setter != null && (setter.returnType != void.class || setter.typeParameters.size() != 1)) { throw createError(new IllegalArgumentException( "Illegal set shortcut on field [" + value + "] for type [" + type + "].")); } - if (getter != null && setter != null && setter.arguments.get(0) != getter.rtn) { + if (getter != null && setter != null && setter.typeParameters.get(0) != getter.returnType) { throw createError(new IllegalArgumentException("Shortcut argument types must match.")); } if ((getter != null || setter != null) && (!read || getter != null) && (!write || setter != null)) { - actual = setter != null ? setter.arguments.get(0) : getter.rtn; + actual = setter != null ? setter.typeParameters.get(0) : getter.returnType; } else { throw createError(new IllegalArgumentException("Illegal shortcut on field [" + value + "] for type [" + type + "].")); } @@ -78,10 +78,10 @@ void analyze(Locals locals) { void write(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); - getter.write(writer); + writer.invokeMethodCall(getter); - if (!getter.rtn.equals(getter.handle.type().returnType())) { - writer.checkCast(MethodWriter.getType(getter.rtn)); + if (!getter.returnType.equals(getter.javaMethod.getReturnType())) { + writer.checkCast(MethodWriter.getType(getter.returnType)); } } @@ -109,10 +109,10 @@ void setup(MethodWriter writer, Globals globals) { void load(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); - getter.write(writer); + writer.invokeMethodCall(getter); - if (getter.rtn != getter.handle.type().returnType()) { - writer.checkCast(MethodWriter.getType(getter.rtn)); + if (getter.returnType != getter.javaMethod.getReturnType()) { + writer.checkCast(MethodWriter.getType(getter.returnType)); } } @@ -120,9 +120,9 @@ void load(MethodWriter writer, Globals globals) { void store(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); - setter.write(writer); + writer.invokeMethodCall(setter); - writer.writePop(MethodWriter.getType(setter.rtn).getSize()); + writer.writePop(MethodWriter.getType(setter.returnType).getSize()); } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java index 600ca95504ac6..b9336fb5aac25 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java @@ -31,14 +31,12 @@ import org.elasticsearch.painless.WriterConstants; import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.lookup.PainlessLookupUtility; -import org.elasticsearch.painless.lookup.PainlessMethod; import org.elasticsearch.painless.node.SSource.Reserved; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.Handle; import org.objectweb.asm.Opcodes; import java.lang.invoke.MethodType; -import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -92,9 +90,12 @@ public int getMaxLoopCounter() { private final List statements; public final boolean synthetic; - Class rtnType = null; + Class returnType; + List> typeParameters; + MethodType methodType; + + org.objectweb.asm.commons.Method method; List parameters = new ArrayList<>(); - PainlessMethod method = null; private Variable loop = null; @@ -120,7 +121,7 @@ void extractVariables(Set variables) { void generateSignature(PainlessLookup painlessLookup) { try { - rtnType = painlessLookup.getJavaClassFromPainlessType(rtnTypeStr); + returnType = painlessLookup.getJavaClassFromPainlessType(rtnTypeStr); } catch (IllegalArgumentException exception) { throw createError(new IllegalArgumentException("Illegal return type [" + rtnTypeStr + "] for function [" + name + "].")); } @@ -145,11 +146,9 @@ void generateSignature(PainlessLookup painlessLookup) { } } - int modifiers = Modifier.STATIC | Modifier.PRIVATE; - org.objectweb.asm.commons.Method method = new org.objectweb.asm.commons.Method(name, MethodType.methodType( - PainlessLookupUtility.typeToJavaType(rtnType), paramClasses).toMethodDescriptorString()); - MethodType methodType = MethodType.methodType(PainlessLookupUtility.typeToJavaType(rtnType), paramClasses); - this.method = new PainlessMethod(name, null, null, rtnType, paramTypes, method, modifiers, null, methodType); + methodType = MethodType.methodType(PainlessLookupUtility.typeToJavaType(returnType), paramClasses); + method = new org.objectweb.asm.commons.Method(name, MethodType.methodType( + PainlessLookupUtility.typeToJavaType(returnType), paramClasses).toMethodDescriptorString()); } @Override @@ -177,7 +176,7 @@ void analyze(Locals locals) { allEscape = statement.allEscape; } - if (!methodEscape && rtnType != void.class) { + if (!methodEscape && returnType != void.class) { throw createError(new IllegalArgumentException("Not all paths provide a return value for method [" + name + "].")); } @@ -192,7 +191,7 @@ void write (ClassVisitor writer, CompilerSettings settings, Globals globals) { if (synthetic) { access |= Opcodes.ACC_SYNTHETIC; } - final MethodWriter function = new MethodWriter(access, method.method, writer, globals.getStatements(), settings); + final MethodWriter function = new MethodWriter(access, method, writer, globals.getStatements(), settings); function.visitCode(); write(function, globals); function.endMethod(); @@ -212,7 +211,7 @@ void write(MethodWriter function, Globals globals) { } if (!methodEscape) { - if (rtnType == void.class) { + if (returnType == void.class) { function.returnValue(); } else { throw createError(new IllegalStateException("Illegal tree structure.")); @@ -225,11 +224,7 @@ void write(MethodWriter function, Globals globals) { } private void initializeConstant(MethodWriter writer) { - final Handle handle = new Handle(Opcodes.H_INVOKESTATIC, - CLASS_TYPE.getInternalName(), - name, - method.method.getDescriptor(), - false); + final Handle handle = new Handle(Opcodes.H_INVOKESTATIC, CLASS_TYPE.getInternalName(), name, method.getDescriptor(), false); writer.push(handle); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java index c354e78a961a3..fe735c0db313e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java @@ -23,6 +23,7 @@ import org.elasticsearch.painless.Constant; import org.elasticsearch.painless.Globals; import org.elasticsearch.painless.Locals; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.Locals.Variable; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; @@ -30,8 +31,6 @@ import org.elasticsearch.painless.SimpleChecksAdapter; import org.elasticsearch.painless.WriterConstants; import org.elasticsearch.painless.lookup.PainlessLookup; -import org.elasticsearch.painless.lookup.PainlessLookupUtility; -import org.elasticsearch.painless.lookup.PainlessMethod; import org.elasticsearch.painless.node.SFunction.FunctionReserved; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.ClassWriter; @@ -165,14 +164,15 @@ void extractVariables(Set variables) { } public void analyze(PainlessLookup painlessLookup) { - Map methods = new HashMap<>(); + Map methods = new HashMap<>(); for (SFunction function : functions) { function.generateSignature(painlessLookup); - String key = PainlessLookupUtility.buildPainlessMethodKey(function.name, function.parameters.size()); + String key = Locals.buildLocalMethodKey(function.name, function.parameters.size()); - if (methods.put(key, function.method) != null) { + if (methods.put(key, + new LocalMethod(function.name, function.returnType, function.typeParameters, function.methodType)) != null) { throw createError(new IllegalArgumentException("Duplicate functions with name [" + function.name + "].")); } } @@ -184,7 +184,7 @@ public void analyze(PainlessLookup painlessLookup) { void analyze(Locals program) { for (SFunction function : functions) { Locals functionLocals = - Locals.newFunctionScope(program, function.rtnType, function.parameters, function.reserved.getMaxLoopCounter()); + Locals.newFunctionScope(program, function.returnType, function.parameters, function.reserved.getMaxLoopCounter()); function.analyze(functionLocals); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSubEachIterable.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSubEachIterable.java index 12e3154eb562e..5450f690f6c1c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSubEachIterable.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSubEachIterable.java @@ -99,7 +99,7 @@ void write(MethodWriter writer, Globals globals) { .getMethodType(org.objectweb.asm.Type.getType(Iterator.class), org.objectweb.asm.Type.getType(Object.class)); writer.invokeDefCall("iterator", methodType, DefBootstrap.ITERATOR); } else { - method.write(writer); + writer.invokeMethodCall(method); } writer.visitVarInsn(MethodWriter.getType(iterator.clazz).getOpcode(Opcodes.ISTORE), iterator.getSlot()); diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java index 654291543017f..1f8410092a052 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java @@ -57,8 +57,8 @@ public class PainlessDocGenerator { private static final PainlessLookup PAINLESS_LOOKUP = PainlessLookupBuilder.buildFromWhitelists(Whitelist.BASE_WHITELISTS); private static final Logger logger = ESLoggerFactory.getLogger(PainlessDocGenerator.class); private static final Comparator FIELD_NAME = comparing(f -> f.name); - private static final Comparator METHOD_NAME = comparing(m -> m.name); - private static final Comparator METHOD_NUMBER_OF_PARAMS = comparing(m -> m.arguments.size()); + private static final Comparator METHOD_NAME = comparing(m -> m.javaMethod.getName()); + private static final Comparator METHOD_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size()); private static final Comparator CONSTRUCTOR_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size()); public static void main(String[] args) throws IOException { @@ -114,10 +114,10 @@ public static void main(String[] args) throws IOException { struct.constructors.values().stream().sorted(CONSTRUCTOR_NUMBER_OF_PARAMS).forEach(documentConstructor); Map> inherited = new TreeMap<>(); struct.methods.values().stream().sorted(METHOD_NAME.thenComparing(METHOD_NUMBER_OF_PARAMS)).forEach(method -> { - if (method.target == clazz) { + if (method.targetClass == clazz) { documentMethod(typeStream, method); } else { - inherited.put(canonicalClassName, method.target); + inherited.put(canonicalClassName, method.targetClass); } }); @@ -212,11 +212,11 @@ private static void documentMethod(PrintStream stream, PainlessMethod method) { emitAnchor(stream, method); stream.print("]]"); - if (null == method.augmentation && Modifier.isStatic(method.modifiers)) { + if (method.targetClass == method.javaMethod.getDeclaringClass() && Modifier.isStatic(method.javaMethod.getModifiers())) { stream.print("static "); } - emitType(stream, method.rtn); + emitType(stream, method.returnType); stream.print(' '); String javadocRoot = javadocRoot(method); @@ -227,7 +227,7 @@ private static void documentMethod(PrintStream stream, PainlessMethod method) { stream.print("]("); boolean first = true; - for (Class arg : method.arguments) { + for (Class arg : method.typeParameters) { if (first) { first = false; } else { @@ -269,11 +269,11 @@ private static void emitAnchor(PrintStream stream, PainlessConstructor construct * Anchor text for a {@link PainlessMethod}. */ private static void emitAnchor(PrintStream stream, PainlessMethod method) { - emitAnchor(stream, method.target); + emitAnchor(stream, method.targetClass); stream.print('-'); stream.print(methodName(method)); stream.print('-'); - stream.print(method.arguments.size()); + stream.print(method.typeParameters.size()); } /** @@ -290,7 +290,7 @@ private static String constructorName(PainlessConstructor constructor) { } private static String methodName(PainlessMethod method) { - return PainlessLookupUtility.typeToCanonicalTypeName(method.target); + return PainlessLookupUtility.typeToCanonicalTypeName(method.targetClass); } /** @@ -359,16 +359,16 @@ private static void emitJavadocLink(PrintStream stream, String root, PainlessMet stream.print("link:{"); stream.print(root); stream.print("-javadoc}/"); - stream.print(classUrlPath(method.augmentation != null ? method.augmentation : method.target)); + stream.print(classUrlPath(method.javaMethod.getDeclaringClass())); stream.print(".html#"); stream.print(methodName(method)); stream.print("%2D"); boolean first = true; - if (method.augmentation != null) { + if (method.targetClass != method.javaMethod.getDeclaringClass()) { first = false; - stream.print(method.target.getName()); + stream.print(method.javaMethod.getDeclaringClass().getName()); } - for (Class clazz: method.arguments) { + for (Class clazz: method.typeParameters) { if (first) { first = false; } else { @@ -400,10 +400,10 @@ private static void emitJavadocLink(PrintStream stream, String root, PainlessFie * Pick the javadoc root for a {@link PainlessMethod}. */ private static String javadocRoot(PainlessMethod method) { - if (method.augmentation != null) { + if (method.targetClass != method.javaMethod.getDeclaringClass()) { return "painless"; } - return javadocRoot(method.target); + return javadocRoot(method.targetClass); } /** From 266a92d23f38644218caa2557584e91ff7a5422a Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 27 Jul 2018 14:49:10 -0700 Subject: [PATCH 5/7] Fixes. --- .../src/main/java/org/elasticsearch/painless/Def.java | 2 +- .../src/main/java/org/elasticsearch/painless/FunctionRef.java | 2 +- .../main/java/org/elasticsearch/painless/node/SFunction.java | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index e4e4414afb9c5..5c9cb9c747531 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java @@ -334,7 +334,7 @@ static MethodHandle lookupReference(PainlessLookup painlessLookup, MethodHandles int arity = interfaceMethod.typeParameters.size(); PainlessMethod implMethod = lookupMethodInternal(painlessLookup, receiverClass, name, arity); return lookupReferenceInternal(painlessLookup, methodHandlesLookup, interfaceType, - PainlessLookupUtility.typeToCanonicalTypeName(implMethod.javaMethod.getDeclaringClass()), + PainlessLookupUtility.typeToCanonicalTypeName(implMethod.targetClass), implMethod.javaMethod.getName(), receiverClass); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java index dff5ff90cf22b..304b03562409c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java @@ -142,7 +142,7 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMe delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); - delegateClassName = delegateMethod.javaMethod.getName(); + delegateClassName = delegateMethod.javaMethod.getDeclaringClass().getName(); isDelegateInterface = delegateMethod.javaMethod.getDeclaringClass().isInterface(); if (Modifier.isStatic(delegateMethod.javaMethod.getModifiers())) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java index b9336fb5aac25..d61a424f83ddb 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java @@ -146,6 +146,7 @@ void generateSignature(PainlessLookup painlessLookup) { } } + typeParameters = paramTypes; methodType = MethodType.methodType(PainlessLookupUtility.typeToJavaType(returnType), paramClasses); method = new org.objectweb.asm.commons.Method(name, MethodType.methodType( PainlessLookupUtility.typeToJavaType(returnType), paramClasses).toMethodDescriptorString()); From 0c9c17438d1cfc6f1b974a9be3cd6b4e82eb3054 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 30 Jul 2018 12:19:17 -0700 Subject: [PATCH 6/7] Reponse to PR comments. --- .../src/main/java/org/elasticsearch/painless/Def.java | 2 +- .../src/main/java/org/elasticsearch/painless/FunctionRef.java | 4 +++- .../java/org/elasticsearch/painless/lookup/PainlessClass.java | 4 ++-- .../elasticsearch/painless/lookup/PainlessClassBuilder.java | 2 +- .../elasticsearch/painless/lookup/PainlessLookupBuilder.java | 2 +- .../elasticsearch/painless/lookup/PainlessLookupUtility.java | 4 ++-- .../elasticsearch/painless/node/ECapturingFunctionRef.java | 2 +- .../java/org/elasticsearch/painless/node/EFunctionRef.java | 2 +- .../main/java/org/elasticsearch/painless/node/EListInit.java | 3 ++- .../main/java/org/elasticsearch/painless/node/EMapInit.java | 3 ++- .../main/java/org/elasticsearch/painless/node/ENewObj.java | 2 +- 11 files changed, 17 insertions(+), 13 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index 863f973a5c2f3..9be2559e67a27 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java +++ b/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 = FunctionRef.getFunctionRef(painlessLookup, clazz, type, call, captures.length); + ref = FunctionRef.resolveFromLookup(painlessLookup, clazz, type, call, captures.length); } final CallSite callSite = LambdaBootstrap.lambdaBootstrap( methodHandlesLookup, diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java index 2752b046f8e24..0da91438326c5 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java @@ -85,7 +85,9 @@ public class FunctionRef { * @param call the right hand side of a method reference expression * @param numCaptures number of captured arguments */ - public static FunctionRef getFunctionRef(PainlessLookup painlessLookup, Class expected, String type, String call, int 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); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java index a10413acb8498..835bfb5c505a4 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java @@ -24,7 +24,7 @@ import java.util.Map; public final class PainlessClass { - public final Map constructors; + public final Map constructors; public final Map staticMethods; public final Map methods; @@ -37,7 +37,7 @@ public final class PainlessClass { public final PainlessMethod functionalMethod; - PainlessClass(Map constructors, + PainlessClass(Map constructors, Map staticMethods, Map methods, Map staticFields, Map fields, Map getterMethodHandles, Map setterMethodHandles, diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java index 9cc4e0fdbb7fe..866f711ba0f3e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java @@ -24,7 +24,7 @@ import java.util.Map; final class PainlessClassBuilder { - final Map constructors; + final Map constructors; final Map staticMethods; final Map methods; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 28ee228601a22..b0a11dea58a7e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -376,7 +376,7 @@ public void addPainlessConstructor(Class targetClass, List> typePara "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", nsme); } - int painlessConstructorKey = buildPainlessConstructorKey(typeParametersSize); + String painlessConstructorKey = buildPainlessConstructorKey(typeParametersSize); PainlessConstructor painlessConstructor = painlessClassBuilder.constructors.get(painlessConstructorKey); if (painlessConstructor == null) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java index 7a4c951235e13..0a181c5f1b02d 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java @@ -339,8 +339,8 @@ public static boolean isConstantType(Class type) { /** * Constructs a painless constructor key used to lookup painless constructors from a painless class. */ - public static int buildPainlessConstructorKey(int constructorArity) { - return constructorArity; + public static String buildPainlessConstructorKey(int constructorArity) { + return CONSTRUCTOR_NAME + "/" + constructorArity; } /** diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java index 50b685c1ca52f..a4f7b1300452a 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java @@ -77,7 +77,7 @@ void analyze(Locals locals) { // static case if (captured.clazz != def.class) { try { - ref = FunctionRef.getFunctionRef(locals.getPainlessLookup(), expected, + ref = FunctionRef.resolveFromLookup(locals.getPainlessLookup(), expected, PainlessLookupUtility.typeToCanonicalTypeName(captured.clazz), call, 1); // check casts between the interface method and the delegate method are legal diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java index 437fb84b79e2f..23fb8a4180e48 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java @@ -90,7 +90,7 @@ void analyze(Locals locals) { } } else { // whitelist lookup - ref = FunctionRef.getFunctionRef(locals.getPainlessLookup(), expected, type, call, 0); + ref = FunctionRef.resolveFromLookup(locals.getPainlessLookup(), expected, type, call, 0); } } catch (IllegalArgumentException e) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java index 1dec938adeb50..bd4bea95ea0ef 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java @@ -64,7 +64,8 @@ void analyze(Locals locals) { actual = ArrayList.class; - constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get(0); + constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get( + PainlessLookupUtility.buildPainlessConstructorKey(0)); if (constructor == null) { throw createError(new IllegalStateException("Illegal tree structure.")); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java index 5013cc302c5b5..5bef62a4b1812 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java @@ -70,7 +70,8 @@ void analyze(Locals locals) { actual = HashMap.class; - constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get(0); + constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get( + PainlessLookupUtility.buildPainlessConstructorKey(0)); if (constructor == null) { throw createError(new IllegalStateException("Illegal tree structure.")); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java index f9d882f39efec..4e08f25738673 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java @@ -66,7 +66,7 @@ void analyze(Locals locals) { } PainlessClass struct = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual); - constructor = struct.constructors.get(arguments.size()); + constructor = struct.constructors.get(PainlessLookupUtility.buildPainlessConstructorKey(arguments.size())); if (constructor != null) { Class[] types = new Class[constructor.typeParameters.size()]; From aff5c3e9be0cf89047d1c458228e6d97b26e5195 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 31 Jul 2018 14:00:55 -0700 Subject: [PATCH 7/7] Reponse to PR comments. --- .../elasticsearch/painless/FunctionRef.java | 98 +++++++++---------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java index e3fb5e972539f..d4671f05b6c9d 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java @@ -109,24 +109,24 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessCo Constructor javaConstructor = delegateConstructor.javaConstructor; MethodType delegateMethodType = delegateConstructor.methodType; - interfaceMethodName = interfaceMethod.javaMethod.getName(); - factoryMethodType = MethodType.methodType(expected, + this.interfaceMethodName = interfaceMethod.javaMethod.getName(); + this.factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); + this.interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); - delegateClassName = javaConstructor.getDeclaringClass().getName(); - isDelegateInterface = false; - delegateInvokeType = H_NEWINVOKESPECIAL; - delegateMethodName = PainlessLookupUtility.CONSTRUCTOR_NAME; + this.delegateClassName = javaConstructor.getDeclaringClass().getName(); + this.isDelegateInterface = false; + this.delegateInvokeType = H_NEWINVOKESPECIAL; + this.delegateMethodName = PainlessLookupUtility.CONSTRUCTOR_NAME; this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); this.interfaceMethod = interfaceMethod; - delegateTypeParameters = delegateConstructor.typeParameters; - delegateReturnType = void.class; + this.delegateTypeParameters = delegateConstructor.typeParameters; + this.delegateReturnType = void.class; - factoryDescriptor = factoryMethodType.toMethodDescriptorString(); - interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); - delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); + this.factoryDescriptor = factoryMethodType.toMethodDescriptorString(); + this.interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); + this.delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); } /** @@ -139,32 +139,32 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessCo public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMethod delegateMethod, int numCaptures) { MethodType delegateMethodType = delegateMethod.methodType; - interfaceMethodName = interfaceMethod.javaMethod.getName(); - factoryMethodType = MethodType.methodType(expected, + this.interfaceMethodName = interfaceMethod.javaMethod.getName(); + this.factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); + this.interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); - delegateClassName = delegateMethod.javaMethod.getDeclaringClass().getName(); - isDelegateInterface = delegateMethod.javaMethod.getDeclaringClass().isInterface(); + this.delegateClassName = delegateMethod.javaMethod.getDeclaringClass().getName(); + this.isDelegateInterface = delegateMethod.javaMethod.getDeclaringClass().isInterface(); if (Modifier.isStatic(delegateMethod.javaMethod.getModifiers())) { - delegateInvokeType = H_INVOKESTATIC; + this.delegateInvokeType = H_INVOKESTATIC; } else if (delegateMethod.javaMethod.getDeclaringClass().isInterface()) { - delegateInvokeType = H_INVOKEINTERFACE; + this.delegateInvokeType = H_INVOKEINTERFACE; } else { - delegateInvokeType = H_INVOKEVIRTUAL; + this.delegateInvokeType = H_INVOKEVIRTUAL; } - delegateMethodName = delegateMethod.javaMethod.getName(); + this.delegateMethodName = delegateMethod.javaMethod.getName(); this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); this.interfaceMethod = interfaceMethod; - delegateTypeParameters = delegateMethod.typeParameters; - delegateReturnType = delegateMethod.returnType; + this.delegateTypeParameters = delegateMethod.typeParameters; + this.delegateReturnType = delegateMethod.returnType; - factoryDescriptor = factoryMethodType.toMethodDescriptorString(); - interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); - delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); + this.factoryDescriptor = factoryMethodType.toMethodDescriptorString(); + this.interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); + this.delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); } /** @@ -177,25 +177,25 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessMe public FunctionRef(Class expected, PainlessMethod interfaceMethod, LocalMethod delegateMethod, int numCaptures) { MethodType delegateMethodType = delegateMethod.methodType; - interfaceMethodName = interfaceMethod.javaMethod.getName(); - factoryMethodType = MethodType.methodType(expected, + this.interfaceMethodName = interfaceMethod.javaMethod.getName(); + this.factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); + this.interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); - delegateClassName = CLASS_NAME; - isDelegateInterface = false; - delegateInvokeType = H_INVOKESTATIC; + this.delegateClassName = CLASS_NAME; + this.isDelegateInterface = false; + this.delegateInvokeType = H_INVOKESTATIC; - delegateMethodName = delegateMethod.name; + this.delegateMethodName = delegateMethod.name; this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); this.interfaceMethod = interfaceMethod; - delegateTypeParameters = delegateMethod.typeParameters; - delegateReturnType = delegateMethod.returnType; + this.delegateTypeParameters = delegateMethod.typeParameters; + this.delegateReturnType = delegateMethod.returnType; - factoryDescriptor = factoryMethodType.toMethodDescriptorString(); - interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); - delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); + this.factoryDescriptor = factoryMethodType.toMethodDescriptorString(); + this.interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); + this.delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); } /** @@ -204,24 +204,24 @@ public FunctionRef(Class expected, PainlessMethod interfaceMethod, LocalMetho */ public FunctionRef(Class expected, PainlessMethod interfaceMethod, String delegateMethodName, MethodType delegateMethodType, int numCaptures) { - interfaceMethodName = interfaceMethod.javaMethod.getName(); - factoryMethodType = MethodType.methodType(expected, + this.interfaceMethodName = interfaceMethod.javaMethod.getName(); + this.factoryMethodType = MethodType.methodType(expected, delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); - interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); + this.interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); - delegateClassName = CLASS_NAME; - delegateInvokeType = H_INVOKESTATIC; + this.delegateClassName = CLASS_NAME; + this.delegateInvokeType = H_INVOKESTATIC; this.delegateMethodName = delegateMethodName; this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); - isDelegateInterface = false; + this.isDelegateInterface = false; this.interfaceMethod = null; - delegateTypeParameters = null; - delegateReturnType = null; + this.delegateTypeParameters = null; + this.delegateReturnType = null; - factoryDescriptor = null; - interfaceType = null; - delegateType = null; + this.factoryDescriptor = null; + this.interfaceType = null; + this.delegateType = null; } /**