Merge analyze and write phases in Painless "user" tree

modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java

@@ -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();
@@ -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);
 

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AExpression.java

@@ -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;
     }
 
     /**
@@ -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.
      */
@@ -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;
     }

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ANode.java

@@ -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;
@@ -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.
      */

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AStatement.java

@@ -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.
@@ -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);
 }

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AStoreable.java

@@ -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;
@@ -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();
     }
 
@@ -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);
 }

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java

@@ -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;
@@ -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);
@@ -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) {
@@ -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.");
         }
@@ -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) {
@@ -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);
@@ -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);
@@ -240,7 +247,9 @@ AssignmentNode write(ClassNode classNode) {
         assignmentNode.setThere(there);
         assignmentNode.setBack(back);
 
-        return assignmentNode;
+        output.expressionNode = assignmentNode;
+
+        return output;
     }
 
     @Override