Skip to content

Commit

Permalink
Remove ScriptClassInfo from Walker (#60316)
Browse files Browse the repository at this point in the history
This change covers two items:

* This removes ScriptClassInfo from Walker so that we can separate semantic checking from user tree 
building entirely. ScriptClassInfo is added to a PainlessSemanticAnalysisPhase and a 
PainlessSemanticHeaderPhase which both extend from their respective Default classes. These classes 
allow for the special casing of the execute method where ScriptClassInfo is used to inject the 
appropriate parameters for the execute method. This allows us to potentially check and use a script 
across more than one context which is especially relevant for stored scripts.

* This removes the BootstrapInjectionPhase and the ScriptInjectionPhase instead moving the code 
more appropriately to the other phases instead.
  • Loading branch information
jdconrad authored Aug 5, 2020
1 parent 02b9f77 commit 9497f66
Show file tree
Hide file tree
Showing 10 changed files with 507 additions and 383 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.node.SClass;
import org.elasticsearch.painless.phase.DefaultSemanticAnalysisPhase;
import org.elasticsearch.painless.phase.DefaultSemanticHeaderPhase;
import org.elasticsearch.painless.phase.DefaultUserTreeToIRTreeVisitor;
import org.elasticsearch.painless.phase.DocFieldsPhase;
import org.elasticsearch.painless.phase.PainlessSemanticAnalysisPhase;
import org.elasticsearch.painless.phase.PainlessSemanticHeaderPhase;
import org.elasticsearch.painless.phase.PainlessUserTreeToIRTreePhase;
import org.elasticsearch.painless.spi.Whitelist;
import org.elasticsearch.painless.symbol.Decorations.IRNodeDecoration;
import org.elasticsearch.painless.symbol.ScriptScope;
Expand Down Expand Up @@ -214,16 +214,14 @@ private static void addFactoryMethod(Map<String, Class<?>> additionalClasses, Cl
ScriptScope compile(Loader loader, String name, String source, CompilerSettings settings) {
String scriptName = Location.computeSourceName(name);
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
SClass root = Walker.buildPainlessTree(scriptClassInfo, scriptName, source, settings);
SClass root = Walker.buildPainlessTree(scriptName, source, settings);
ScriptScope scriptScope = new ScriptScope(painlessLookup, settings, scriptClassInfo, scriptName, source, root.getIdentifier() + 1);
new DefaultSemanticHeaderPhase().visitClass(root, scriptScope);
new DefaultSemanticAnalysisPhase().visitClass(root, scriptScope);
new DefaultUserTreeToIRTreeVisitor().visitClass(root, scriptScope);
new PainlessSemanticHeaderPhase().visitClass(root, scriptScope);
new PainlessSemanticAnalysisPhase().visitClass(root, scriptScope);
// TODO(stu): Make this phase optional #60156
new DocFieldsPhase().visitClass(root, scriptScope);
new PainlessUserTreeToIRTreePhase().visitClass(root, scriptScope);
ClassNode classNode = (ClassNode)scriptScope.getDecoration(root, IRNodeDecoration.class).getIRNode();
DefBootstrapInjectionPhase.phase(classNode);
ScriptInjectionPhase.phase(scriptScope, classNode);
byte[] bytes = classNode.write();

try {
Expand All @@ -249,17 +247,15 @@ ScriptScope compile(Loader loader, String name, String source, CompilerSettings
byte[] compile(String name, String source, CompilerSettings settings, Printer debugStream) {
String scriptName = Location.computeSourceName(name);
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
SClass root = Walker.buildPainlessTree(scriptClassInfo, scriptName, source, settings);
SClass root = Walker.buildPainlessTree(scriptName, source, settings);
ScriptScope scriptScope = new ScriptScope(painlessLookup, settings, scriptClassInfo, scriptName, source, root.getIdentifier() + 1);
new DefaultSemanticHeaderPhase().visitClass(root, scriptScope);
new DefaultSemanticAnalysisPhase().visitClass(root, scriptScope);
new DefaultUserTreeToIRTreeVisitor().visitClass(root, scriptScope);
new PainlessSemanticHeaderPhase().visitClass(root, scriptScope);
new PainlessSemanticAnalysisPhase().visitClass(root, scriptScope);
// TODO(stu): Make this phase optional #60156
new DocFieldsPhase().visitClass(root, scriptScope);
new PainlessUserTreeToIRTreePhase().visitClass(root, scriptScope);
ClassNode classNode = (ClassNode)scriptScope.getDecoration(root, IRNodeDecoration.class).getIRNode();
classNode.setDebugStream(debugStream);
DefBootstrapInjectionPhase.phase(classNode);
ScriptInjectionPhase.phase(scriptScope, classNode);

return classNode.write();
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.elasticsearch.painless.CompilerSettings;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Operation;
import org.elasticsearch.painless.ScriptClassInfo;
import org.elasticsearch.painless.antlr.PainlessParser.AddsubContext;
import org.elasticsearch.painless.antlr.PainlessParser.AfterthoughtContext;
import org.elasticsearch.painless.antlr.PainlessParser.ArgumentContext;
Expand Down Expand Up @@ -107,7 +106,6 @@
import org.elasticsearch.painless.antlr.PainlessParser.TypeContext;
import org.elasticsearch.painless.antlr.PainlessParser.VariableContext;
import org.elasticsearch.painless.antlr.PainlessParser.WhileContext;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.node.AExpression;
import org.elasticsearch.painless.node.ANode;
import org.elasticsearch.painless.node.AStatement;
Expand Down Expand Up @@ -161,29 +159,31 @@
import java.util.Collections;
import java.util.List;

import static java.util.Collections.emptyList;

/**
* Converts the ANTLR tree to a Painless tree.
*/
public final class Walker extends PainlessParserBaseVisitor<ANode> {

public static SClass buildPainlessTree(ScriptClassInfo mainMethod, String sourceName, String sourceText, CompilerSettings settings) {
return new Walker(mainMethod, sourceName, sourceText, settings).source;
public static SClass buildPainlessTree(String sourceName, String sourceText, CompilerSettings settings) {
return new Walker(sourceName, sourceText, settings).source;
}

private final ScriptClassInfo scriptClassInfo;
private final SClass source;
private final CompilerSettings settings;
private final String sourceName;

private int identifier;

private Walker(ScriptClassInfo scriptClassInfo, String sourceName, String sourceText, CompilerSettings settings) {
this.scriptClassInfo = scriptClassInfo;
private final SClass source;

private Walker(String sourceName, String sourceText, CompilerSettings settings) {
this.settings = settings;
this.sourceName = sourceName;
this.source = (SClass)visit(buildAntlrTree(sourceText));

this.identifier = 0;

this.source = (SClass)visit(buildAntlrTree(sourceText));
}

private int nextIdentifier() {
Expand Down Expand Up @@ -247,32 +247,13 @@ public ANode visitSource(SourceContext ctx) {
// part of the overall class
List<AStatement> statements = new ArrayList<>();

// add gets methods as declarations available for the user as variables
for (int index = 0; index < scriptClassInfo.getGetMethods().size(); ++index) {
org.objectweb.asm.commons.Method method = scriptClassInfo.getGetMethods().get(index);
String name = method.getName().substring(3);
name = Character.toLowerCase(name.charAt(0)) + name.substring(1);

statements.add(new SDeclaration(nextIdentifier(), location(ctx),
PainlessLookupUtility.typeToCanonicalTypeName(scriptClassInfo.getGetReturns().get(index)), name, null));
}

for (StatementContext statement : ctx.statement()) {
statements.add((AStatement)visit(statement));
}

String returnCanonicalTypeName = PainlessLookupUtility.typeToCanonicalTypeName(scriptClassInfo.getExecuteMethodReturnType());
List<String> paramTypes = new ArrayList<>();
List<String> paramNames = new ArrayList<>();

for (ScriptClassInfo.MethodArgument argument : scriptClassInfo.getExecuteArguments()) {
paramTypes.add(PainlessLookupUtility.typeToCanonicalTypeName(argument.getClazz()));
paramNames.add(argument.getName());
}

// generate the execute method from the collected statements and parameters
SFunction execute = new SFunction(nextIdentifier(), location(ctx), returnCanonicalTypeName, "execute", paramTypes, paramNames,
new SBlock(nextIdentifier(), location(ctx), statements), true, false, false, true);
SFunction execute = new SFunction(nextIdentifier(), location(ctx), "<internal>", "execute", emptyList(), emptyList(),
new SBlock(nextIdentifier(), location(ctx), statements), false, false, false, false);
functions.add(execute);

return new SClass(nextIdentifier(), location(ctx), functions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,6 @@ public void visitFunction(SFunction userFunctionNode, ScriptScope scriptScope) {
if (methodEscape) {
functionScope.setCondition(userFunctionNode, MethodEscape.class);
}

// TODO: do not specialize for execute
// TODO: https://github.com/elastic/elasticsearch/issues/51841
if ("execute".equals(functionName)) {
scriptScope.setUsedVariables(functionScope.getUsedVariables());
}
// TODO: end
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ public void visitFunction(SFunction userFunctionNode, ScriptScope scriptScope) {
String functionName = userFunctionNode.getFunctionName();
List<String> canonicalTypeNameParameters = userFunctionNode.getCanonicalTypeNameParameters();
List<String> parameterNames = userFunctionNode.getParameterNames();
int parameterCount = canonicalTypeNameParameters.size();

if (canonicalTypeNameParameters.size() != parameterNames.size()) {
if (parameterCount != parameterNames.size()) {
throw userFunctionNode.createError(new IllegalStateException("invalid function definition: " +
"parameter types size [" + canonicalTypeNameParameters.size() + "] is not equal to " +
"parameter names size [" + parameterNames.size() + "] for function [" + functionName +"]"));
Expand Down
Loading

0 comments on commit 9497f66

Please sign in to comment.