Move AStatement mutable members into isolated Input/Output objects

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

@@ -30,66 +30,76 @@
  */
 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
+    Input input;
+    Output output;
 
     /**
      * Standard constructor with location used for error tracking.
@@ -101,7 +111,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