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 2021/04/16 04:02:48 UTC

[groovy] branch master updated (122134b -> 0d51fd0)

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

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


    from 122134b  GROOVY-10036: STC: pass explicit type arguments of extension method call
     new da8f95a  GROOVY-10035: Eliminate redundant type cast
     new 0d51fd0  Trivial tweak

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/groovy/ast/tools/ClassNodeUtils.java    | 18 ++++++++
 .../codehaus/groovy/classgen/asm/OperandStack.java | 10 ++---
 .../codehaus/groovy/runtime/ArrayTypeUtils.java    | 50 ++++++++++++++++++++++
 src/test/groovy/bugs/Groovy9126.groovy             |  4 --
 4 files changed, 71 insertions(+), 11 deletions(-)

[groovy] 01/02: GROOVY-10035: Eliminate redundant type cast

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit da8f95aee5fef73c713b0c7c1d5a115c4602ac25
Author: Daniel Sun <su...@apache.org>
AuthorDate: Wed Apr 14 15:22:27 2021 +0800

    GROOVY-10035: Eliminate redundant type cast
---
 .../apache/groovy/ast/tools/ClassNodeUtils.java    | 18 ++++++++
 .../codehaus/groovy/classgen/asm/OperandStack.java |  9 +---
 .../codehaus/groovy/runtime/ArrayTypeUtils.java    | 50 ++++++++++++++++++++++
 src/test/groovy/bugs/Groovy9126.groovy             |  4 --
 4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
index d7d6657..984c530 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
@@ -49,6 +49,9 @@ import java.util.function.Predicate;
 import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.isGenerated;
 import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.markAsGenerated;
 import static org.codehaus.groovy.ast.ClassHelper.boolean_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
