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/14 07:22:51 UTC
[groovy] 01/01: GROOVY-10035: Eliminate redundant type cast
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY-10035
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit aaa4cf6aacc9cadbe8348a4d0711305d8cdcaf81
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',