Skip to content

Painless: Refactor Lookup into Multiple Classes #32017

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

Closed
wants to merge 35 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
92bb68c
Create a package lookup and move Definition into the package.
jdconrad Jul 6, 2018
3c871dd
Rename Definition to PainlessLookup.
jdconrad Jul 6, 2018
ddec1a6
Rename PainlessLookup.Method to PainlessMethod.
jdconrad Jul 6, 2018
6563f0a
Rename PainlessLookup.MethodKey to PainlessMethodKey.
jdconrad Jul 6, 2018
91136ec
Rename PainlessLookup.Field to PainlessField.
jdconrad Jul 6, 2018
a07cdcb
Rename PainlessLookup.Struct to PainlessClass.
jdconrad Jul 6, 2018
092b48a
Rename PainlessLookup.Cast to PainlessCast.
jdconrad Jul 6, 2018
80359a6
Rename Whitelist.Struct to WhitelistClass.
jdconrad Jul 6, 2018
e99d079
Rename Whitelist.Constructor to WhitelistConstructor.
jdconrad Jul 6, 2018
ac1084d
Rename Whitelist.Method to WhitelistMethod.
jdconrad Jul 6, 2018
4dcabff
Rename Whitelist.Field to WhitelistField.
jdconrad Jul 6, 2018
faf91ba
Partil refactor of PainlessLookup.
jdconrad Jul 7, 2018
8964bdf
Response to PR comments.
jdconrad Jul 7, 2018
288236d
Merge branch 'master' into clean2
jdconrad Jul 7, 2018
8236632
Merge branch 'clean2' into clean3
jdconrad Jul 7, 2018
97ab588
More progress on the builder.
jdconrad Jul 8, 2018
22187f0
More changes.
jdconrad Jul 8, 2018
7e180cd
Merge branch 'master' into clean3
jdconrad Jul 8, 2018
52fe031
Progress.
jdconrad Jul 8, 2018
49c2249
More progress.
jdconrad Jul 8, 2018
b768197
Cleaned up error messages.
jdconrad Jul 9, 2018
5f827f8
Completion of addClass, addConstructor, addMethod, and addField to
jdconrad Jul 9, 2018
db1d8ae
Progress.
jdconrad Jul 9, 2018
2328326
Move more methods.
jdconrad Jul 9, 2018
b79f3b3
More progress.
jdconrad Jul 9, 2018
473cb66
Merge branch 'master' into clean3
jdconrad Jul 10, 2018
8e551bc
Merge remote-tracking branch 'origin/master' into clean3
jdconrad Jul 10, 2018
2c5213c
Progress.
jdconrad Jul 10, 2018
ce48b2f
Progress.
jdconrad Jul 11, 2018
fd13543
Progress.
jdconrad Jul 11, 2018
5ba170e
More progress.
jdconrad Jul 12, 2018
9a905c7
Progress.
jdconrad Jul 12, 2018
898b5a3
staticMembers to members
jdconrad Jul 12, 2018
d2b1753
Merge branch 'master' into clean3
jdconrad Jul 12, 2018
b944227
Merge branch 'master' into clean3
jdconrad Jul 13, 2018
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 @@ -19,9 +19,9 @@

package org.elasticsearch.painless;

import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessLookup.def;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.def;

import java.util.Objects;

