Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Painless: Clean up PainlessMethod #32476

Merged
merged 18 commits into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand All @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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.targetClass),
implMethod.javaMethod.getName(), receiverClass);
}

/** Returns a method handle to an implementation of clazz, given method reference signature. */
Expand All @@ -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(),
Expand All @@ -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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -108,7 +109,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);
Expand Down Expand Up @@ -138,37 +139,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.getDeclaringClass().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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you use this. consistently when setting members in ctors here? It is difficult to tell what might be locals declared earlier in the method and what are members.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

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());
Expand All @@ -181,7 +204,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);
Expand Down Expand Up @@ -215,7 +238,7 @@ private static PainlessConstructor lookup(PainlessLookup painlessLookup, Class<?

// lookup requested constructor
PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(painlessLookup.getJavaClassFromPainlessType(type));
PainlessConstructor impl = struct.constructors.get(PainlessLookupUtility.buildPainlessConstructorKey(method.arguments.size()));
PainlessConstructor impl = struct.constructors.get(PainlessLookupUtility.buildPainlessConstructorKey(method.typeParameters.size()));

if (impl == null) {
throw new IllegalArgumentException("Unknown reference [" + type + "::new] matching [" + expected + "]");
Expand All @@ -242,16 +265,16 @@ private static PainlessMethod 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Class<?>> typeParameters;
public final MethodType methodType;

public LocalMethod(String name, Class<?> returnType, List<Class<?>> 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 */
Expand Down Expand Up @@ -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<PainlessMethod> methods) {
public static Locals newProgramScope(PainlessLookup painlessLookup, Collection<LocalMethod> methods) {
Locals locals = new Locals(null, painlessLookup, null, null);
for (PainlessMethod method : methods) {
for (LocalMethod method : methods) {
locals.addMethod(method);
}
return locals;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -199,7 +223,7 @@ public PainlessLookup getPainlessLookup() {
// variable name -> variable
private Map<String,Variable> variables;
// method name+arity -> methods
private Map<String,PainlessMethod> methods;
private Map<String,LocalMethod> methods;

/**
* Create a new Locals
Expand Down Expand Up @@ -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;
}
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Loading