Skip to content

Commit

Permalink
Remove PainlessCast from AExpression as mutable state (#56047)
Browse files Browse the repository at this point in the history
PainlessCast currently exists as mutable state on the AExpression node, but this 
is no longer necessary as each cast is only used directly in the semantic pass 
after its creation. This change moves it to be local state during the semantic 
pass as opposed to mutable state on the nodes.

Relates #53702
  • Loading branch information
jdconrad authored May 4, 2020
1 parent f36ab09 commit 022d3d7
Show file tree
Hide file tree
Showing 28 changed files with 193 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.painless.node;

import org.elasticsearch.painless.AnalyzerCaster;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.CastNode;
Expand Down Expand Up @@ -132,20 +131,16 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
throw new UnsupportedOperationException();
}

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

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

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

return castNode;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
boolean cat = false;
Class<?> promote = null;
Class<?> shiftDistance = null;
PainlessCast rightCast;
PainlessCast there = null;
PainlessCast back = null;

Expand Down Expand Up @@ -178,7 +179,8 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
rightInput.expected = promote;
}

rhs.cast(rightInput, rightOutput);
rightCast = AnalyzerCaster.getLegalCast(rhs.location,
rightOutput.actual, rightInput.expected, rightInput.explicit, rightInput.internal);

there = AnalyzerCaster.getLegalCast(location, leftOutput.actual, promote, false, false);
back = AnalyzerCaster.getLegalCast(location, promote, leftOutput.actual, true, false);
Expand Down Expand Up @@ -210,7 +212,8 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
rightOutput = rhs.analyze(classNode, scriptRoot, scope, rightInput);
}

rhs.cast(rightInput, rightOutput);
rightCast = AnalyzerCaster.getLegalCast(rhs.location,
rightOutput.actual, rightInput.expected, rightInput.explicit, rightInput.internal);
} else {
throw new IllegalStateException("Illegal tree structure.");
}
Expand All @@ -220,7 +223,7 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
AssignmentNode assignmentNode = new AssignmentNode();

assignmentNode.setLeftNode(leftOutput.expressionNode);
assignmentNode.setRightNode(rhs.cast(rightOutput));
assignmentNode.setRightNode(cast(rightOutput.expressionNode, rightCast));

assignmentNode.setLocation(location);
assignmentNode.setExpressionType(output.actual);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.BinaryMathNode;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.def;
import org.elasticsearch.painless.symbol.ScriptRoot;
Expand Down Expand Up @@ -141,13 +142,15 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
}
}

left.cast(leftInput, leftOutput);
right.cast(rightInput, rightOutput);
PainlessCast leftCast = AnalyzerCaster.getLegalCast(left.location,
leftOutput.actual, leftInput.expected, leftInput.explicit, leftInput.internal);
PainlessCast rightCast = AnalyzerCaster.getLegalCast(right.location,
rightOutput.actual, rightInput.expected, rightInput.explicit, rightInput.internal);

BinaryMathNode binaryMathNode = new BinaryMathNode();

binaryMathNode.setLeftNode(left.cast(leftOutput));
binaryMathNode.setRightNode(right.cast(rightOutput));
binaryMathNode.setLeftNode(cast(leftOutput.expressionNode, leftCast));
binaryMathNode.setRightNode(cast(rightOutput.expressionNode, rightCast));

binaryMathNode.setLocation(location);
binaryMathNode.setExpressionType(output.actual);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@

package org.elasticsearch.painless.node;

import org.elasticsearch.painless.AnalyzerCaster;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Operation;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.BooleanNode;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.symbol.ScriptRoot;

import java.util.Objects;
Expand Down Expand Up @@ -62,19 +64,21 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
Input leftInput = new Input();
leftInput.expected = boolean.class;
Output leftOutput = left.analyze(classNode, scriptRoot, scope, leftInput);
left.cast(leftInput, leftOutput);
PainlessCast leftCast = AnalyzerCaster.getLegalCast(left.location,
leftOutput.actual, leftInput.expected, leftInput.explicit, leftInput.internal);

Input rightInput = new Input();
rightInput.expected = boolean.class;
Output rightOutput = right.analyze(classNode, scriptRoot, scope, rightInput);
right.cast(rightInput, rightOutput);
PainlessCast rightCast = AnalyzerCaster.getLegalCast(right.location,
rightOutput.actual, rightInput.expected, rightInput.explicit, rightInput.internal);

output.actual = boolean.class;

BooleanNode booleanNode = new BooleanNode();

booleanNode.setLeftNode(left.cast(leftOutput));
booleanNode.setRightNode(right.cast(rightOutput));
booleanNode.setLeftNode(cast(leftOutput.expressionNode, leftCast));
booleanNode.setRightNode(cast(rightOutput.expressionNode, rightCast));