Expand Down Expand Up @@ -465,8 +465,9 @@ public static PainlessCast getLegalCast(Location location, Class<?> actual, Clas
(actual.isAssignableFrom(expected) && explicit)) {
return PainlessCast.standard(actual, expected, explicit);
} else {
throw location.createError(new ClassCastException(
"Cannot cast from [" + PainlessLookup.ClassToName(actual) + "] to [" + PainlessLookup.ClassToName(expected) + "]."));
throw location.createError(new ClassCastException("Cannot cast from [" +
PainlessLookupUtility.anyTypeToPainlessTypeName(actual) + "] to [" +
PainlessLookupUtility.anyTypeToPainlessTypeName(expected) + "]."));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static final class Loader extends SecureClassLoader {
*/
@Override
public Class<?> findClass(String name) throws ClassNotFoundException {
Class<?> found = painlessLookup.getClassFromBinaryName(name);
Class<?> found = painlessLookup.painlessTypeNameToPainlessType(name);

return found != null ? found : super.findClass(name);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

package org.elasticsearch.painless;

import org.elasticsearch.painless.lookup.PainlessClass;
import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.lookup.PainlessClass;
import org.elasticsearch.painless.lookup.PainlessMethodKey;

import java.lang.invoke.CallSite;
import java.lang.invoke.MethodHandle;
Expand Down Expand Up @@ -184,10 +184,10 @@ static MethodHandle arrayLengthGetter(Class<?> arrayType) {
* @throws IllegalArgumentException if no matching whitelisted method was found.
*/
static PainlessMethod lookupMethodInternal(PainlessLookup painlessLookup, Class<?> receiverClass, String name, int arity) {
PainlessMethodKey key = new PainlessMethodKey(name, arity);
String key = PainlessLookupUtility.buildPainlessMethodKey(name, arity);
// check whitelist for matching method
for (Class<?> clazz = receiverClass; clazz != null; clazz = clazz.getSuperclass()) {
PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(clazz);
PainlessClass struct = painlessLookup.getPainlessClassFromJavaClass(clazz);

if (struct != null) {
PainlessMethod method = struct.methods.get(key);
Expand All @@ -197,7 +197,7 @@ static PainlessMethod lookupMethodInternal(PainlessLookup painlessLookup, Class<
}

for (Class<?> iface : clazz.getInterfaces()) {
struct = painlessLookup.getPainlessStructFromJavaClass(iface);
struct = painlessLookup.getPainlessClassFromJavaClass(iface);

if (struct != null) {
PainlessMethod method = struct.methods.get(key);
Expand Down Expand Up @@ -302,7 +302,7 @@ static MethodHandle lookupMethod(PainlessLookup painlessLookup, MethodHandles.Lo
nestedType,
0,
DefBootstrap.REFERENCE,
PainlessLookup.ClassToName(interfaceType));
PainlessLookupUtility.anyTypeToPainlessTypeName(interfaceType));
filter = nested.dynamicInvoker();
} else {
throw new AssertionError();
Expand All @@ -326,15 +326,15 @@ static MethodHandle lookupMethod(PainlessLookup painlessLookup, MethodHandles.Lo
*/
static MethodHandle lookupReference(PainlessLookup painlessLookup, MethodHandles.Lookup methodHandlesLookup, String interfaceClass,
Class<?> receiverClass, String name) throws Throwable {
Class<?> interfaceType = painlessLookup.getJavaClassFromPainlessType(interfaceClass);
PainlessMethod interfaceMethod = painlessLookup.getPainlessStructFromJavaClass(interfaceType).functionalMethod;
Class<?> interfaceType = painlessLookup.painlessTypeNameToPainlessType(interfaceClass);
PainlessMethod interfaceMethod = painlessLookup.getPainlessClassFromJavaClass(interfaceType).functionalMethod;
if (interfaceMethod == null) {
throw new IllegalArgumentException("Class [" + interfaceClass + "] is not a functional interface");
}
int arity = interfaceMethod.arguments.size();
PainlessMethod implMethod = lookupMethodInternal(painlessLookup, receiverClass, name, arity);
return lookupReferenceInternal(painlessLookup, methodHandlesLookup, interfaceType, implMethod.owner.name,
implMethod.name, receiverClass);
return lookupReferenceInternal(painlessLookup, methodHandlesLookup, interfaceType,
PainlessLookupUtility.anyTypeToPainlessTypeName(implMethod.target), implMethod.name, receiverClass);
}

/** Returns a method handle to an implementation of clazz, given method reference signature. */
Expand All @@ -344,10 +344,10 @@ private static MethodHandle lookupReferenceInternal(PainlessLookup painlessLooku
final FunctionRef ref;
if ("this".equals(type)) {
// user written method
PainlessMethod interfaceMethod = painlessLookup.getPainlessStructFromJavaClass(clazz).functionalMethod;
PainlessMethod interfaceMethod = painlessLookup.getPainlessClassFromJavaClass(clazz).functionalMethod;
if (interfaceMethod == null) {
throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] " +
"to [" + PainlessLookup.ClassToName(clazz) + "], not a functional interface");
throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] to [" +
PainlessLookupUtility.anyTypeToPainlessTypeName(clazz) + "], not a functional interface");
}
int arity = interfaceMethod.arguments.size() + captures.length;
final MethodHandle handle;
Expand Down Expand Up @@ -418,7 +418,7 @@ public static String getUserFunctionHandleFieldName(String name, int arity) {
static MethodHandle lookupGetter(PainlessLookup painlessLookup, Class<?> receiverClass, String name) {
// first try whitelist
for (Class<?> clazz = receiverClass; clazz != null; clazz = clazz.getSuperclass()) {
PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(clazz);
PainlessClass struct = painlessLookup.getPainlessClassFromJavaClass(clazz);

if (struct != null) {
MethodHandle handle = struct.getters.get(name);
Expand All @@ -428,7 +428,7 @@ static MethodHandle lookupGetter(PainlessLookup painlessLookup, Class<?> receive
}

for (final Class<?> iface : clazz.getInterfaces()) {
struct = painlessLookup.getPainlessStructFromJavaClass(iface);
struct = painlessLookup.getPainlessClassFromJavaClass(iface);

if (struct != null) {
MethodHandle handle = struct.getters.get(name);
Expand Down Expand Up @@ -489,7 +489,7 @@ static MethodHandle lookupGetter(PainlessLookup painlessLookup, Class<?> receive
static MethodHandle lookupSetter(PainlessLookup painlessLookup, Class<?> receiverClass, String name) {
// first try whitelist
for (Class<?> clazz = receiverClass; clazz != null; clazz = clazz.getSuperclass()) {
PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(clazz);
PainlessClass struct = painlessLookup.getPainlessClassFromJavaClass(clazz);

if (struct != null) {
MethodHandle handle = struct.setters.get(name);
Expand All @@ -499,7 +499,7 @@ static MethodHandle lookupSetter(PainlessLookup painlessLookup, Class<?> receive
}

for (final Class<?> iface : clazz.getInterfaces()) {
struct = painlessLookup.getPainlessStructFromJavaClass(iface);
struct = painlessLookup.getPainlessClassFromJavaClass(iface);

if (struct != null) {
MethodHandle handle = struct.setters.get(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@

import org.elasticsearch.painless.lookup.PainlessClass;
import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.lookup.PainlessMethodKey;
import org.objectweb.asm.Type;

import java.lang.invoke.MethodType;
Expand Down Expand Up @@ -81,7 +81,7 @@ public class FunctionRef {
* @param numCaptures number of captured arguments
*/
public FunctionRef(PainlessLookup painlessLookup, Class<?> expected, String type, String call, int numCaptures) {
this(expected, painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod,
this(expected, painlessLookup.getPainlessClassFromJavaClass(expected).functionalMethod,
lookup(painlessLookup, expected, type, call, numCaptures > 0), numCaptures);
}

Expand All @@ -101,22 +101,22 @@ public FunctionRef(Class<?> expected, PainlessMethod interfaceMethod, PainlessMe
interfaceMethodType = interfaceMethod.getMethodType().dropParameterTypes(0, 1);

// the Painless$Script class can be inferred if owner is null
if (delegateMethod.owner == 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.owner.clazz.getName();
isDelegateInterface = delegateMethod.owner.clazz.isInterface();
delegateClassName = delegateMethod.target.getName();
isDelegateInterface = delegateMethod.target.isInterface();
}

if ("<init>".equals(delegateMethod.name)) {
delegateInvokeType = H_NEWINVOKESPECIAL;
} else if (Modifier.isStatic(delegateMethod.modifiers)) {
delegateInvokeType = H_INVOKESTATIC;
} else if (delegateMethod.owner.clazz.isInterface()) {
} else if (delegateMethod.target.isInterface()) {
delegateInvokeType = H_INVOKEINTERFACE;
} else {
delegateInvokeType = H_INVOKEVIRTUAL;
Expand Down Expand Up @@ -165,21 +165,23 @@ private static PainlessMethod lookup(PainlessLookup painlessLookup, Class<?> exp
String type, String call, boolean receiverCaptured) {
// check its really a functional interface
// for e.g. Comparable
PainlessMethod method = painlessLookup.getPainlessStructFromJavaClass(expected).functionalMethod;
PainlessMethod method = painlessLookup.getPainlessClassFromJavaClass(expected).functionalMethod;
if (method == null) {
throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] " +
"to [" + PainlessLookup.ClassToName(expected) + "], not a functional interface");
"to [" + PainlessLookupUtility.anyTypeToPainlessTypeName(expected) + "], " +
"not a functional interface");
}

// lookup requested method
PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(painlessLookup.getJavaClassFromPainlessType(type));
PainlessClass struct = painlessLookup.getPainlessClassFromJavaClass(painlessLookup.painlessTypeNameToPainlessType(type));
final PainlessMethod impl;
// ctor ref
if ("new".equals(call)) {
impl = struct.constructors.get(new PainlessMethodKey("<init>", method.arguments.size()));
impl = struct.constructors.get(PainlessLookupUtility.buildPainlessMethodKey("<init>", method.arguments.size()));
} else {
// look for a static impl first
PainlessMethod staticImpl = struct.staticMethods.get(new PainlessMethodKey(call, method.arguments.size()));
PainlessMethod staticImpl = struct.staticMethods.get(
PainlessLookupUtility.buildPainlessMethodKey(call, method.arguments.size()));
if (staticImpl == null) {
// otherwise a virtual impl
final int arity;
Expand All @@ -190,7 +192,7 @@ private static PainlessMethod lookup(PainlessLookup painlessLookup, Class<?> exp
// receiver passed
arity = method.arguments.size() - 1;
}
impl = struct.methods.get(new PainlessMethodKey(call, arity));
impl = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey(call, arity));
} else {
impl = staticImpl;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

package org.elasticsearch.painless;

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 org.elasticsearch.painless.lookup.PainlessMethodKey;
import org.elasticsearch.painless.ScriptClassInfo.MethodArgument;

import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -143,7 +143,7 @@ public Variable getVariable(Location location, String name) {
}

/** Looks up a method. Returns null if the method does not exist. */
public PainlessMethod getMethod(PainlessMethodKey key) {
public PainlessMethod getMethod(String key) {
PainlessMethod method = lookupMethod(key);
if (method != null) {
return method;
Expand Down Expand Up @@ -199,7 +199,7 @@ public PainlessLookup getPainlessLookup() {
// variable name -> variable
private Map<String,Variable> variables;
// method name+arity -> methods
private Map<PainlessMethodKey,PainlessMethod> methods;
private Map<String,PainlessMethod> methods;

/**
* Create a new Locals
Expand Down Expand Up @@ -237,7 +237,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(PainlessMethodKey key) {
private PainlessMethod lookupMethod(String key) {
if (methods == null) {
return null;
}
Expand All @@ -260,7 +260,7 @@ private void addMethod(PainlessMethod method) {
if (methods == null) {
methods = new HashMap<>();
}
methods.put(new PainlessMethodKey(method.name, method.arguments.size()), method);
methods.put(PainlessLookupUtility.buildPainlessMethodKey(method.name, method.arguments.size()), method);
// TODO: check result
}

Expand Down Expand Up @@ -292,7 +292,7 @@ public int getSlot() {
@Override
public String toString() {
StringBuilder b = new StringBuilder();
b.append("Variable[type=").append(PainlessLookup.ClassToName(clazz));
b.append("Variable[type=").append(PainlessLookupUtility.anyTypeToPainlessTypeName(clazz));
b.append(",name=").append(name);
b.append(",slot=").append(slot);
if (readonly) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
package org.elasticsearch.painless;

import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessLookup.def;
import org.elasticsearch.painless.lookup.def;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.Label;
import org.objectweb.asm.Opcodes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public Map<String, List<String>> getHeaders(PainlessLookup painlessLookup) {
if (objectToExplain != null) {
toString = objectToExplain.toString();
javaClassName = objectToExplain.getClass().getName();
PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(objectToExplain.getClass());
PainlessClass struct = painlessLookup.getPainlessClassFromJavaClass(objectToExplain.getClass());
if (struct != null) {
painlessClassName = struct.name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.painless.Compiler.Loader;
import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.lookup.WhitelistsToPainlessLookup;
import org.elasticsearch.painless.spi.Whitelist;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.ScriptContext;
Expand Down Expand Up @@ -102,9 +102,11 @@ public PainlessScriptEngine(Settings settings, Map<ScriptContext<?>, List<Whitel
for (Map.Entry<ScriptContext<?>, List<Whitelist>> entry : contexts.entrySet()) {
ScriptContext<?> context = entry.getKey();
if (context.instanceClazz.equals(SearchScript.class) || context.instanceClazz.equals(ExecutableScript.class)) {
contextsToCompilers.put(context, new Compiler(GenericElasticsearchScript.class, new PainlessLookup(entry.getValue())));
contextsToCompilers.put(context, new Compiler(GenericElasticsearchScript.class,
WhitelistsToPainlessLookup.build(entry.getValue())));
} else {
contextsToCompilers.put(context, new Compiler(context.instanceClazz, new PainlessLookup(entry.getValue())));
contextsToCompilers.put(context, new Compiler(context.instanceClazz,
WhitelistsToPainlessLookup.build(entry.getValue())));
}
}

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.PainlessLookup;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;

import java.lang.invoke.MethodType;
import java.lang.reflect.Field;
Expand Down Expand Up @@ -182,14 +183,14 @@ private MethodArgument methodArgument(PainlessLookup painlessLookup, Class<?> cl

private static Class<?> definitionTypeForClass(PainlessLookup painlessLookup, Class<?> type,
Function<Class<?>, String> unknownErrorMessageSource) {
type = PainlessLookup.ObjectClassTodefClass(type);
type = PainlessLookupUtility.javaObjectTypeToPainlessDefType(type);
Class<?> componentType = type;

while (componentType.isArray()) {
componentType = componentType.getComponentType();
}

if (painlessLookup.getPainlessStructFromJavaClass(componentType) == null) {
if (painlessLookup.getPainlessClassFromJavaClass(componentType) == null) {
throw new IllegalArgumentException(unknownErrorMessageSource.apply(componentType));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import org.antlr.v4.runtime.LexerNoViableAltException;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.misc.Interval;
import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.lookup.PainlessLookup;

/**
* A lexer that is customized for painless. It:
Expand Down
Loading