Move AStatement mutable members into isolated Input/Output objects (#…

…53348)

This is the follow up to #53075, isolating the mutable data on the 
statement nodes as the referenced change did the expression nodes.

This is a step toward the long-term goal of making the "user" trees nodes 
immutable. This change isolates the mutable data for statement nodes in 
the "user" tree during the semantic (analysis) phase by moving the 
mutable data into Input and Output objects. These objects are created 
locally during the semantic phase to share information required for 
semantic checking between parent-child nodes.

Note that for this change, Input and Output are still stored in a mutable 
way on the statement nodes. This will not be the case once the semantic 
(analysis) phase and ir generation (write) phase are combined in a future 
change.

Relates #49869

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

@@ -91,6 +91,7 @@ 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

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

@@ -30,66 +30,77 @@
  */
 public abstract class AStatement extends ANode {
 
-    /**
-     * Set to true when the final statement in an {@link SClass} is reached.
-     * Used to determine whether or not an auto-return is necessary.
-     */
-    boolean lastSource = false;
-
-    /**
-     * Set to true when a loop begins.  Used by {@link SBlock} to help determine
-     * when the final statement of a loop is reached.
-     */
-    boolean beginLoop = false;
-
-    /**
-     * Set to true when inside a loop.  Used by {@link SBreak} and {@link SContinue}
-     * to determine if a break/continue statement is legal.
-     */
-    boolean inLoop = false;
-
-    /**
-     * Set to true when on the last statement of a loop.  Used by {@link SContinue}
-     * to prevent extraneous continue statements.
-     */
-    boolean lastLoop = false;
-
-    /**
-     * Set to true if a statement would cause the method to exit.  Used to
-     * determine whether or not an auto-return is necessary.
-     */
-    boolean methodEscape = false;
-
-    /**
-     * Set to true if a statement would cause a loop to exit.  Used to
-     * prevent unreachable statements.
-     */
-    boolean loopEscape = false;
-
-    /**
-     * Set to true if all current paths escape from the current {@link SBlock}.
-     * Used during the analysis phase to prevent unreachable statements and
-     * the writing phase to prevent extraneous bytecode gotos from being written.
-     */
-    boolean allEscape = false;
-
-    /**
-     * Set to true if any continue statement occurs in a loop.  Used to prevent
-     * unnecessary infinite loops.
-     */
-    boolean anyContinue = false;
+    public static class Input {
+
+        /**
+         * Set to true when the final statement in an {@link SClass} is reached.
+         * Used to determine whether or not an auto-return is necessary.
+         */
+        boolean lastSource = false;
+
+        /**
+         * Set to true when a loop begins.  Used by {@link SBlock} to help determine
+         * when the final statement of a loop is reached.
+         */
+        boolean beginLoop = false;
+
+        /**
+         * Set to true when inside a loop.  Used by {@link SBreak} and {@link SContinue}
+         * to determine if a break/continue statement is legal.
+         */
+        boolean inLoop = false;
+
+        /**
+         * Set to true when on the last statement of a loop.  Used by {@link SContinue}
+         * to prevent extraneous continue statements.
+         */
+        boolean lastLoop = false;
+    }
 
-    /**
-     * Set to true if any break statement occurs in a loop.  Used to prevent
-     * extraneous loops.
-     */
-    boolean anyBreak = false;
+    public static class Output {
+
+        /**
+         * Set to true if a statement would cause the method to exit.  Used to
+         * determine whether or not an auto-return is necessary.
+         */
+        boolean methodEscape = false;
+
+        /**
+         * Set to true if a statement would cause a loop to exit.  Used to
+         * prevent unreachable statements.
+         */
+        boolean loopEscape = false;
+
+        /**
+         * Set to true if all current paths escape from the current {@link SBlock}.
+         * Used during the analysis phase to prevent unreachable statements and
+         * the writing phase to prevent extraneous bytecode gotos from being written.
+         */
+        boolean allEscape = false;
+
+        /**
+         * Set to true if any continue statement occurs in a loop.  Used to prevent
+         * unnecessary infinite loops.
+         */
+        boolean anyContinue = false;
+
+        /**
+         * Set to true if any break statement occurs in a loop.  Used to prevent
+         * extraneous loops.
+         */
+        boolean anyBreak = false;
+
+        /**
+         * Set to the approximate number of statements in a loop block to prevent
+         * infinite loops during runtime.
+         */
+        int statementCount = 0;
+    }
 
-    /**
-     * Set to the approximate number of statements in a loop block to prevent
-     * infinite loops during runtime.
-     */
-    int statementCount = 0;
+    // TODO: remove placeholders once analysis and write are combined into build
+    // TODO: https://github.com/elastic/elasticsearch/issues/53561
+    Input input;
+    Output output;
 
     /**
      * Standard constructor with location used for error tracking.
@@ -101,7 +112,9 @@ public abstract class AStatement extends ANode {
     /**
      * Checks for errors and collects data for the writing phase.
      */
-    abstract void analyze(ScriptRoot scriptRoot, Scope scope);
+    Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
+        throw new UnsupportedOperationException();
+    }
 
     /**
      * Writes ASM based on the data collected during the analysis phase.

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

@@ -165,17 +165,18 @@ Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
         if (block.statements.isEmpty()) {
             throw createError(new IllegalArgumentException("cannot generate empty lambda"));
         }
-        block.lastSource = true;
-        block.analyze(scriptRoot, lambdaScope);
-        captures = new ArrayList<>(lambdaScope.getCaptures());
+        AStatement.Input blockInput = new AStatement.Input();
+        blockInput.lastSource = true;
+        AStatement.Output blockOutput = block.analyze(scriptRoot, lambdaScope, blockInput);
 
-        if (block.methodEscape == false) {
+        if (blockOutput.methodEscape == false) {
             throw createError(new IllegalArgumentException("not all paths return a value for lambda"));
         }
 
         maxLoopCounter = scriptRoot.getCompilerSettings().getMaxLoopCounter();
 
         // prepend capture list to lambda's arguments
+        captures = new ArrayList<>(lambdaScope.getCaptures());
         this.typeParameters = new ArrayList<>(captures.size() + typeParameters.size());
         parameterNames = new ArrayList<>(captures.size() + paramNameStrs.size());
         for (Variable var : captures) {

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

@@ -44,7 +44,10 @@ public SBlock(Location location, List<AStatement> statements) {
     }
 
     @Override
-    void analyze(ScriptRoot scriptRoot, Scope scope) {
+    Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
+        this.input = input;
+        output = new Output();
+
         if (statements == null || statements.isEmpty()) {
             throw createError(new IllegalArgumentException("A block must contain at least one statement."));
         }
@@ -54,22 +57,25 @@ void analyze(ScriptRoot scriptRoot, Scope scope) {
         for (AStatement statement : statements) {
             // Note that we do not need to check after the last statement because
             // there is no statement that can be unreachable after the last.
-            if (allEscape) {
+            if (output.allEscape) {
                 throw createError(new IllegalArgumentException("Unreachable statement."));
             }
 
-            statement.inLoop = inLoop;
-            statement.lastSource = lastSource && statement == last;
-            statement.lastLoop = (beginLoop || lastLoop) && statement == last;
-            statement.analyze(scriptRoot, scope);
-
-            methodEscape = statement.methodEscape;
-            loopEscape = statement.loopEscape;
-            allEscape = statement.allEscape;
-            anyContinue |= statement.anyContinue;
-            anyBreak |= statement.anyBreak;
-            statementCount += statement.statementCount;
+            Input statementInput = new Input();
+            statementInput.inLoop = input.inLoop;
+            statementInput.lastSource = input.lastSource && statement == last;
+            statementInput.lastLoop = (input.beginLoop || input.lastLoop) && statement == last;
+            Output statementOutput = statement.analyze(scriptRoot, scope, statementInput);
+
+            output.methodEscape = statementOutput.methodEscape;
+            output.loopEscape = statementOutput.loopEscape;
+            output.allEscape = statementOutput.allEscape;
+            output.anyContinue |= statementOutput.anyContinue;
+            output.anyBreak |= statementOutput.anyBreak;
+            output.statementCount += statementOutput.statementCount;
         }
+
+        return output;
     }
 
     @Override
@@ -81,8 +87,8 @@ BlockNode write(ClassNode classNode) {
         }
 
         blockNode.setLocation(location);
-        blockNode.setAllEscape(allEscape);
-        blockNode.setStatementCount(statementCount);
+        blockNode.setAllEscape(output.allEscape);
+        blockNode.setStatementCount(output.statementCount);
 
         return blockNode;
     }

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

@@ -35,15 +35,20 @@ public SBreak(Location location) {
     }
 
     @Override
-    void analyze(ScriptRoot scriptRoot, Scope scope) {
-        if (!inLoop) {
+    Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
+        this.input = input;
+        output = new Output();
+
+        if (input.inLoop == false) {
             throw createError(new IllegalArgumentException("Break statement outside of a loop."));
         }
 
-        loopEscape = true;
-        allEscape = true;
-        anyBreak = true;
-        statementCount = 1;
+        output.loopEscape = true;
+        output.allEscape = true;
+        output.anyBreak = true;
+        output.statementCount = 1;
+
+        return output;
     }
 
     @Override

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

@@ -46,8 +46,11 @@ public SCatch(Location location, DType baseException, SDeclaration declaration,
     }
 
     @Override
-    void analyze(ScriptRoot scriptRoot, Scope scope) {
-        declaration.analyze(scriptRoot, scope);
+    Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
+        this.input = input;
+        output = new Output();
+
+        declaration.analyze(scriptRoot, scope, new Input());
 
         Class<?> baseType = baseException.resolveType(scriptRoot.getPainlessLookup()).getType();
         Class<?> type = scope.getVariable(location, declaration.name).getType();
@@ -59,18 +62,21 @@ void analyze(ScriptRoot scriptRoot, Scope scope) {
         }
 
         if (block != null) {
-            block.lastSource = lastSource;
-            block.inLoop = inLoop;
-            block.lastLoop = lastLoop;
-            block.analyze(scriptRoot, scope);
-
-            methodEscape = block.methodEscape;
-            loopEscape = block.loopEscape;
-            allEscape = block.allEscape;
-            anyContinue = block.anyContinue;
-            anyBreak = block.anyBreak;
-            statementCount = block.statementCount;
+            Input blockInput = new Input();
+            blockInput.lastSource = input.lastSource;
+            blockInput.inLoop = input.inLoop;
+            blockInput.lastLoop = input.lastLoop;
+            Output blockOutput = block.analyze(scriptRoot, scope, blockInput);
+
+            output.methodEscape = blockOutput.methodEscape;
+            output.loopEscape = blockOutput.loopEscape;
+            output.allEscape = blockOutput.allEscape;
+            output.anyContinue = blockOutput.anyContinue;
+            output.anyBreak = blockOutput.anyBreak;
+            output.statementCount = blockOutput.statementCount;
         }
+
+        return output;
     }
 
     @Override

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

@@ -35,18 +35,23 @@ public SContinue(Location location) {
     }
 
     @Override
-    void analyze(ScriptRoot scriptRoot, Scope scope) {
-        if (!inLoop) {
+    Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
+        this.input = input;
+        output = new Output();
+
+        if (input.inLoop == false) {
             throw createError(new IllegalArgumentException("Continue statement outside of a loop."));
         }
 
-        if (lastLoop) {
+        if (input.lastLoop) {
             throw createError(new IllegalArgumentException("Extraneous continue statement."));
         }
 
-        allEscape = true;
-        anyContinue = true;
-        statementCount = 1;
+        output.allEscape = true;
+        output.anyContinue = true;
+        output.statementCount = 1;
+
+        return output;
     }
 
     @Override

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

@@ -44,12 +44,17 @@ public SDeclBlock(Location location, List<SDeclaration> declarations) {
     }
 
     @Override
-    void analyze(ScriptRoot scriptRoot, Scope scope) {
+    Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
+        this.input = input;
+        output = new Output();
+
         for (SDeclaration declaration : declarations) {
-            declaration.analyze(scriptRoot, scope);
+            declaration.analyze(scriptRoot, scope, new Input());
         }
 
-        statementCount = declarations.size();
+        output.statementCount = declarations.size();
+
+        return output;
     }
 
     @Override

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

@@ -48,18 +48,23 @@ public SDeclaration(Location location, DType type, String name, boolean requires
     }
 
     @Override
-    void analyze(ScriptRoot scriptRoot, Scope scope) {
+    Output analyze(ScriptRoot scriptRoot, Scope scope, Input input) {
+        this.input = input;
+        output = new Output();
+
         DResolvedType resolvedType = type.resolveType(scriptRoot.getPainlessLookup());
         type = resolvedType;
 
         if (expression != null) {
-            Input expressionInput = new Input();
+            AExpression.Input expressionInput = new AExpression.Input();
             expressionInput.expected = resolvedType.getType();
             expression.analyze(scriptRoot, scope, expressionInput);
             expression.cast();
         }
 
         scope.defineVariable(location, resolvedType.getType(), name, false);
+
+        return output;
     }
 
     @Override