Skip to content

Commit

Permalink
Move specialized execute method code into an external phase (#51954)
Browse files Browse the repository at this point in the history
This change moves almost all of the customized code required to decorate 
the execute method into an external phase. This external phase operates 
only on the ir tree after the user tree has generated the initial ir tree from 
user specified code. The external phase uses ir nodes to create the 
necessary customized code instead of writing asm directly. This is a first 
example of modifying the ir tree to customize it specifically for Elasticsearch 
and leaves the ir nodes in a more generic state.

Another change required for this was to remove the notion of auto-return 
from the ir tree completely. The user tree is now responsible for generating 
appropriate ir tree nodes to support auto-return. This is the first example of 
divergent user and ir trees as the user tree is intended to be higher level 
while the ir tree is supposed to provide lower level translation into asm.

Relates to #49869
Relates to #51841
  • Loading branch information
jdconrad authored Feb 11, 2020
1 parent ba9c4fb commit 026474d
Show file tree
Hide file tree
Showing 13 changed files with 517 additions and 191 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ ScriptRoot compile(Loader loader, String name, String source, CompilerSettings s
SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, null);
ScriptRoot scriptRoot = root.analyze(painlessLookup, settings);
ClassNode classNode = root.writeClass();
ScriptInjectionPhase.phase(scriptRoot, classNode);
Map<String, Object> statics = classNode.write();

try {
Expand Down Expand Up @@ -240,8 +241,9 @@ ScriptRoot compile(Loader loader, String name, String source, CompilerSettings s
byte[] compile(String name, String source, CompilerSettings settings, Printer debugStream) {
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, debugStream);
root.analyze(painlessLookup, settings);
ScriptRoot scriptRoot = root.analyze(painlessLookup, settings);
ClassNode classNode = root.writeClass();
ScriptInjectionPhase.phase(scriptRoot, classNode);
classNode.write();

return classNode.getBytes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ public void invokeMethodCall(PainlessMethod painlessMethod) {
// 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);
painlessMethod.javaMethod.getName(), method.getDescriptor(), true);
} else {
invokeStatic(type, method);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ public boolean isFinal() {
public static class FunctionScope extends Scope {

protected final Class<?> returnType;
protected final Set<String> usedVariables = new HashSet<>();

public FunctionScope(Class<?> returnType) {
super(new HashSet<>());
this.returnType = Objects.requireNonNull(returnType);
}

Expand Down Expand Up @@ -128,10 +128,6 @@ public Class<?> getReturnType() {
public String getReturnCanonicalTypeName() {
return PainlessLookupUtility.typeToCanonicalTypeName(returnType);
}

public Set<String> getUsedVariables() {
return Collections.unmodifiableSet(usedVariables);
}
}

/**
Expand All @@ -150,6 +146,7 @@ public static class LambdaScope extends Scope {
protected final Set<Variable> captures = new HashSet<>();

protected LambdaScope(Scope parent, Class<?> returnType) {
super(parent.usedVariables);
this.parent = parent;
this.returnType = returnType;
}
Expand Down Expand Up @@ -180,6 +177,8 @@ public Variable getVariable(Location location, String name) {
variable = parent.getVariable(location, name);
variable = new Variable(variable.getType(), variable.getName(), true);
captures.add(variable);
} else {
usedVariables.add(name);
}

return variable;
Expand Down Expand Up @@ -213,6 +212,7 @@ public static class BlockScope extends Scope {
protected final Scope parent;

protected BlockScope(Scope parent) {
super(parent.usedVariables);
this.parent = parent;
}

Expand All @@ -238,6 +238,8 @@ public Variable getVariable(Location location, String name) {

if (variable == null) {
variable = parent.getVariable(location, name);
} else {
usedVariables.add(name);
}

return variable;
Expand All @@ -263,9 +265,10 @@ public static FunctionScope newFunctionScope(Class<?> returnType) {
}

protected final Map<String, Variable> variables = new HashMap<>();
protected final Set<String> usedVariables;

protected Scope() {
// do nothing
protected Scope(Set<String> usedVariables) {
this.usedVariables = usedVariables;
}

/**
Expand Down Expand Up @@ -312,4 +315,13 @@ public boolean isInternalVariableDefined(String name) {
public Variable getInternalVariable(Location location, String name) {
return getVariable(location, "#" + name);
}

/**
* Returns the set of variables used within a top-level {@link FunctionScope}
* including local variables no longer in scope upon completion of writing a
* function to ASM bytecode.
*/
public Set<String> getUsedVariables() {
return Collections.unmodifiableSet(usedVariables);
}
}
Loading

0 comments on commit 026474d

Please sign in to comment.