From c69e62d96fef43e526bd130acc429a5a8536cc56 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 30 Jul 2018 14:46:24 -0700 Subject: [PATCH] Painless: Add PainlessConstructor (#32447) PainlessMethod was being used as both a method and a constructor, and while there are similarities, there are also some major differences. This allows the reflection objects to be stored reducing the number of other pieces of data stored in a PainlessMethod as they are now redundant. This temporarily increases some of the code in FunctionRef and PainlessDocGenerator as they now differentiate between constructors and methods, BUT is also makes the code more maintainable because there aren't checks in several places anymore to differentiate. --- .../java/org/elasticsearch/painless/Def.java | 2 +- .../elasticsearch/painless/FunctionRef.java | 119 +++++++++++++----- .../painless/lookup/PainlessClass.java | 6 +- .../painless/lookup/PainlessClassBuilder.java | 4 +- .../painless/lookup/PainlessConstructor.java | 39 ++++++ .../lookup/PainlessLookupBuilder.java | 59 ++++++--- .../lookup/PainlessLookupUtility.java | 7 ++ .../painless/node/ECapturingFunctionRef.java | 6 +- .../painless/node/EFunctionRef.java | 2 +- .../painless/node/EListInit.java | 11 +- .../elasticsearch/painless/node/EMapInit.java | 11 +- .../elasticsearch/painless/node/ENewObj.java | 18 +-- .../painless/PainlessDocGenerator.java | 99 +++++++++++++-- 13 files changed, 307 insertions(+), 76 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/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index 8a90f53b4fd..9be2559e67a 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 @@ public final class Def { ref = new FunctionRef(clazz, interfaceMethod, call, handle.type(), captures.length); } else { // whitelist lookup - ref = new FunctionRef(painlessLookup, clazz, type, call, captures.length); + ref = FunctionRef.resolveFromLookup(painlessLookup, clazz, type, call, captures.length); } final CallSite callSite = LambdaBootstrap.lambdaBootstrap( methodHandlesLookup, 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 7b7e8d25f39..0da91438326 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,13 +20,16 @@ package org.elasticsearch.painless; import org.elasticsearch.painless.lookup.PainlessClass; +import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.painless.lookup.PainlessMethod; import org.objectweb.asm.Type; import java.lang.invoke.MethodType; +import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; +import java.util.List; import static org.elasticsearch.painless.WriterConstants.CLASS_NAME; import static org.objectweb.asm.Opcodes.H_INVOKEINTERFACE; @@ -59,8 +62,10 @@ public class FunctionRef { /** interface method */ public final PainlessMethod interfaceMethod; - /** delegate method */ - public final PainlessMethod delegateMethod; + /** delegate method type parameters */ + public final List> delegateTypeParameters; + /** delegate method return type */ + public final Class delegateReturnType; /** factory method type descriptor */ public final String factoryDescriptor; @@ -80,9 +85,47 @@ public class FunctionRef { * @param call the right hand side of a method reference expression * @param numCaptures number of captured arguments */ - public FunctionRef(PainlessLookup painlessLookup, Class expected, String type, String call, int numCaptures) { - this(expected, painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod, - lookup(painlessLookup, expected, type, call, numCaptures > 0), numCaptures); + public static FunctionRef resolveFromLookup( + PainlessLookup painlessLookup, Class expected, String type, String call, int numCaptures) { + + if ("new".equals(call)) { + return new FunctionRef(expected, painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod, + lookup(painlessLookup, expected, type), numCaptures); + } else { + return new FunctionRef(expected, painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod, + lookup(painlessLookup, expected, type, call, numCaptures > 0), numCaptures); + } + } + + /** + * Creates a new FunctionRef (already resolved) + * @param expected functional interface type to implement + * @param interfaceMethod functional interface method + * @param delegateConstructor implementation constructor + * @param numCaptures number of captured arguments + */ + public FunctionRef(Class expected, PainlessMethod interfaceMethod, PainlessConstructor delegateConstructor, int numCaptures) { + Constructor javaConstructor = delegateConstructor.javaConstructor; + MethodType delegateMethodType = delegateConstructor.methodType; + + interfaceMethodName = interfaceMethod.name; + factoryMethodType = MethodType.methodType(expected, + delegateMethodType.dropParameterTypes(numCaptures, delegateMethodType.parameterCount())); + interfaceMethodType = interfaceMethod.methodType.dropParameterTypes(0, 1); + + delegateClassName = javaConstructor.getDeclaringClass().getName(); + isDelegateInterface = false; + delegateInvokeType = H_NEWINVOKESPECIAL; + delegateMethodName = PainlessLookupUtility.CONSTRUCTOR_NAME; + this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); + + this.interfaceMethod = interfaceMethod; + delegateTypeParameters = delegateConstructor.typeParameters; + delegateReturnType = void.class; + + factoryDescriptor = factoryMethodType.toMethodDescriptorString(); + interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); + delegateType = Type.getMethodType(this.delegateMethodType.toMethodDescriptorString()); } /** @@ -112,9 +155,7 @@ public class FunctionRef { 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; @@ -126,7 +167,8 @@ public class FunctionRef { this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); this.interfaceMethod = interfaceMethod; - this.delegateMethod = delegateMethod; + delegateTypeParameters = delegateMethod.arguments; + delegateReturnType = delegateMethod.rtn; factoryDescriptor = factoryMethodType.toMethodDescriptorString(); interfaceType = Type.getMethodType(interfaceMethodType.toMethodDescriptorString()); @@ -151,13 +193,37 @@ public class FunctionRef { isDelegateInterface = false; this.interfaceMethod = null; - delegateMethod = null; + delegateTypeParameters = null; + delegateReturnType = null; factoryDescriptor = null; interfaceType = null; delegateType = null; } + /** + * Looks up {@code type} from the whitelist, and returns a matching constructor. + */ + private static PainlessConstructor lookup(PainlessLookup painlessLookup, Class expected, String type) { + // check its really a functional interface + // for e.g. Comparable + PainlessMethod method = painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod; + if (method == null) { + throw new IllegalArgumentException("Cannot convert function reference [" + type + "::new] " + + "to [" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "], not a functional interface"); + } + + // lookup requested constructor + PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(painlessLookup.getJavaClassFromPainlessType(type)); + PainlessConstructor impl = struct.constructors.get(PainlessLookupUtility.buildPainlessConstructorKey(method.arguments.size())); + + if (impl == null) { + throw new IllegalArgumentException("Unknown reference [" + type + "::new] matching [" + expected + "]"); + } + + return impl; + } + /** * Looks up {@code type::call} from the whitelist, and returns a matching method. */ @@ -174,27 +240,22 @@ public class FunctionRef { // lookup requested method PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(painlessLookup.getJavaClassFromPainlessType(type)); final PainlessMethod impl; - // ctor ref - if ("new".equals(call)) { - impl = struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("", 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/PainlessClass.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java index 24dcf0ebdba..835bfb5c505 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.Collections; 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 2f41ed5dca8..866f711ba0f 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.HashMap; 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 00000000000..76597c1a29d --- /dev/null +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java @@ -0,0 +1,39 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.painless.lookup; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodType; +import java.lang.reflect.Constructor; +import java.util.List; + +public class PainlessConstructor { + public final Constructor javaConstructor; + public final List> typeParameters; + public final MethodHandle methodHandle; + public final MethodType methodType; + + PainlessConstructor(Constructor javaConstructor, List> typeParameters, MethodHandle methodHandle, MethodType methodType) { + this.javaConstructor = javaConstructor; + this.typeParameters = typeParameters; + this.methodHandle = methodHandle; + this.methodType = methodType; + } +} 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 ba48fbea734..b0a11dea58a 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,15 +40,47 @@ import java.util.Map; import java.util.Objects; import java.util.regex.Pattern; -import static org.elasticsearch.painless.lookup.PainlessLookupUtility.CONSTRUCTOR_NAME; import static org.elasticsearch.painless.lookup.PainlessLookupUtility.DEF_CLASS_NAME; +import static org.elasticsearch.painless.lookup.PainlessLookupUtility.buildPainlessConstructorKey; import static org.elasticsearch.painless.lookup.PainlessLookupUtility.buildPainlessFieldKey; import static org.elasticsearch.painless.lookup.PainlessLookupUtility.buildPainlessMethodKey; import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typeToCanonicalTypeName; import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typeToJavaType; import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typesToCanonicalTypeNames; -public class PainlessLookupBuilder { +public final class PainlessLookupBuilder { + + private static class PainlessConstructorCacheKey { + + private final Class targetType; + private final List> 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; + } + + PainlessConstructorCacheKey that = (PainlessConstructorCacheKey)object; + + return Objects.equals(targetType, that.targetType) && + Objects.equals(typeParameters, that.typeParameters); + } + + @Override + public int hashCode() { + return Objects.hash(targetType, typeParameters); + } + } private static class PainlessMethodCacheKey { @@ -120,8 +152,9 @@ public class PainlessLookupBuilder { } } - 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]*$"); @@ -343,11 +376,10 @@ public class PainlessLookupBuilder { "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", nsme); } - String painlessMethodKey = buildPainlessMethodKey(CONSTRUCTOR_NAME, typeParametersSize); - PainlessMethod painlessConstructor = painlessClassBuilder.constructors.get(painlessMethodKey); + String painlessConstructorKey = buildPainlessConstructorKey(typeParametersSize); + PainlessConstructor painlessConstructor = painlessClassBuilder.constructors.get(painlessConstructorKey); if (painlessConstructor == null) { - org.objectweb.asm.commons.Method asmConstructor = org.objectweb.asm.commons.Method.getMethod(javaConstructor); MethodHandle methodHandle; try { @@ -359,17 +391,16 @@ public class PainlessLookupBuilder { MethodType methodType = methodHandle.type(); - painlessConstructor = painlessMethodCache.computeIfAbsent( - new PainlessMethodCacheKey(targetClass, CONSTRUCTOR_NAME, typeParameters), - key -> new PainlessMethod(CONSTRUCTOR_NAME, targetClass, null, void.class, typeParameters, - asmConstructor, javaConstructor.getModifiers(), methodHandle, methodType) + painlessConstructor = painlessConstuctorCache.computeIfAbsent( + new PainlessConstructorCacheKey(targetClass, typeParameters), + key -> new PainlessConstructor(javaConstructor, typeParameters, methodHandle, methodType) ); - painlessClassBuilder.constructors.put(painlessMethodKey, painlessConstructor); - } else if (painlessConstructor.arguments.equals(typeParameters) == false){ + painlessClassBuilder.constructors.put(painlessConstructorKey, painlessConstructor); + } else if (painlessConstructor.typeParameters.equals(typeParameters) == false){ throw new IllegalArgumentException("cannot have constructors " + "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] and " + - "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(painlessConstructor.arguments) + "] " + + "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(painlessConstructor.typeParameters) + "] " + "with the same arity and different type parameters"); } } 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 86d3f876638..0a181c5f1b0 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 final class PainlessLookupUtility { type == String.class; } + /** + * Constructs a painless constructor key used to lookup painless constructors from a painless class. + */ + public static String buildPainlessConstructorKey(int constructorArity) { + return CONSTRUCTOR_NAME + "/" + constructorArity; + } + /** * Constructs a painless method key used to lookup painless methods from a painless class. */ 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 7b35bc1b48e..a4f7b130045 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,18 +77,18 @@ public final class ECapturingFunctionRef extends AExpression implements ILambda // static case if (captured.clazz != def.class) { try { - ref = new FunctionRef(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 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/EFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java index d787db5d41c..23fb8a4180e 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 @@ public final class EFunctionRef extends AExpression implements ILambda { } } else { // whitelist lookup - ref = new FunctionRef(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 e0af653d209..bd4bea95ea0 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.Globals; 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 @@ import java.util.Set; 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,8 @@ public final class EListInit extends AExpression { actual = ArrayList.class; - constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors - .get(PainlessLookupUtility.buildPainlessMethodKey("", 0)); + constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get( + PainlessLookupUtility.buildPainlessConstructorKey(0)); if (constructor == null) { throw createError(new IllegalStateException("Illegal tree structure.")); @@ -92,7 +94,8 @@ public final class EListInit extends AExpression { 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 d81f08dc3cc..5bef62a4b18 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.Globals; 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,8 @@ public final class EMapInit extends AExpression { actual = HashMap.class; - constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors - .get(PainlessLookupUtility.buildPainlessMethodKey("", 0)); + constructor = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual).constructors.get( + PainlessLookupUtility.buildPainlessConstructorKey(0)); if (constructor == null) { throw createError(new IllegalStateException("Illegal tree structure.")); @@ -111,7 +113,8 @@ public final class EMapInit extends AExpression { 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 f092a17c9fc..4e08f257386 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.Locals; 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 @@ public final class ENewObj extends AExpression { } PainlessClass struct = locals.getPainlessLookup().getPainlessStructFromJavaClass(actual); - constructor = struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("", arguments.size())); + constructor = struct.constructors.get(PainlessLookupUtility.buildPainlessConstructorKey(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 @@ public final class ENewObj extends AExpression { argument.write(writer, globals); } - writer.invokeConstructor(Type.getType(constructor.target), constructor.method); + writer.invokeConstructor( + Type.getType(constructor.javaConstructor.getDeclaringClass()), Method.getMethod(constructor.javaConstructor)); } @Override 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 ff0d4231175..65429154301 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.io.PathUtils; 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 class PainlessDocGenerator { 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 @@ public class PainlessDocGenerator { 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 @@ public class PainlessDocGenerator { 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 @@ public class PainlessDocGenerator { 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 @@ public class PainlessDocGenerator { 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 @@ public class PainlessDocGenerator { } } + /** + * 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}. *