booleanNode.setLocation(location);
booleanNode.setExpressionType(output.actual);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.painless.node;

import org.elasticsearch.painless.AnalyzerCaster;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.BraceNode;
Expand All @@ -28,6 +29,7 @@
import org.elasticsearch.painless.ir.ExpressionNode;
import org.elasticsearch.painless.ir.ListSubShortcutNode;
import org.elasticsearch.painless.ir.MapSubShortcutNode;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.lookup.def;
Expand Down Expand Up @@ -66,12 +68,13 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
Input indexInput = new Input();
indexInput.expected = int.class;
Output indexOutput = index.analyze(classNode, scriptRoot, scope, indexInput);
index.cast(indexInput, indexOutput);
PainlessCast indexCast = AnalyzerCaster.getLegalCast(index.location,
indexOutput.actual, indexInput.expected, indexInput.explicit, indexInput.internal);

output.actual = prefixOutput.actual.getComponentType();

BraceSubNode braceSubNode = new BraceSubNode();
braceSubNode.setChildNode(index.cast(indexOutput));
braceSubNode.setChildNode(cast(indexOutput.expressionNode, indexCast));
braceSubNode.setLocation(location);
braceSubNode.setExpressionType(output.actual);
expressionNode = braceSubNode;
Expand Down Expand Up @@ -109,20 +112,22 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
}

Output indexOutput;
PainlessCast indexCast;

if ((input.read == false || getter != null) && (input.write == false || setter != null)) {
Input indexInput = new Input();
indexInput.expected = setter != null ? setter.typeParameters.get(0) : getter.typeParameters.get(0);
indexOutput = index.analyze(classNode, scriptRoot, scope, indexInput);
index.cast(indexInput, indexOutput);
indexCast = AnalyzerCaster.getLegalCast(index.location,
indexOutput.actual, indexInput.expected, indexInput.explicit, indexInput.internal);

output.actual = setter != null ? setter.typeParameters.get(1) : getter.returnType;
} else {
throw createError(new IllegalArgumentException("Illegal map shortcut for type [" + canonicalClassName + "]."));
}

MapSubShortcutNode mapSubShortcutNode = new MapSubShortcutNode();
mapSubShortcutNode.setChildNode(index.cast(indexOutput));
mapSubShortcutNode.setChildNode(cast(indexOutput.expressionNode, indexCast));
mapSubShortcutNode.setLocation(location);
mapSubShortcutNode.setExpressionType(output.actual);
mapSubShortcutNode.setGetter(getter);
Expand Down Expand Up @@ -150,20 +155,22 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
}

Output indexOutput;
PainlessCast indexCast;

if ((input.read == false || getter != null) && (input.write == false || setter != null)) {
Input indexInput = new Input();
indexInput.expected = int.class;
indexOutput = index.analyze(classNode, scriptRoot, scope, indexInput);
index.cast(indexInput, indexOutput);
indexCast = AnalyzerCaster.getLegalCast(index.location,
indexOutput.actual, indexInput.expected, indexInput.explicit, indexInput.internal);

output.actual = setter != null ? setter.typeParameters.get(1) : getter.returnType;
} else {
throw createError(new IllegalArgumentException("Illegal list shortcut for type [" + canonicalClassName + "]."));
}

ListSubShortcutNode listSubShortcutNode = new ListSubShortcutNode();
listSubShortcutNode.setChildNode(index.cast(indexOutput));
listSubShortcutNode.setChildNode(cast(indexOutput.expressionNode, indexCast));
listSubShortcutNode.setLocation(location);
listSubShortcutNode.setExpressionType(output.actual);
listSubShortcutNode.setGetter(getter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.painless.node;

import org.elasticsearch.painless.AnalyzerCaster;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.CallNode;
Expand All @@ -27,6 +28,7 @@
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.ir.ExpressionNode;
import org.elasticsearch.painless.ir.NullSafeSubNode;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.lookup.def;
import org.elasticsearch.painless.spi.annotation.NonDeterministicAnnotation;
Expand Down Expand Up @@ -84,18 +86,15 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
throw createError(new IllegalArgumentException(
"Argument(s) cannot be of [void] type when calling method [" + name + "]."));
}

expressionInput.expected = expressionOutput.actual;
argument.cast(expressionInput, expressionOutput);
}

// TODO: remove ZonedDateTime exception when JodaCompatibleDateTime is removed
output.actual = input.expected == null || input.expected == ZonedDateTime.class || input.explicit ? def.class : input.expected;

CallSubDefNode callSubDefNode = new CallSubDefNode();