+import static org.codehaus.groovy.runtime.ArrayTypeUtils.dimension;
+import static org.codehaus.groovy.runtime.ArrayTypeUtils.elementType;
 import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
 
 /**
@@ -355,6 +358,21 @@ public class ClassNodeUtils {
         return cNode.getOuterClass() != null && !Modifier.isStatic(cNode.getModifiers());
     }
 
+    /**
+     * Check if the source ClassNode is compatible with the target ClassNode
+     */
+    public static boolean isCompatibleWith(ClassNode source, ClassNode target) {
+        if (source.equals(target)) return true;
+
+        if (source.isArray() && target.isArray() && dimension(source) == dimension(target)) {
+            source = elementType(source);
+            target = elementType(target);
+        }
+
+        return !isPrimitiveType(source) && !isPrimitiveType(target)
+                && (source.isDerivedFrom(target) || source.implementsInterface(target));
+    }
+
     public static boolean hasNoArgConstructor(final ClassNode cNode) {
         List<ConstructorNode> constructors = cNode.getDeclaredConstructors();
         for (ConstructorNode next : constructors) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
index ec75c58..73f6689 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
@@ -18,6 +18,7 @@
  */
 package org.codehaus.groovy.classgen.asm;
 
+import org.apache.groovy.ast.tools.ClassNodeUtils;
 import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
@@ -331,19 +332,13 @@ public class OperandStack {
 
         ClassNode top = stack.get(size - 1);
         targetType = targetType.redirect();
-        if (targetType == top) return;
+        if (ClassNodeUtils.isCompatibleWith(top, targetType)) return;
 
         if (coerce) {
             controller.getInvocationWriter().coerce(top, targetType);
             return;
         }
 
-        // GROOVY-10034: no need to cast non-primitive array when target is "java.lang.Object[]"
-        if (targetType.isArray() && targetType.getComponentType().equals(ClassHelper.OBJECT_TYPE)
-                && top.isArray() && !ClassHelper.isPrimitiveType(top.getComponentType())) {
-            return;
-        }
-
         boolean primTarget = ClassHelper.isPrimitiveType(targetType);
         boolean primTop = ClassHelper.isPrimitiveType(top);
 
diff --git a/src/main/java/org/codehaus/groovy/runtime/ArrayTypeUtils.java b/src/main/java/org/codehaus/groovy/runtime/ArrayTypeUtils.java
index 6902461..8556789 100644
--- a/src/main/java/org/codehaus/groovy/runtime/ArrayTypeUtils.java
+++ b/src/main/java/org/codehaus/groovy/runtime/ArrayTypeUtils.java
@@ -19,6 +19,8 @@
 
 package org.codehaus.groovy.runtime;
 
+import org.codehaus.groovy.ast.ClassNode;
+
 /**
  * Utilities for handling array types
  */
@@ -43,6 +45,24 @@ public class ArrayTypeUtils {
     }
 
     /**
+     * Calculate the dimension of array
+     *
+     * @param clazz the type of array
+     * @return the dimension of array
+     */
+    public static int dimension(ClassNode clazz) {
+        checkArrayType(clazz);
+
+        int result = 0;
+        while (clazz.isArray()) {
+            result++;
+            clazz = clazz.getComponentType();
+        }
+
+        return result;
+    }
+
+    /**
      * Get the type of array elements
      *
      * @param clazz the type of array
@@ -59,6 +79,22 @@ public class ArrayTypeUtils {
     }
 
     /**
+     * Get the type of array elements
+     *
+     * @param clazz the type of array
+     * @return the type of elements
+     */
+    public static ClassNode elementType(ClassNode clazz) {
+        checkArrayType(clazz);
+
+        while (clazz.isArray()) {
+            clazz = clazz.getComponentType();
+        }
+
+        return clazz;
+    }
+
+    /**
      * Get the type of array elements by the dimension
      *
      * @param clazz the type of array
@@ -92,4 +128,18 @@ public class ArrayTypeUtils {
             throw new IllegalArgumentException(clazz.getCanonicalName() + " is not array type");
         }
     }
+
+    /**
+     * Check whether the type passed in is array type.
+     * If the type is not array type, throw IllegalArgumentException.
+     */
+    private static void checkArrayType(ClassNode clazz) {
+        if (null == clazz) {
+            throw new IllegalArgumentException("clazz can not be null");
+        }
+
+        if (!clazz.isArray()) {
+            throw new IllegalArgumentException(clazz.getName() + " is not array type");
+        }
+    }
 }
diff --git a/src/test/groovy/bugs/Groovy9126.groovy b/src/test/groovy/bugs/Groovy9126.groovy
index fb72f9c..258e724 100644
--- a/src/test/groovy/bugs/Groovy9126.groovy
+++ b/src/test/groovy/bugs/Groovy9126.groovy
@@ -22,8 +22,6 @@ import groovy.transform.CompileStatic
 import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
 import org.junit.Test
 
-import static groovy.test.GroovyAssert.assertScript
-
 @CompileStatic
 final class Groovy9126 extends AbstractBytecodeTestCase {
 
@@ -64,7 +62,6 @@ final class Groovy9126 extends AbstractBytecodeTestCase {
                  'L0',
                  'LINENUMBER 4 L0',
                  'ALOAD 0',
-                 'CHECKCAST script',
                  'BIPUSH 123',
                  'INVOKESTATIC java/lang/Integer.valueOf (I)Ljava/lang/Integer;',
                  'INVOKEVIRTUAL script.println (Ljava/lang/Object;)V',
@@ -95,7 +92,6 @@ final class Groovy9126 extends AbstractBytecodeTestCase {
                  'L0',
                  'LINENUMBER 4 L0',
                  'ALOAD 0',
-                 'CHECKCAST script',
                  'BIPUSH 123',
                  'INVOKESTATIC java/lang/Integer.valueOf (I)Ljava/lang/Integer;',
                  'INVOKEVIRTUAL script.println (Ljava/lang/Object;)V',

[groovy] 02/02: Trivial tweak

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 0d51fd00259a48bf8ce9350626a95dfbe9d8ca34
Author: Daniel Sun <su...@apache.org>
AuthorDate: Thu Apr 15 08:04:11 2021 +0800

    Trivial tweak
---
 src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
index 73f6689..23476c4 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
@@ -332,7 +332,8 @@ public class OperandStack {
 
         ClassNode top = stack.get(size - 1);
         targetType = targetType.redirect();
-        if (ClassNodeUtils.isCompatibleWith(top, targetType)) return;
+        if (top == targetType /* for better performance */
+                || ClassNodeUtils.isCompatibleWith(top, targetType)) return;
 
         if (coerce) {
             controller.getInvocationWriter().coerce(top, targetType);