You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/07/31 23:00:41 UTC

[groovy] 04/04: GROOVY-9661: address ArrayExpression inconsistencies

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 18a26758d6be8e8719fb5f362653eb94ae34d29b
Author: Paul King <pa...@asert.com.au>
AuthorDate: Sat Aug 1 00:17:51 2020 +1000

    GROOVY-9661: address ArrayExpression inconsistencies
    
    (cherry picked from commit cd5498eaf4db6dcb274570e088ed00bb1a3a6bec)
---
 .../codehaus/groovy/ast/expr/ArrayExpression.java  | 112 +++++++++++++--------
 .../groovy/classgen/AsmClassGenerator.java         |  75 +++++++-------
 2 files changed, 109 insertions(+), 78 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/expr/ArrayExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/ArrayExpression.java
index c316179..3051b3f 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/ArrayExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/ArrayExpression.java
@@ -23,42 +23,53 @@ import org.codehaus.groovy.ast.GroovyCodeVisitor;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 
 /**
- * Represents an array object construction either using a fixed size
- * or an initializer expression
+ * Represents an array object construction.
+ * One of:
+ * <ul>
+ *     <li>a fixed size array (e.g. {@code new String[3]} or {@code new Integer[2][3])}</li>
+ *     <li>an array with an explicit initializer (e.g. {@code new String[]{ "foo", "bar" &#125;})</li>
+ * </ul>
  */
 public class ArrayExpression extends Expression {
-    private final List<Expression> expressions;
-    private final List<Expression> sizeExpression;
+    private final List<Expression> initExpressions;
+    private final List<Expression> sizeExpressions;
 
     private final ClassNode elementType;
 
-    private static ClassNode makeArray(ClassNode base, List<Expression> sizeExpression) {
+    private static ClassNode makeArray(ClassNode base, List<Expression> sizeExpressions) {
         ClassNode ret = base.makeArray();
-        if (sizeExpression == null) return ret;
-        int size = sizeExpression.size();
+        if (sizeExpressions == null) return ret;
+        int size = sizeExpressions.size();
         for (int i = 1; i < size; i++) {
             ret = ret.makeArray();
         }
         return ret;
     }
 
-    public ArrayExpression(ClassNode elementType, List<Expression> expressions, List<Expression> sizeExpression) {
-        //expect to get the elementType
-        super.setType(makeArray(elementType, sizeExpression));
-        if (expressions == null) expressions = Collections.emptyList();
+    public ArrayExpression(ClassNode elementType, List<Expression> initExpressions, List<Expression> sizeExpressions) {
+        super.setType(makeArray(elementType, sizeExpressions));
         this.elementType = elementType;
-        this.expressions = expressions;
-        this.sizeExpression = sizeExpression;
-
-        for (Object item : expressions) {
+        this.sizeExpressions = sizeExpressions;
+        this.initExpressions = initExpressions == null ? Collections.emptyList() : initExpressions;
+        if (initExpressions == null) {
+            if (sizeExpressions == null || sizeExpressions.isEmpty()) {
+                throw new IllegalArgumentException("Either an initializer or defined size must be given");
+            }
+        }
+        if (!this.initExpressions.isEmpty() && sizeExpressions != null && !sizeExpressions.isEmpty()) {
+            throw new IllegalArgumentException("Both an initializer (" + formatInitExpressions() +
+                    ") and a defined size (" + formatSizeExpressions() + ") cannot be given");
+        }
+        for (Object item : this.initExpressions) {
             if (item != null && !(item instanceof Expression)) {
                 throw new ClassCastException("Item: " + item + " is not an Expression");
             }
         }
-        if (sizeExpression != null) {
-            for (Object item : sizeExpression) {
+        if (!hasInitializer()) {
+            for (Object item : sizeExpressions) {
                 if (!(item instanceof Expression)) {
                     throw new ClassCastException("Item: " + item + " is not an Expression");
                 }
@@ -66,20 +77,25 @@ public class ArrayExpression extends Expression {
         }
     }
 
-
     /**
-     * Creates an array using an initializer expression
+     * Creates an array using an initializer (list of expressions corresponding to array elements)
      */
-    public ArrayExpression(ClassNode elementType, List<Expression> expressions) {
-        this(elementType, expressions, null);
+    public ArrayExpression(ClassNode elementType, List<Expression> initExpressions) {
+        this(elementType, initExpressions, null);
     }
 
-    public void addExpression(Expression expression) {
-        expressions.add(expression);
+    /**
+     * Add another element to the initializer expressions
+     */
+    public void addExpression(Expression initExpression) {
+        initExpressions.add(initExpression);
     }
 
+    /**
+     * Get the initializer expressions
+     */
     public List<Expression> getExpressions() {
-        return expressions;
+        return initExpressions;
     }
 
     public void visit(GroovyCodeVisitor visitor) {
@@ -91,17 +107,22 @@ public class ArrayExpression extends Expression {
     }
 
     public Expression transformExpression(ExpressionTransformer transformer) {
-        List<Expression> exprList = transformExpressions(expressions, transformer);
+        List<Expression> exprList = transformExpressions(initExpressions, transformer);
         List<Expression> sizes = null;
-        if (sizeExpression != null) sizes = transformExpressions(sizeExpression, transformer);
+        if (!hasInitializer()) {
+            sizes = transformExpressions(sizeExpressions, transformer);
+        }
         Expression ret = new ArrayExpression(elementType, exprList, sizes);
         ret.setSourcePosition(this);
         ret.copyNodeMetaData(this);
         return ret;
     }
 
+    /**
+     * Get a particular initializer expression
+     */
     public Expression getExpression(int i) {
-        return expressions.get(i);
+        return initExpressions.get(i);
     }
 
     public ClassNode getElementType() {
@@ -109,26 +130,35 @@ public class ArrayExpression extends Expression {
     }
 
     public String getText() {
-        StringBuilder buffer = new StringBuilder("[");
-        boolean first = true;
-        for (Expression expression : expressions) {
-            if (first) {
-                first = false;
-            } else {
-                buffer.append(", ");
-            }
+        return "[" + formatInitExpressions() + "]";
+    }
 
-            buffer.append(expression.getText());
-        }
-        buffer.append("]");
-        return buffer.toString();
+    private String formatInitExpressions() {
+        return "{" + initExpressions.stream().map(Expression::getText).collect(Collectors.joining(", ")) + "}";
+    }
+
+    private String formatSizeExpressions() {
+        return sizeExpressions.stream().map(e -> "[" + e.getText() + "]").collect(Collectors.joining());
+    }
+
+    /**
+     * @return true if the array expression is defined by an explicit initializer
+     */
+    public boolean hasInitializer() {
+        return sizeExpressions == null;
     }
 
+    /**
+     * @return a list with elements corresponding to the array's dimensions
+     */
     public List<Expression> getSizeExpression() {
-        return sizeExpression;
+        return sizeExpressions;
     }
 
     public String toString() {
-        return super.toString() + expressions;
+        if (hasInitializer()) {
+            return super.toString() + "[elementType: " + getElementType() + ", init: {" + formatInitExpressions() + "}]";
+        }
+        return super.toString() + "[elementType: " + getElementType() + ", size: " + formatSizeExpressions() + "]";
     }
 }
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index e1fa3a4..924085f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -1594,12 +1594,14 @@ public class AsmClassGenerator extends ClassGenerator {
         MethodVisitor mv = controller.getMethodVisitor();
         ClassNode elementType = expression.getElementType();
         String arrayTypeName = BytecodeHelper.getClassInternalName(elementType);
-        List<Expression> sizeExpression = expression.getSizeExpression();
 
         int size = 0;
         int dimensions = 0;
-        if (sizeExpression != null) {
-            for (Expression element : sizeExpression) {
+        if (expression.hasInitializer()) {
+            size = expression.getExpressions().size();
+            BytecodeHelper.pushConstant(mv, size);
+        } else {
+            for (Expression element : expression.getSizeExpression()) {
                 if (element == ConstantExpression.EMPTY_EXPRESSION) break;
                 dimensions += 1;
                 // let's convert to an int
@@ -1607,45 +1609,44 @@ public class AsmClassGenerator extends ClassGenerator {
                 controller.getOperandStack().doGroovyCast(ClassHelper.int_TYPE);
             }
             controller.getOperandStack().remove(dimensions);
-        } else {
-            size = expression.getExpressions().size();
-            BytecodeHelper.pushConstant(mv, size);
         }
 
         int storeIns = AASTORE;
-        if (sizeExpression != null) {
-            arrayTypeName = BytecodeHelper.getTypeDescription(expression.getType());
-            mv.visitMultiANewArrayInsn(arrayTypeName, dimensions);
-        } else if (ClassHelper.isPrimitiveType(elementType)) {
-            int primType = 0;
-            if (elementType == ClassHelper.boolean_TYPE) {
-                primType = T_BOOLEAN;
-                storeIns = BASTORE;
-            } else if (elementType == ClassHelper.char_TYPE) {
-                primType = T_CHAR;
-                storeIns = CASTORE;
-            } else if (elementType == ClassHelper.float_TYPE) {
-                primType = T_FLOAT;
-                storeIns = FASTORE;
-            } else if (elementType == ClassHelper.double_TYPE) {
-                primType = T_DOUBLE;
-                storeIns = DASTORE;
-            } else if (elementType == ClassHelper.byte_TYPE) {
-                primType = T_BYTE;
-                storeIns = BASTORE;
-            } else if (elementType == ClassHelper.short_TYPE) {
-                primType = T_SHORT;
-                storeIns = SASTORE;
-            } else if (elementType == ClassHelper.int_TYPE) {
-                primType = T_INT;
-                storeIns = IASTORE;
-            } else if (elementType == ClassHelper.long_TYPE) {
-                primType = T_LONG;
-                storeIns = LASTORE;
+        if (expression.hasInitializer()) {
+            if (ClassHelper.isPrimitiveType(elementType)) {
+                int primType = 0;
+                if (elementType == ClassHelper.boolean_TYPE) {
+                    primType = T_BOOLEAN;
+                    storeIns = BASTORE;
+                } else if (elementType == ClassHelper.char_TYPE) {
+                    primType = T_CHAR;
+                    storeIns = CASTORE;
+                } else if (elementType == ClassHelper.float_TYPE) {
+                    primType = T_FLOAT;
+                    storeIns = FASTORE;
+                } else if (elementType == ClassHelper.double_TYPE) {
+                    primType = T_DOUBLE;
+                    storeIns = DASTORE;
+                } else if (elementType == ClassHelper.byte_TYPE) {
+                    primType = T_BYTE;
+                    storeIns = BASTORE;
+                } else if (elementType == ClassHelper.short_TYPE) {
+                    primType = T_SHORT;
+                    storeIns = SASTORE;
+                } else if (elementType == ClassHelper.int_TYPE) {
+                    primType = T_INT;
+                    storeIns = IASTORE;
+                } else if (elementType == ClassHelper.long_TYPE) {
+                    primType = T_LONG;
+                    storeIns = LASTORE;
+                }
+                mv.visitIntInsn(NEWARRAY, primType);
+            } else {
+                mv.visitTypeInsn(ANEWARRAY, arrayTypeName);
             }
-            mv.visitIntInsn(NEWARRAY, primType);
         } else {
-            mv.visitTypeInsn(ANEWARRAY, arrayTypeName);
+            arrayTypeName = BytecodeHelper.getTypeDescription(expression.getType());
+            mv.visitMultiANewArrayInsn(arrayTypeName, dimensions);
         }
 
         for (int i = 0; i < size; i += 1) {