You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/04/12 18:04:00 UTC

[groovy] 01/01: GROOVY-10029: SC: add toArray call for "Type[] array = collectionOfType"

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

emilles pushed a commit to branch GROOVY-10029
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 9463a2533c48c520a5b60907fc073b2af88530a2
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Apr 12 13:03:41 2021 -0500

    GROOVY-10029: SC: add toArray call for "Type[] array = collectionOfType"
---
 .../java/org/codehaus/groovy/ast/ClassHelper.java  |  2 +
 .../transformers/BinaryExpressionTransformer.java  | 43 +++++++---
 .../transform/stc/StaticTypeCheckingSupport.java   |  3 +-
 .../ArraysAndCollectionsStaticCompileTest.groovy   | 99 +++++++++++++---------
 4 files changed, 94 insertions(+), 53 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
index 7c76c02..358f5ec 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
@@ -64,6 +64,7 @@ import java.lang.ref.SoftReference;
 import java.lang.reflect.Modifier;
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -148,6 +149,7 @@ public class ClassHelper {
             TUPLE_TYPE = makeWithoutCaching(Tuple.class),
             ITERABLE_TYPE = makeWithoutCaching(Iterable.class),
             REFERENCE_TYPE = makeWithoutCaching(Reference.class),
+            COLLECTION_TYPE = makeWithoutCaching(Collection.class),
             COMPARABLE_TYPE = makeWithoutCaching(Comparable.class),
             GROOVY_OBJECT_TYPE = makeWithoutCaching(GroovyObject.class),
             GENERATED_LAMBDA_TYPE = makeWithoutCaching(GeneratedLambda.class),
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
index 38032db..96ef6f1 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
@@ -21,6 +21,8 @@ package org.codehaus.groovy.transform.sc.transformers;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.expr.ArrayExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.DeclarationExpression;
@@ -43,6 +45,7 @@ import org.codehaus.groovy.transform.stc.StaticTypesMarker;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 
@@ -53,6 +56,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.boolX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.ternaryX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
@@ -81,24 +85,34 @@ public class BinaryExpressionTransformer {
         Object[] list = bin.getNodeMetaData(StaticCompilationMetadataKeys.BINARY_EXP_TARGET);
         Token operation = bin.getOperation();
         int operationType = operation.getType();
-        Expression rightExpression = bin.getRightExpression();
         Expression leftExpression = bin.getLeftExpression();
-        if (bin instanceof DeclarationExpression && leftExpression instanceof VariableExpression) {
+        Expression rightExpression = bin.getRightExpression();
+        if (bin instanceof DeclarationExpression
+                && leftExpression instanceof VariableExpression
+                && rightExpression instanceof ConstantExpression) {
             ClassNode declarationType = ((VariableExpression) leftExpression).getOriginType();
-            if (rightExpression instanceof ConstantExpression) {
-                ClassNode unwrapper = ClassHelper.getUnwrapper(declarationType);
-                ClassNode wrapper = ClassHelper.getWrapper(declarationType);
-                if (!rightExpression.getType().equals(declarationType)
-                        && wrapper.isDerivedFrom(ClassHelper.Number_TYPE)
-                        && WideningCategories.isDoubleCategory(unwrapper)) {
-                    ConstantExpression constant = (ConstantExpression) rightExpression;
-                    if (!constant.isNullExpression()) {
-                        return optimizeConstantInitialization(bin, operation, constant, leftExpression, declarationType);
-                    }
+            if (!rightExpression.getType().equals(declarationType)
+                    && ClassHelper.getWrapper(declarationType).isDerivedFrom(ClassHelper.Number_TYPE)
+                    && WideningCategories.isDoubleCategory(ClassHelper.getUnwrapper(declarationType))) {
+                ConstantExpression constant = (ConstantExpression) rightExpression;
+                if (!constant.isNullExpression()) {
+                    return optimizeConstantInitialization(bin, operation, constant, leftExpression, declarationType);
                 }
             }
         }
         if (operationType == Types.ASSIGN) {
+            // GROOVY-10029: add "?.toArray(new T[0])" to "T[] array = collectionOfT" assignments
+            ClassNode leftType = findType(leftExpression), rightType = findType(rightExpression);
+            if (leftType.isArray() && !(rightExpression instanceof ListExpression) && isOrImplements(rightType, ClassHelper.COLLECTION_TYPE)) {
+                ArrayExpression emptyArray = new ArrayExpression(leftType.getComponentType(), null, Collections.singletonList(CONSTANT_ZERO));
+                rightExpression = callX(rightExpression, "toArray", args(emptyArray));
+                rightExpression.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, leftType);
+                ((MethodCallExpression) rightExpression).setMethodTarget(
+                        rightType.getMethod("toArray", new Parameter[]{new Parameter(ClassHelper.OBJECT_TYPE.makeArray(), "a")}));
+                ((MethodCallExpression) rightExpression).setImplicitThis(false);
+                ((MethodCallExpression) rightExpression).setSafe(true);
+            }
+
             MethodNode directMCT = leftExpression.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
             if (directMCT != null) {
                 Expression left = staticCompilationTransformer.transform(leftExpression);
@@ -128,6 +142,11 @@ public class BinaryExpressionTransformer {
                     );
                 }
             }
+
+            // if not transformed to setter call but RHS has been transformed...
+            if (rightExpression != bin.getRightExpression()) {
+                bin.setRightExpression(rightExpression);
+            }
         } else if (operationType == Types.COMPARE_EQUAL || operationType == Types.COMPARE_NOT_EQUAL) {
             // let's check if one of the operands is the null constant
             CompareToNullExpression compareToNullExpression = null;
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index d0e73ea..d5e54d9 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -79,6 +79,7 @@ import static org.codehaus.groovy.ast.ClassHelper.Boolean_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.Byte_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.CLASS_Type;
 import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.COLLECTION_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.Character_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.Double_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.Enum_Type;
@@ -162,7 +163,7 @@ public abstract class StaticTypeCheckingSupport {
 
     protected static final ClassNode Matcher_TYPE = makeWithoutCaching(Matcher.class);
     protected static final ClassNode ArrayList_TYPE = makeWithoutCaching(ArrayList.class);
-    protected static final ClassNode Collection_TYPE = makeWithoutCaching(Collection.class);
+    protected static final ClassNode Collection_TYPE = COLLECTION_TYPE; // TODO: deprecate?
     protected static final ClassNode Deprecated_TYPE = makeWithoutCaching(Deprecated.class);
     protected static final ClassNode LinkedHashMap_TYPE = makeWithoutCaching(LinkedHashMap.class);
     protected static final ClassNode LinkedHashSet_TYPE = makeWithoutCaching(LinkedHashSet.class);
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
index b61bffb..038bcd4 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
@@ -80,24 +80,21 @@ class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsSTCTest
 
     // GROOVY-7656
     void testSpreadSafeMethodCallsOnListLiteralShouldNotCreateListTwice() {
-        try {
-            assertScript '''
-                class Foo {
-                    static void test() {
-                        def list = [1, 2]
-                        def lengths = [list << 3]*.size()
-                        assert lengths == [3]
-                        assert list == [1, 2, 3]
-                    }
+        assertScript '''
+            class Foo {
+                static void test() {
+                    def list = [1, 2]
+                    def lengths = [list << 3]*.size()
+                    assert lengths == [3]
+                    assert list == [1, 2, 3]
                 }
-                Foo.test()
-            '''
-        } finally {
-            assert astTrees['Foo'][1].count('ScriptBytecodeAdapter.createList') == 4
-        }
+            }
+            Foo.test()
+        '''
+        assert astTrees['Foo'][1].count('ScriptBytecodeAdapter.createList') == 4
     }
 
-    //GROOVY-7442
+    // GROOVY-7442
     void testSpreadDotOperatorWithinAssert() {
         assertScript '''
             def myMethod(String a, String b) {
@@ -108,44 +105,66 @@ class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsSTCTest
         '''
     }
 
-    //GROOVY-7688
+    // GROOVY-7688
     void testSpreadSafeMethodCallReceiversWithSideEffectsShouldNotBeVisitedTwice() {
-        try {
-            assertScript '''
-                class Foo {
-                    static void test() {
-                        def list = ['a', 'b']
-                        def lengths = list.toList()*.length()
-                        assert lengths == [1, 1]
-                    }
+        assertScript '''
+            class Foo {
+                static void test() {
+                    def list = ['a', 'b']
+                    def lengths = list.toList()*.length()
+                    assert lengths == [1, 1]
                 }
-                Foo.test()
-            '''
-        } finally {
-            assert astTrees['Foo'][1].count('DefaultGroovyMethods.toList') == 1
-        }
+            }
+            Foo.test()
+        '''
+        assert astTrees['Foo'][1].count('DefaultGroovyMethods.toList') == 1
     }
 
-    //GROOVY-8074
+    // GROOVY-8074
     void testMapSubclassPropertyStyleAccess() {
         assertScript '''
             class MyMap extends LinkedHashMap {
                 def foo = 1
             }
-        
+
             def map = new MyMap()
             map.put('foo', 42)
-            assert map.foo == 42               
+            assert map.foo == 42
         '''
     }
 
-    @Override
-    void testForInLoop() {
-        try {
-            super.testForInLoop()
-        } finally {
-            println astTrees
-        }
+    // GROOVY-10029
+    void testCollectionToArrayAssignmentSC() {
+        assertScript '''
+            class C {
+                static List<String> m() {
+                    return ['foo']
+                }
+                static main(args) {
+                    String[] strings = m()
+                    assert strings.length == 1
+                    assert strings[0] == 'foo'
+                }
+            }
+        '''
+        String out = astTrees['C'][1]
+        out = out.substring(out.indexOf('main([Ljava/lang/String;)'))
+        assert out.contains('INVOKEINTERFACE java/util/List.toArray')
+        assert !out.contains('INVOKEDYNAMIC cast(Ljava/util/List;)') : 'dynamic cast should have been replaced by direct method call'
     }
-}
 
+    void testCollectionToObjectAssignmentSC() {
+        assertScript '''
+            def collectionOfI = [1,2,3]
+
+            def obj
+            obj = new String[0]
+            obj = new Number[1]
+            obj = collectionOfI
+
+            assert obj instanceof List
+        '''
+        String out = astTrees.values()[0][1]
+        assert !out.contains('INVOKEINTERFACE java/util/List.toArray')
+    }
+}