Remove loop counter from Reserved in Painless AST. (#45298)

This change adds a compiler pass to give each node the chance to store settings necessary for analysis and writing. This removes the need to pass this in a somewhat convoluted way through an additional class called Reserved, and also removes the need to have the Walker set values for settings on reserved. This is next step in decoupling the Painless grammar from the Painless AST.
elastic · Aug 8, 2019 · c57f6be · c57f6be
1 parent 42e8b36
commit c57f6be
Show file tree

Hide file tree

Showing 66 changed files with 520 additions and 59 deletions.
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java
@@ -208,8 +208,8 @@ private static void addFactoryMethod(Map<String, Class<?>> additionalClasses, Cl
      */
     Constructor<?> compile(Loader loader, MainMethodReserved reserved, String name, String source, CompilerSettings settings) {
         ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
-        SSource root = Walker.buildPainlessTree(scriptClassInfo, reserved, name, source, settings, painlessLookup,
-                null);
+        SSource root = Walker.buildPainlessTree(scriptClassInfo, reserved, name, source, settings, painlessLookup, null);
+        root.storeSettings(settings);
         root.analyze(painlessLookup);
         Map<String, Object> statics = root.write();
 
@@ -240,6 +240,7 @@ byte[] compile(String name, String source, CompilerSettings settings, Printer de
         ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
         SSource root = Walker.buildPainlessTree(scriptClassInfo, new MainMethodReserved(), name, source, settings, painlessLookup,
                 debugStream);
+        root.storeSettings(settings);
         root.analyze(painlessLookup);
         root.write();
 

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/antlr/Walker.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/antlr/Walker.java
@@ -253,7 +253,7 @@ public ANode visitSource(SourceContext ctx) {
             statements.add((AStatement)visit(statement));
         }
 
-        return new SSource(scriptClassInfo, settings, sourceName, sourceText, debugStream,
+        return new SSource(scriptClassInfo, sourceName, sourceText, debugStream,
                            (MainMethodReserved)reserved.pop(), location(ctx), functions, statements);
     }
 
@@ -319,8 +319,6 @@ public ANode visitIf(IfContext ctx) {
 
     @Override
     public ANode visitWhile(WhileContext ctx) {
-        reserved.peek().setMaxLoopCounter(settings.getMaxLoopCounter());
-
         AExpression expression = (AExpression)visit(ctx.expression());
 
         if (ctx.trailer() != null) {
@@ -336,8 +334,6 @@ public ANode visitWhile(WhileContext ctx) {
 
     @Override
     public ANode visitDo(DoContext ctx) {
-        reserved.peek().setMaxLoopCounter(settings.getMaxLoopCounter());
-
         AExpression expression = (AExpression)visit(ctx.expression());
         SBlock block = (SBlock)visit(ctx.block());
 
@@ -346,8 +342,6 @@ public ANode visitDo(DoContext ctx) {
 
     @Override
     public ANode visitFor(ForContext ctx) {
-        reserved.peek().setMaxLoopCounter(settings.getMaxLoopCounter());
-
         ANode initializer = ctx.initializer() == null ? null : visit(ctx.initializer());
         AExpression expression = ctx.expression() == null ? null : (AExpression)visit(ctx.expression());
         AExpression afterthought = ctx.afterthought() == null ? null : (AExpression)visit(ctx.afterthought());
@@ -365,8 +359,6 @@ public ANode visitFor(ForContext ctx) {
 
     @Override
     public ANode visitEach(EachContext ctx) {
-        reserved.peek().setMaxLoopCounter(settings.getMaxLoopCounter());
-
         String type = ctx.decltype().getText();
         String name = ctx.ID().getText();
         AExpression expression = (AExpression)visit(ctx.expression());
@@ -377,8 +369,6 @@ public ANode visitEach(EachContext ctx) {
 
     @Override
     public ANode visitIneach(IneachContext ctx) {
-        reserved.peek().setMaxLoopCounter(settings.getMaxLoopCounter());
-
         String name = ctx.ID().getText();
         AExpression expression = (AExpression)visit(ctx.expression());
         SBlock block = (SBlock)visit(ctx.trailer());

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ANode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ANode.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -53,6 +54,11 @@ public abstract class ANode {
         this.location = Objects.requireNonNull(location);
     }
 
+    /**
+     * Store settings required for future compiler passes.
+     */
+    abstract void storeSettings(CompilerSettings settings);
+
     /**
      * Adds all variable names referenced to the variable set.
      * <p>

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java
@@ -21,6 +21,7 @@
 
 
 import org.elasticsearch.painless.AnalyzerCaster;
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.DefBootstrap;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
@@ -62,6 +63,15 @@ public EAssignment(Location location, AExpression lhs, AExpression rhs, boolean
         this.operation = operation;
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        lhs.storeSettings(settings);
+
+        if (rhs != null) {
+            rhs.storeSettings(settings);
+        }
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         lhs.extractVariables(variables);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBinary.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBinary.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.painless.node;
 
 import org.elasticsearch.painless.AnalyzerCaster;
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.DefBootstrap;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
@@ -57,6 +58,12 @@ public EBinary(Location location, Operation operation, AExpression left, AExpres
         this.right = Objects.requireNonNull(right);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        left.storeSettings(settings);
+        right.storeSettings(settings);
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         left.extractVariables(variables);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBool.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBool.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -47,6 +48,12 @@ public EBool(Location location, Operation operation, AExpression left, AExpressi
         this.right = Objects.requireNonNull(right);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        left.storeSettings(settings);
+        right.storeSettings(settings);
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         left.extractVariables(variables);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBoolean.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBoolean.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -37,6 +38,11 @@ public EBoolean(Location location, boolean constant) {
         this.constant = constant;
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        // Do nothing.
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         // Do nothing.

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Locals.LocalMethod;
@@ -59,6 +60,13 @@ public ECallLocal(Location location, String name, List<AExpression> arguments) {
         this.arguments = Objects.requireNonNull(arguments);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        for (AExpression argument : arguments) {
+            argument.storeSettings(settings);
+        }
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         for (AExpression argument : arguments) {

diff --git a/...es/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java b/...es/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.DefBootstrap;
 import org.elasticsearch.painless.FunctionRef;
 import org.elasticsearch.painless.Globals;
@@ -52,6 +53,11 @@ public ECapturingFunctionRef(Location location, String variable, String call) {
         this.call = Objects.requireNonNull(call);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        // Do nothing.
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         variables.add(variable);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECast.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECast.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -44,9 +45,14 @@ final class ECast extends AExpression {
         this.cast = Objects.requireNonNull(cast);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        throw createError(new IllegalStateException("illegal tree structure"));
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
-        throw new IllegalStateException("Illegal tree structure.");
+        throw createError(new IllegalStateException("Illegal tree structure."));
     }
 
     @Override

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EComp.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EComp.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.painless.node;
 
 import org.elasticsearch.painless.AnalyzerCaster;
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.DefBootstrap;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
@@ -56,6 +57,12 @@ public EComp(Location location, Operation operation, AExpression left, AExpressi
         this.right = Objects.requireNonNull(right);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        left.storeSettings(settings);
+        right.storeSettings(settings);
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         left.extractVariables(variables);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConditional.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConditional.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.painless.node;
 
 import org.elasticsearch.painless.AnalyzerCaster;
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -47,6 +48,13 @@ public EConditional(Location location, AExpression condition, AExpression left,
         this.right = Objects.requireNonNull(right);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        condition.storeSettings(settings);
+        left.storeSettings(settings);
+        right.storeSettings(settings);
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         condition.extractVariables(variables);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConstant.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConstant.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -38,6 +39,11 @@ final class EConstant extends AExpression {
         this.constant = constant;
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        throw new IllegalStateException("illegal tree structure");
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         throw new IllegalStateException("Illegal tree structure.");

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EDecimal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EDecimal.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -41,7 +42,14 @@ public EDecimal(Location location, String value) {
     }
 
     @Override
-    void extractVariables(Set<String> variables) {}
+    void storeSettings(CompilerSettings settings) {
+        // Do nothing.
+    }
+
+    @Override
+    void extractVariables(Set<String> variables) {
+        // Do nothing.
+    }
 
     @Override
     void analyze(Locals locals) {

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EElvis.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EElvis.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.painless.node;
 
 import org.elasticsearch.painless.AnalyzerCaster;
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -45,6 +46,12 @@ public EElvis(Location location, AExpression lhs, AExpression rhs) {
         this.rhs = requireNonNull(rhs);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        lhs.storeSettings(settings);
+        rhs.storeSettings(settings);
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         lhs.extractVariables(variables);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EExplicit.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EExplicit.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
@@ -42,6 +43,11 @@ public EExplicit(Location location, String type, AExpression child) {
         this.child = Objects.requireNonNull(child);
     }
 
+    @Override
+    void storeSettings(CompilerSettings settings) {
+        child.storeSettings(settings);
+    }
+
     @Override
     void extractVariables(Set<String> variables) {
         child.extractVariables(variables);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.painless.node;
 
+import org.elasticsearch.painless.CompilerSettings;
 import org.elasticsearch.painless.FunctionRef;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
@@ -47,7 +48,14 @@ public EFunctionRef(Location location, String type, String call) {
     }
 
     @Override
-    void extractVariables(Set<String> variables) {}
+    void storeSettings(CompilerSettings settings) {
+        // do nothing
+    }
+
+    @Override
+    void extractVariables(Set<String> variables) {
+        // do nothing
+    }
 
     @Override
     void analyze(Locals locals) {