Add string concatenation node and update ir tree visitor (#61099)

Two changes: - The first is the addition of a specialized string concatenation node as part of the ir tree. This allows us to roll up all of the binary nodes containing string concatenations into one node as a set of arguments which will happen in the next PR. This will help in removing optimization complexity from the semantic phases. - This updates the ir tree visitor to have a base class with visitChildren for each node making new ir phases easier to write as only the nodes that are relevant need to be visited. This change is mostly mechanical with the addition of a visitChildren to each ir node.
elastic · Aug 13, 2020 · 5f222a3 · 5f222a3
1 parent 9e9f4e3
commit 5f222a3
Show file tree

Hide file tree

Showing 74 changed files with 946 additions and 415 deletions.
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ArgumentsNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ArgumentsNode.java
@@ -36,6 +36,6 @@ public List<ExpressionNode> getArgumentNodes() {
         return argumentNodes;
     }
 
-    /* ---- end tree structure */
+    /* ---- end tree structure ---- */
 
 }
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BinaryMathNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BinaryMathNode.java
@@ -38,7 +38,6 @@ public class BinaryMathNode extends BinaryNode {
     private Operation operation;
     private Class<?> binaryType;
     private Class<?> shiftType;
-    private boolean cat;
     private int flags;
 
     public void setOperation(Operation operation) {
@@ -73,14 +72,6 @@ public String getShiftCanonicalTypeName() {
         return PainlessLookupUtility.typeToCanonicalTypeName(shiftType);
     }
 
-    public void setCat(boolean cat) {
-        this.cat = cat;
-    }
-
-    public boolean getCat() {
-        return cat;
-    }
-
     public void setFlags(int flags) {
         this.flags = flags;
     }
@@ -92,8 +83,14 @@ public int getFlags() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitBinaryMath(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitBinaryMath(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        getLeftNode().visit(irTreeVisitor, scope);
+        getRightNode().visit(irTreeVisitor, scope);
     }
 
     /* ---- end visitor ---- */
@@ -102,27 +99,7 @@ public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor,
     protected void write(ClassWriter classWriter, MethodWriter methodWriter, WriteScope writeScope) {
         methodWriter.writeDebugInfo(location);
 
-        if (getBinaryType() == String.class && operation == Operation.ADD) {
-            if (cat == false) {
-                methodWriter.writeNewStrings();
-            }
-
-            getLeftNode().write(classWriter, methodWriter, writeScope);
-
-            if (getLeftNode() instanceof BinaryMathNode == false || ((BinaryMathNode)getLeftNode()).getCat() == false) {
-                methodWriter.writeAppendStrings(getLeftNode().getExpressionType());
-            }
-
-            getRightNode().write(classWriter, methodWriter, writeScope);
-
-            if (getRightNode() instanceof BinaryMathNode == false || ((BinaryMathNode)getRightNode()).getCat() == false) {
-                methodWriter.writeAppendStrings(getRightNode().getExpressionType());
-            }
-
-            if (cat == false) {
-                methodWriter.writeToStrings();
-            }
-        } else if (operation == Operation.FIND || operation == Operation.MATCH) {
+        if (operation == Operation.FIND || operation == Operation.MATCH) {
             getRightNode().write(classWriter, methodWriter, writeScope);
             getLeftNode().write(classWriter, methodWriter, writeScope);
             methodWriter.invokeVirtual(org.objectweb.asm.Type.getType(Pattern.class), WriterConstants.PATTERN_MATCHER);

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BinaryNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BinaryNode.java
@@ -21,6 +21,7 @@
 
 import org.elasticsearch.painless.ClassWriter;
 import org.elasticsearch.painless.MethodWriter;
+import org.elasticsearch.painless.phase.IRTreeVisitor;
 import org.elasticsearch.painless.symbol.WriteScope;
 
 public class BinaryNode extends ExpressionNode {
@@ -46,7 +47,20 @@ public ExpressionNode getRightNode() {
         return rightNode;
     }
 
-    /* ---- end tree structure ---- */
+    /* ---- end tree structure, begin visitor ---- */
+
+    @Override
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitBinary(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        leftNode.visit(irTreeVisitor, scope);
+        rightNode.visit(irTreeVisitor, scope);
+    }
+
+    /* ---- end visitor ---- */
 
     @Override
     protected void write(ClassWriter classWriter, MethodWriter methodWriter, WriteScope writeScope) {

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BlockNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BlockNode.java
@@ -65,8 +65,15 @@ public int getStatementCount() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitBlock(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitBlock(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        for (StatementNode statementNode : statementNodes) {
+            statementNode.visit(irTreeVisitor, scope);
+        }
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BooleanNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BooleanNode.java
@@ -44,8 +44,14 @@ public Operation getOperation() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitBoolean(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitBoolean(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        getLeftNode().visit(irTreeVisitor, scope);
+        getRightNode().visit(irTreeVisitor, scope);
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BreakNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/BreakNode.java
@@ -29,8 +29,13 @@ public class BreakNode extends StatementNode {
     /* ---- begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitBreak(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitBreak(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        // do nothing; terminal node
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/CastNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/CastNode.java
@@ -42,8 +42,13 @@ public PainlessCast getCast() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitCast(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitCast(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        getChildNode().visit(irTreeVisitor, scope);
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/CatchNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/CatchNode.java
@@ -62,8 +62,13 @@ public BlockNode getBlockNode() {
     /* ---- end tree structure, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitCatch(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitCatch(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        blockNode.visit(irTreeVisitor, scope);
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ClassNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ClassNode.java
@@ -55,7 +55,7 @@ public void addFieldNode(FieldNode fieldNode) {
     public List<FieldNode> getFieldsNodes() {
         return fieldNodes;
     }
-    
+
     public void addFunctionNode(FunctionNode functionNode) {
         functionNodes.add(functionNode);
     }
@@ -92,8 +92,17 @@ public ScriptScope getScriptScope() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitClass(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitClass(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        clinitBlockNode.visit(irTreeVisitor, scope);
+
+        for (FunctionNode functionNode : functionNodes) {
+            functionNode.visit(irTreeVisitor, scope);
+        }
     }
 
     /* ---- end visitor ---- */
@@ -164,4 +173,9 @@ public byte[] write() {
         classVisitor.visitEnd();
         return classWriter.getClassBytes();
     }
+
+    @Override
+    public void write(ClassWriter classWriter, MethodWriter methodWriter, WriteScope writeScope) {
+        throw new UnsupportedOperationException("use write() instead");
+    }
 }
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ComparisonNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ComparisonNode.java
@@ -63,8 +63,14 @@ public String getComparisonCanonicalTypeName() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitComparison(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitComparison(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        getLeftNode().visit(irTreeVisitor, scope);
+        getRightNode().visit(irTreeVisitor, scope);
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ConditionalNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ConditionalNode.java
@@ -43,8 +43,15 @@ public ExpressionNode getConditionNode() {
     /* ---- end tree structure, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitConditional(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitConditional(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        conditionNode.visit(irTreeVisitor, scope);
+        getLeftNode().visit(irTreeVisitor, scope);
+        getRightNode().visit(irTreeVisitor, scope);
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ConstantNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ConstantNode.java
@@ -41,8 +41,13 @@ public Object getConstant() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitConstant(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitConstant(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        // do nothing; terminal node
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ContinueNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/ContinueNode.java
@@ -29,8 +29,13 @@ public class ContinueNode extends StatementNode {
     /* ---- begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitContinue(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitContinue(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        // do nothing; terminal node
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DeclarationBlockNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DeclarationBlockNode.java
@@ -44,8 +44,15 @@ public List<DeclarationNode> getDeclarationsNodes() {
     /* ---- end tree structure, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitDeclarationBlock(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitDeclarationBlock(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        for (DeclarationNode declarationNode : declarationNodes) {
+            declarationNode.visit(irTreeVisitor, scope);
+        }
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DeclarationNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DeclarationNode.java
@@ -69,8 +69,15 @@ public String getDeclarationCanonicalTypeName() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitDeclaration(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitDeclaration(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        if (expressionNode != null) {
+            expressionNode.visit(irTreeVisitor, scope);
+        }
     }
 
     /* ---- end visitor ---- */

diff --git a/.../lang-painless/src/main/java/org/elasticsearch/painless/ir/DefInterfaceReferenceNode.java b/.../lang-painless/src/main/java/org/elasticsearch/painless/ir/DefInterfaceReferenceNode.java
@@ -42,8 +42,13 @@ public String getDefReferenceEncoding() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitDefInterfaceReference(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitDefInterfaceReference(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        // do nothing; terminal node
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DoWhileLoopNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DoWhileLoopNode.java
@@ -32,8 +32,17 @@ public class DoWhileLoopNode extends LoopNode {
     /* ---- begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitDoWhileLoop(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitDoWhileLoop(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        getBlockNode().visit(irTreeVisitor, scope);
+
+        if (getConditionNode() != null) {
+            getConditionNode().visit(irTreeVisitor, scope);
+        }
     }
 
     /* ---- end visitor ---- */

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DupNode.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ir/DupNode.java
@@ -50,8 +50,13 @@ public int getDepth() {
     /* ---- end node data, begin visitor ---- */
 
     @Override
-    public <Input, Output> Output visit(IRTreeVisitor<Input, Output> irTreeVisitor, Input input) {
-        return irTreeVisitor.visitDup(this, input);
+    public <Scope> void visit(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        irTreeVisitor.visitDup(this, scope);
+    }
+
+    @Override
+    public <Scope> void visitChildren(IRTreeVisitor<Scope> irTreeVisitor, Scope scope) {
+        getChildNode().visit(irTreeVisitor, scope);
     }
 
     /* ---- end visitor ---- */