Skip to content

Commit

Permalink
Merge analyze and write phases in Painless "user" tree (#53685)
Browse files Browse the repository at this point in the history
This is the next step in producing immutable state in the Painless "user" 
tree. This combines the analyze phase and the write phase so that both 
semantic checking and the ir tree are generated in the same phase. This 
allows the removal of the Input/Output objects created in the previous PRs 
from being mutable state on the nodes. Instead they are now local state 
for the analyze phase.

Relates: #53561
Relates: #49869
  • Loading branch information
jdconrad authored Mar 23, 2020
1 parent c5d0731 commit 83e82d3
Show file tree
Hide file tree
Showing 70 changed files with 1,074 additions and 1,459 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,7 @@ ScriptRoot compile(Loader loader, String name, String source, CompilerSettings s
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, null);
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, root);
root.analyze(scriptRoot);
ClassNode classNode = root.writeClass();
ClassNode classNode = root.writeClass(scriptRoot);
DefBootstrapInjectionPhase.phase(classNode);
ScriptInjectionPhase.phase(scriptRoot, classNode);
byte[] bytes = classNode.write();
Expand Down Expand Up @@ -240,8 +239,7 @@ byte[] compile(String name, String source, CompilerSettings settings, Printer de
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, debugStream);
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, root);
root.analyze(scriptRoot);
ClassNode classNode = root.writeClass();
ClassNode classNode = root.writeClass(scriptRoot);
DefBootstrapInjectionPhase.phase(classNode);
ScriptInjectionPhase.phase(scriptRoot, classNode);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,18 @@ public static class Output {
* called on this node to get the type of the node after the cast.</b>
*/
Class<?> actual = null;

/**
* The {@link PainlessCast} to convert this expression's actual type
* to the parent expression's expected type. {@code null} if no cast
* is required.
*/
PainlessCast painlessCast = null;

/**
* The {@link ExpressionNode}(s) generated from this expression.
*/
ExpressionNode expressionNode = null;
}

/**
Expand All @@ -90,16 +102,6 @@ public static class Output {
*/
AExpression prefix;

// TODO: remove placeholders once analysis and write are combined into build
// TODO: https://github.com/elastic/elasticsearch/issues/53561
// This are used to support the transition from a mutable to immutable state.
// Currently, the IR tree is built during the user tree "write" phase, so
// these are stored on the node to set during the "semantic" phase and then
// use during the "write" phase.
Input input = null;
Output output = null;
PainlessCast cast = null;

/**
* Standard constructor with location used for error tracking.
*/
Expand All @@ -121,29 +123,24 @@ public static class Output {
/**
* Checks for errors and collects data for the writing phase.
*/
Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
throw new UnsupportedOperationException();
}

/**
* Writes ASM based on the data collected during the analysis phase.
*/
abstract ExpressionNode write(ClassNode classNode);

void cast() {
cast = AnalyzerCaster.getLegalCast(location, output.actual, input.expected, input.explicit, input.internal);
void cast(Input input, Output output) {
output.painlessCast = AnalyzerCaster.getLegalCast(location, output.actual, input.expected, input.explicit, input.internal);
}

ExpressionNode cast(ExpressionNode expressionNode) {
if (cast == null) {
return expressionNode;
ExpressionNode cast(Output output) {
if (output.painlessCast == null) {
return output.expressionNode;
}

CastNode castNode = new CastNode();
castNode.setLocation(location);
castNode.setExpressionType(cast.targetType);
castNode.setCast(cast);
castNode.setChildNode(expressionNode);
castNode.setExpressionType(output.painlessCast.targetType);
castNode.setCast(output.painlessCast);
castNode.setChildNode(output.expressionNode);

return castNode;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
package org.elasticsearch.painless.node;

import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.ir.IRNode;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -51,12 +49,6 @@ public abstract class ANode {
this.location = Objects.requireNonNull(location);
}

/**
* Writes ASM based on the data collected during the analysis phase.
* @param classNode the root {@link ClassNode}
*/
abstract IRNode write(ClassNode classNode);

/**
* Create an error with location information pointing to this node.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ public static class Output {
* Set to the approximate number of statements in a loop block to prevent
* infinite loops during runtime.
*/
int statementCount = 0;
}
int statementCount = 1;

// TODO: remove placeholders once analysis and write are combined into build
// TODO: https://github.com/elastic/elasticsearch/issues/53561
Input input;
Output output;
/**
* The {@link StatementNode}(s) generated from this expression.
*/
StatementNode statementNode = null;
}

/**
* Standard constructor with location used for error tracking.
Expand All @@ -112,12 +112,7 @@ public static class Output {
/**
* Checks for errors and collects data for the writing phase.
*/
Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
throw new UnsupportedOperationException();
}

/**
* Writes ASM based on the data collected during the analysis phase.
*/
abstract StatementNode write(ClassNode classNode);
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.symbol.ScriptRoot;

import java.util.Objects;
Expand Down Expand Up @@ -57,7 +58,7 @@ public static class Input extends AExpression.Input {
this.prefix = Objects.requireNonNull(prefix);
}

Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
throw new UnsupportedOperationException();
}