for (int argument = 0; argument < arguments.size(); ++ argument) {
callSubDefNode.addArgumentNode(arguments.get(argument).cast(argumentOutputs.get(argument)));
for (Output argumentOutput : argumentOutputs) {
callSubDefNode.addArgumentNode(argumentOutput.expressionNode);
}

callSubDefNode.setLocation(location);
Expand All @@ -114,7 +113,8 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

scriptRoot.markNonDeterministic(method.annotations.containsKey(NonDeterministicAnnotation.class));

List<Output> argumentOutputs = new ArrayList<>();
List<Output> argumentOutputs = new ArrayList<>(arguments.size());
List<PainlessCast> argumentCasts = new ArrayList<>(arguments.size());

for (int argument = 0; argument < arguments.size(); ++argument) {
AExpression expression = arguments.get(argument);
Expand All @@ -123,16 +123,18 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
expressionInput.expected = method.typeParameters.get(argument);
expressionInput.internal = true;
Output expressionOutput = expression.analyze(classNode, scriptRoot, scope, expressionInput);
expression.cast(expressionInput, expressionOutput);
argumentOutputs.add(expressionOutput);
argumentCasts.add(AnalyzerCaster.getLegalCast(expression.location,
expressionOutput.actual, expressionInput.expected, expressionInput.explicit, expressionInput.internal));

}

output.actual = method.returnType;

CallSubNode callSubNode = new CallSubNode();

for (int argument = 0; argument < arguments.size(); ++ argument) {
callSubNode.addArgumentNode(arguments.get(argument).cast(argumentOutputs.get(argument)));
callSubNode.addArgumentNode(cast(argumentOutputs.get(argument).expressionNode, argumentCasts.get(argument)));
}

callSubNode.setLocation(location);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@

package org.elasticsearch.painless.node;

import org.elasticsearch.painless.AnalyzerCaster;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.ir.FieldNode;
import org.elasticsearch.painless.ir.MemberCallNode;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessClassBinding;
import org.elasticsearch.painless.lookup.PainlessInstanceBinding;
import org.elasticsearch.painless.lookup.PainlessMethod;
Expand Down Expand Up @@ -158,6 +160,7 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
}

List<Output> argumentOutputs = new ArrayList<>(arguments.size());
List<PainlessCast> argumentCasts = new ArrayList<>(arguments.size());
// if the class binding is using an implicit this reference then the arguments counted must
// be incremented by 1 as the this reference will not be part of the arguments passed into
// the class binding call
Expand All @@ -168,14 +171,16 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
argumentInput.expected = typeParameters.get(argument + classBindingOffset);
argumentInput.internal = true;
Output argumentOutput = expression.analyze(classNode, scriptRoot, scope, argumentInput);
expression.cast(argumentInput, argumentOutput);
argumentOutputs.add(argumentOutput);
argumentCasts.add(AnalyzerCaster.getLegalCast(expression.location,
argumentOutput.actual, argumentInput.expected, argumentInput.explicit, argumentInput.internal));

}

MemberCallNode memberCallNode = new MemberCallNode();

for (int argument = 0; argument < arguments.size(); ++argument) {
memberCallNode.addArgumentNode(arguments.get(argument).cast(argumentOutputs.get(argument)));
memberCallNode.addArgumentNode(cast(argumentOutputs.get(argument).expressionNode, argumentCasts.get(argument)));
}

memberCallNode.setLocation(location);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.ir.ComparisonNode;
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.def;
import org.elasticsearch.painless.symbol.ScriptRoot;
Expand Down Expand Up @@ -98,15 +99,17 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in
throw createError(new IllegalArgumentException("extraneous comparison of [null] constants"));
}

left.cast(leftInput, leftOutput);
right.cast(rightInput, rightOutput);
PainlessCast leftCast = AnalyzerCaster.getLegalCast(left.location,
leftOutput.actual, leftInput.expected, leftInput.explicit, leftInput.internal);
PainlessCast rightCast = AnalyzerCaster.getLegalCast(right.location,
rightOutput.actual, rightInput.expected, rightInput.explicit, rightInput.internal);

output.actual = boolean.class;

ComparisonNode comparisonNode = new ComparisonNode();

comparisonNode.setLeftNode(left.cast(leftOutput));
comparisonNode.setRightNode(right.cast(rightOutput));
comparisonNode.setLeftNode(cast(leftOutput.expressionNode, leftCast));
comparisonNode.setRightNode(cast(rightOutput.expressionNode, rightCast));

comparisonNode.setLocation(location);
comparisonNode.setExpressionType(output.actual);
Expand Down
Loading

0 comments on commit 022d3d7

Please sign in to comment.