Change Painless function node to use a block instead of raw statements (#46884)

This change improves the node structure of SFunction. SFunction now uses 
an SBlock instead of a List of AStatments reducing code duplication and 
gives a future target for symbol table scoping.

modules/lang-painless/src/main/java/org/elasticsearch/painless/antlr/Walker.java

@@ -272,7 +272,7 @@ public final class Walker extends PainlessParserBaseVisitor<ANode> {
             statements.add((AStatement)visit(ctx.block().dstatement()));
         }
 
-        return new SFunction(location(ctx), rtnType, name, paramTypes, paramNames, statements, false);
+        return new SFunction(location(ctx), rtnType, name, paramTypes, paramNames, new SBlock(location(ctx), statements), false);
     }
 
     @Override

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

@@ -175,8 +175,8 @@ public final class ELambda extends AExpression implements ILambda {
 
         // desugar lambda body into a synthetic method
         String name = locals.getNextSyntheticName();
-        desugared = new SFunction(
-                location, PainlessLookupUtility.typeToCanonicalTypeName(returnType), name, paramTypes, paramNames, statements, true);
+        desugared = new SFunction(location, PainlessLookupUtility.typeToCanonicalTypeName(returnType), name, paramTypes, paramNames,
+                new SBlock(location, statements), true);
         desugared.storeSettings(settings);
         desugared.generateSignature(locals.getPainlessLookup());
         desugared.analyze(Locals.newLambdaScope(locals.getProgramScope(), desugared.name, returnType,

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

@@ -28,6 +28,7 @@ import org.elasticsearch.painless.MethodWriter;
 import org.objectweb.asm.Type;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Objects;
 import java.util.Set;
 
@@ -61,9 +62,11 @@ public final class ENewArrayFunctionRef extends AExpression implements ILambda {
 
     @Override
     void analyze(Locals locals) {
-        SReturn code = new SReturn(location, new ENewArray(location, type, Arrays.asList(new EVariable(location, "size")), false));
+        SReturn code = new SReturn(location,
+                new ENewArray(location, type, Arrays.asList(new EVariable(location, "size")), false));
         function = new SFunction(location, type, locals.getNextSyntheticName(),
-                Arrays.asList("int"), Arrays.asList("size"), Arrays.asList(code), true);
+                Collections.singletonList("int"), Collections.singletonList("size"),
+                new SBlock(location, Collections.singletonList(code)), true);
         function.storeSettings(settings);
         function.generateSignature(locals.getPainlessLookup());
         function.extractVariables(null);

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

@@ -36,7 +36,7 @@ import static java.util.Collections.emptyList;
  */
 public final class SBlock extends AStatement {
 
-    private final List<AStatement> statements;
+    final List<AStatement> statements;
 
     public SBlock(Location location, List<AStatement> statements) {
         super(location);

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

@@ -50,7 +50,7 @@ public final class SFunction extends AStatement {
     public final String name;
     private final List<String> paramTypeStrs;
     private final List<String> paramNameStrs;
-    private final List<AStatement> statements;
+    private final SBlock block;
     public final boolean synthetic;
 
     private CompilerSettings settings;
@@ -65,7 +65,7 @@ public final class SFunction extends AStatement {
     private Variable loop = null;
 
     public SFunction(Location location, String rtnType, String name,
-                     List<String> paramTypes, List<String> paramNames, List<AStatement> statements,
+                     List<String> paramTypes, List<String> paramNames, SBlock block,
                      boolean synthetic) {
         super(location);
 
@@ -73,27 +73,23 @@ public final class SFunction extends AStatement {
         this.name = Objects.requireNonNull(name);
         this.paramTypeStrs = Collections.unmodifiableList(paramTypes);
         this.paramNameStrs = Collections.unmodifiableList(paramNames);
-        this.statements = Collections.unmodifiableList(statements);
+        this.block = Objects.requireNonNull(block);
         this.synthetic = synthetic;
     }
 
     @Override
     void storeSettings(CompilerSettings settings) {
-        for (AStatement statement : statements) {
-            statement.storeSettings(settings);
-        }
+        block.storeSettings(settings);
 
         this.settings = settings;
     }
 
     @Override
     void extractVariables(Set<String> variables) {
-        for (AStatement statement : statements) {
-            // we reset the list for function scope
-            // note this is not stored for this node
-            // but still required for lambdas
-            statement.extractVariables(new HashSet<>());
-        }
+        // we reset the list for function scope
+        // note this is not stored for this node
+        // but still required for lambdas
+        block.extractVariables(new HashSet<>());
     }
 
     void generateSignature(PainlessLookup painlessLookup) {
@@ -131,28 +127,14 @@ public final class SFunction extends AStatement {
 
     @Override
     void analyze(Locals locals) {
-        if (statements == null || statements.isEmpty()) {
+        if (block.statements.isEmpty()) {
             throw createError(new IllegalArgumentException("Cannot generate an empty function [" + name + "]."));
         }
 
         locals = Locals.newLocalScope(locals);
-
-        AStatement last = statements.get(statements.size() - 1);
-
-        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) {
-                throw createError(new IllegalArgumentException("Unreachable statement."));
-            }
-
-            statement.lastSource = statement == last;
-
-            statement.analyze(locals);
-
-            methodEscape = statement.methodEscape;
-            allEscape = statement.allEscape;
-        }
+        block.lastSource = true;
+        block.analyze(locals);
+        methodEscape = block.methodEscape;
 
         if (!methodEscape && returnType != void.class) {
             throw createError(new IllegalArgumentException("Not all paths provide a return value for method [" + name + "]."));
@@ -184,9 +166,7 @@ public final class SFunction extends AStatement {
             function.visitVarInsn(Opcodes.ISTORE, loop.getSlot());
         }
 
-        for (AStatement statement : statements) {
-            statement.write(function, globals);
-        }
+        block.write(function, globals);
 
         if (!methodEscape) {
             if (returnType == void.class) {
@@ -205,6 +185,6 @@ public final class SFunction extends AStatement {
         if (false == (paramTypeStrs.isEmpty() && paramNameStrs.isEmpty())) {
             description.add(joinWithName("Args", pairwiseToString(paramTypeStrs, paramNameStrs), emptyList()));
         }
-        return multilineToString(description, statements);
+        return multilineToString(description, block.statements);
     }
 }