Expand All @@ -66,11 +67,4 @@ Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
* rhs actual type to avoid an unnecessary cast.
*/
abstract boolean isDefOptimized();

/**
* If this node or a sub-node of this node uses dynamic calls then
* actual will be set to this value. This is used for an optimization
* during assignment to def type targets.
*/
abstract void updateActual(Class<?> actual);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@
import org.elasticsearch.painless.Operation;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.AssignmentNode;
import org.elasticsearch.painless.ir.BinaryMathNode;
import org.elasticsearch.painless.ir.BraceNode;
import org.elasticsearch.painless.ir.BraceSubDefNode;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.ir.DotNode;
import org.elasticsearch.painless.ir.DotSubDefNode;
import org.elasticsearch.painless.ir.ExpressionNode;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.def;
import org.elasticsearch.painless.symbol.ScriptRoot;
Expand All @@ -37,19 +43,13 @@
/**
* Represents an assignment with the lhs and rhs as child nodes.
*/
public final class EAssignment extends AExpression {
public class EAssignment extends AExpression {

private AExpression lhs;
private AExpression rhs;
private final boolean pre;
private final boolean post;
private Operation operation;

private boolean cat = false;
private Class<?> promote = null;
private Class<?> shiftDistance; // for shifts, the RHS is promoted independently
private PainlessCast there = null;
private PainlessCast back = null;
protected final AExpression lhs;
protected final AExpression rhs;
protected final boolean pre;
protected final boolean post;
protected final Operation operation;

public EAssignment(Location location, AExpression lhs, AExpression rhs, boolean pre, boolean post, Operation operation) {
super(location);
Expand All @@ -62,11 +62,20 @@ public EAssignment(Location location, AExpression lhs, AExpression rhs, boolean
}

@Override
Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
this.input = input;
output = new Output();
Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
Output output = new Output();

AExpression rhs = this.rhs;
Operation operation = this.operation;
boolean cat = false;
Class<?> promote = null;
Class<?> shiftDistance = null;
PainlessCast there = null;
PainlessCast back = null;

Output leftOutput;

Input rightInput = new Input();
Output rightOutput;

if (lhs instanceof AStoreable) {
Expand All @@ -75,7 +84,7 @@ Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {

leftInput.read = input.read;
leftInput.write = true;
leftOutput = lhs.analyze(scriptRoot, scope, leftInput);
leftOutput = lhs.analyze(classNode, scriptRoot, scope, leftInput);
} else {
throw new IllegalArgumentException("Left-hand side cannot be assigned a value.");
}
Expand Down Expand Up @@ -117,7 +126,7 @@ Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
}

if (operation != null) {
rightOutput = rhs.analyze(scriptRoot, scope, new Input());
rightOutput = rhs.analyze(classNode, scriptRoot, scope, rightInput);
boolean shift = false;

if (operation == Operation.MUL) {
Expand Down Expand Up @@ -161,25 +170,25 @@ Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {

if (cat) {
if (rhs instanceof EBinary && ((EBinary)rhs).operation == Operation.ADD && rightOutput.actual == String.class) {
((EBinary)rhs).cat = true;
((BinaryMathNode)rightOutput.expressionNode).setCat(true);
}
}

if (shift) {
if (promote == def.class) {
// shifts are promoted independently, but for the def type, we need object.
rhs.input.expected = promote;
rightInput.expected = promote;
} else if (shiftDistance == long.class) {
rhs.input.expected = int.class;
rhs.input.explicit = true;
rightInput.expected = int.class;
rightInput.explicit = true;
} else {
rhs.input.expected = shiftDistance;
rightInput.expected = shiftDistance;
}
} else {
rhs.input.expected = promote;
rightInput.expected = promote;
}

rhs.cast();
rhs.cast(rightInput, rightOutput);

there = AnalyzerCaster.getLegalCast(location, leftOutput.actual, promote, false, false);
back = AnalyzerCaster.getLegalCast(location, promote, leftOutput.actual, true, false);
Expand All @@ -188,46 +197,44 @@ Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
} else if (rhs != null) {
AStoreable lhs = (AStoreable)this.lhs;

// TODO: move this optimization to a later phase
// If the lhs node is a def optimized node we update the actual type to remove the need for a cast.
if (lhs.isDefOptimized()) {
rightOutput = rhs.analyze(scriptRoot, scope, new Input());
rightOutput = rhs.analyze(classNode, scriptRoot, scope, rightInput);

if (rightOutput.actual == void.class) {
throw createError(new IllegalArgumentException("Right-hand side cannot be a [void] type for assignment."));
}

rhs.input.expected = rightOutput.actual;
lhs.updateActual(rightOutput.actual);
rightInput.expected = rightOutput.actual;
leftOutput.actual = rightOutput.actual;
leftOutput.expressionNode.setExpressionType(rightOutput.actual);

ExpressionNode expressionNode = leftOutput.expressionNode;

if (expressionNode instanceof DotNode && ((DotNode)expressionNode).getRightNode() instanceof DotSubDefNode) {
((DotNode)expressionNode).getRightNode().setExpressionType(leftOutput.actual);
} else if (expressionNode instanceof BraceNode && ((BraceNode)expressionNode).getRightNode() instanceof BraceSubDefNode) {
((BraceNode)expressionNode).getRightNode().setExpressionType(leftOutput.actual);
}
// Otherwise, we must adapt the rhs type to the lhs type with a cast.
} else {
Input rightInput = new Input();
rightInput.expected = leftOutput.actual;
rhs.analyze(scriptRoot, scope, rightInput);
rightOutput = rhs.analyze(classNode, scriptRoot, scope, rightInput);
}

rhs.cast();
rhs.cast(rightInput, rightOutput);
} else {
throw new IllegalStateException("Illegal tree structure.");
}

output.statement = true;
output.actual = input.read ? leftOutput.actual : void.class;

return output;
}

/**
* Handles writing byte code for variable/method chains for all given possibilities
* including String concatenation, compound assignment, regular assignment, and simple
* reads. Includes proper duplication for chained assignments and assignments that are
* also read from.
*/
@Override
AssignmentNode write(ClassNode classNode) {
AssignmentNode assignmentNode = new AssignmentNode();

assignmentNode.setLeftNode(lhs.write(classNode));
assignmentNode.setRightNode(rhs.cast(rhs.write(classNode)));
assignmentNode.setLeftNode(leftOutput.expressionNode);
assignmentNode.setRightNode(rhs.cast(rightOutput));

assignmentNode.setLocation(location);
assignmentNode.setExpressionType(output.actual);
Expand All @@ -240,7 +247,9 @@ AssignmentNode write(ClassNode classNode) {
assignmentNode.setThere(there);
assignmentNode.setBack(back);

return assignmentNode;
output.expressionNode = assignmentNode;

return output;
}

@Override
Expand Down
Loading

0 comments on commit 83e82d3

Please sign in to comment.