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/03/29 14:25:59 UTC
[groovy] 02/03: GROOVY-9955: invoke static method using object
expression type not owner
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4c42e552e09096b263a025f67c977a1efbfbb401
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Feb 26 13:25:27 2021 -0600
GROOVY-9955: invoke static method using object expression type not owner
Conflicts:
src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
---
.../groovy/classgen/asm/InvocationWriter.java | 93 +++++++++++-----------
.../classgen/asm/sc/StaticInvocationWriter.java | 52 ++++++------
.../stc/FieldsAndPropertiesSTCTest.groovy | 2 +
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 14 ++++
4 files changed, 90 insertions(+), 71 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
index b676bf1..d73066b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -48,7 +48,6 @@ import org.codehaus.groovy.syntax.SyntaxException;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
@@ -60,7 +59,11 @@ import java.util.TreeMap;
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
+import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isClassClassNodeWrappingConcreteType;
import static org.objectweb.asm.Opcodes.AALOAD;
+import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACONST_NULL;
import static org.objectweb.asm.Opcodes.ALOAD;
import static org.objectweb.asm.Opcodes.ATHROW;
@@ -149,17 +152,12 @@ public class InvocationWriter {
makeCall(origin, new ClassExpression(sender), receiver, message, arguments, adapter, safe, spreadSafe, implicitThis);
}
-
- protected boolean writeDirectMethodCall(MethodNode target, boolean implicitThis, Expression receiver, TupleExpression args) {
- if (target==null) return false;
-
- String methodName = target.getName();
- CompileStack compileStack = controller.getCompileStack();
- OperandStack operandStack = controller.getOperandStack();
- ClassNode declaringClass = target.getDeclaringClass();
- ClassNode classNode = controller.getClassNode();
- MethodVisitor mv = controller.getMethodVisitor();
+ protected boolean writeDirectMethodCall(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args) {
+ if (target == null) return false;
+
+ ClassNode declaringClass = target.getDeclaringClass();
+ String methodName = target.getName();
int opcode = INVOKEVIRTUAL;
if (target.isStatic()) {
opcode = INVOKESTATIC;
@@ -169,12 +167,17 @@ public class InvocationWriter {
opcode = INVOKESPECIAL;
}
+ CompileStack compileStack = controller.getCompileStack();
+ OperandStack operandStack = controller.getOperandStack();
+ MethodVisitor mv = controller.getMethodVisitor();
+ ClassNode classNode = controller.getClassNode();
+
// handle receiver
int argumentsToRemove = 0;
if (opcode!=INVOKESTATIC) {
if (receiver!=null) {
// load receiver if not static invocation
- // todo: fix inner class case
+ // TODO: fix inner class case
if (implicitThis
&& !classNode.isDerivedFrom(declaringClass)
&& !classNode.implementsInterface(declaringClass)
@@ -195,57 +198,57 @@ public class InvocationWriter {
compileStack.popImplicitThis();
argumentsToRemove++;
} else {
- mv.visitIntInsn(ALOAD,0);
+ mv.visitIntInsn(ALOAD, 0);
operandStack.push(classNode);
argumentsToRemove++;
}
}
- int stackSize = operandStack.getStackLength();
+ ClassNode receiverType;
+ if (receiver == null) {
+ receiverType = declaringClass;
+ } else {
+ receiverType = controller.getTypeChooser().resolveType(receiver, classNode);
+ if (isClassClassNodeWrappingConcreteType(receiverType) && target.isStatic()) {
+ receiverType = receiverType.getGenericsTypes()[0].getType();
+ }
+ }
+ int stackLen = operandStack.getStackLength();
String owner = BytecodeHelper.getClassInternalName(declaringClass);
- ClassNode receiverType = receiver!=null?controller.getTypeChooser().resolveType(receiver, classNode):declaringClass;
- if (opcode == INVOKEVIRTUAL && ClassHelper.OBJECT_TYPE.equals(declaringClass)) {
+ if (opcode == INVOKEVIRTUAL && declaringClass.equals(ClassHelper.OBJECT_TYPE)) {
// avoid using a narrowed type if the method is defined on object because it can interfere
// with delegate type inference in static compilation mode and trigger a ClassCastException
receiverType = declaringClass;
- }
- if (opcode == INVOKEVIRTUAL) {
- if (!receiverType.equals(declaringClass)
- && !ClassHelper.OBJECT_TYPE.equals(declaringClass)
- && !receiverType.isArray()
- && !receiverType.isInterface()
- && !ClassHelper.isPrimitiveType(receiverType) // e.g int.getClass()
- && receiverType.isDerivedFrom(declaringClass)) {
-
- owner = BytecodeHelper.getClassInternalName(receiverType);
- ClassNode top = operandStack.getTopOperand();
- if (!receiverType.equals(top)) {
- mv.visitTypeInsn(CHECKCAST, owner);
- }
- } else if (target.isPublic()
- && (!Modifier.isPublic(declaringClass.getModifiers())
- && !receiverType.equals(declaringClass))
- && receiverType.isDerivedFrom(declaringClass)
- && !receiverType.getPackageName().equals(classNode.getPackageName())) {
- // package private class, public method
- // see GROOVY-6962
- owner = BytecodeHelper.getClassInternalName(receiverType);
+ } else if (opcode == INVOKEVIRTUAL
+ && !receiverType.isArray()
+ && !receiverType.isInterface()
+ && !isPrimitiveType(receiverType)
+ && !receiverType.equals(declaringClass)
+ && receiverType.isDerivedFrom(declaringClass)) {
+
+ owner = BytecodeHelper.getClassInternalName(receiverType);
+ if (!receiverType.equals(operandStack.getTopOperand())) {
+ mv.visitTypeInsn(CHECKCAST, owner);
}
+ } else if (opcode != INVOKESPECIAL && (declaringClass.getModifiers() & (ACC_FINAL | ACC_PUBLIC)) == 0 && !receiverType.equals(declaringClass)
+ && (declaringClass.isInterface() ? receiverType.implementsInterface(declaringClass) : receiverType.isDerivedFrom(declaringClass))) {
+ // GROOVY-6962, GROOVY-9955: method declared by inaccessible class
+ owner = BytecodeHelper.getClassInternalName(receiverType);
}
loadArguments(args.getExpressions(), target.getParameters());
- String desc = BytecodeHelper.getMethodDescriptor(target.getReturnType(), target.getParameters());
- mv.visitMethodInsn(opcode, owner, methodName, desc, declaringClass.isInterface());
- ClassNode ret = target.getReturnType().redirect();
- if (ret==ClassHelper.VOID_TYPE) {
- ret = ClassHelper.OBJECT_TYPE;
+ String descriptor = BytecodeHelper.getMethodDescriptor(target.getReturnType(), target.getParameters());
+ mv.visitMethodInsn(opcode, owner, methodName, descriptor, declaringClass.isInterface());
+ ClassNode returnType = target.getReturnType().redirect();
+ if (returnType == ClassHelper.VOID_TYPE) {
+ returnType = ClassHelper.OBJECT_TYPE;
mv.visitInsn(ACONST_NULL);
}
- argumentsToRemove += (operandStack.getStackLength()-stackSize);
+ argumentsToRemove += (operandStack.getStackLength() - stackLen);
controller.getOperandStack().remove(argumentsToRemove);
- controller.getOperandStack().push(ret);
+ controller.getOperandStack().push(returnType);
return true;
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 352fa2b..b385ee4 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -712,11 +712,10 @@ public class StaticInvocationWriter extends InvocationWriter {
private final Expression receiver;
private final MethodNode target;
- private ClassNode resolvedType;
-
public CheckcastReceiverExpression(final Expression receiver, final MethodNode target) {
this.receiver = receiver;
this.target = target;
+ setType(null);
}
@Override
@@ -750,32 +749,33 @@ public class StaticInvocationWriter extends InvocationWriter {
@Override
public ClassNode getType() {
- if (resolvedType!=null) {
- return resolvedType;
- }
- ClassNode type;
- if (target instanceof ExtensionMethodNode) {
- type = ((ExtensionMethodNode) target).getExtensionMethodNode().getDeclaringClass();
- } else {
- type = getWrapper(controller.getTypeChooser().resolveType(receiver, controller.getClassNode()));
- ClassNode declaringClass = target.getDeclaringClass();
- if (type.getClass() != ClassNode.class
- && type.getClass() != InnerClassNode.class
- && type.getClass() != DecompiledClassNode.class) {
- type = declaringClass; // ex: LUB type
- }
- if (OBJECT_TYPE.equals(type) && !OBJECT_TYPE.equals(declaringClass)) {
- // can happen for compiler rewritten code, where type information is missing
- type = declaringClass;
- }
- if (OBJECT_TYPE.equals(declaringClass)) {
- // check cast not necessary because Object never evolves
- // and it prevents a potential ClassCastException if the delegate of a closure
- // is changed in a statically compiled closure
- type = OBJECT_TYPE;
+ ClassNode type = super.getType();
+ if (type == null) {
+ if (target instanceof ExtensionMethodNode) {
+ type = ((ExtensionMethodNode) target).getExtensionMethodNode().getDeclaringClass();
+ } else {
+ type = controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
+ if (ClassHelper.isPrimitiveType(type)) {
+ type = ClassHelper.getWrapper(type);
+ }
+ ClassNode declaringClass = target.getDeclaringClass();
+ if (type.getClass() != ClassNode.class
+ && type.getClass() != InnerClassNode.class
+ && type.getClass() != DecompiledClassNode.class) {
+ type = declaringClass; // ex: LUB type
+ }
+ if (ClassHelper.OBJECT_TYPE.equals(declaringClass)) {
+ // checkcast not necessary because Object never evolves
+ // and it prevents a potential ClassCastException if the
+ // delegate of a closure is changed in a SC closure
+ type = ClassHelper.OBJECT_TYPE;
+ } else if (ClassHelper.OBJECT_TYPE.equals(type)) {
+ // can happen for compiler rewritten code, where type information is missing
+ type = declaringClass;
+ }
}
+ setType(type);
}
- resolvedType = type;
return type;
}
}
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index cd4d50c..2a9721f 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -227,6 +227,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
void testStaticPropertyWithInheritanceFromAnotherSourceUnit() {
assertScript '''
import groovy.transform.stc.FieldsAndPropertiesSTCTest.Public
+ assert Public.answer == 42
assert Public.CONST == 'XX'
assert Public.VALUE == null
Public.VALUE = 'YY'
@@ -1068,6 +1069,7 @@ import org.codehaus.groovy.ast.stmt.AssertStatement
}
@PackageScope static class PackagePrivate {
+ public static Number getAnswer() { 42 }
public static final String CONST = 'XX'
public static String VALUE
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
index cafcf92..247d7db 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -847,4 +847,18 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
assert new A(['foo1', 'foo2']).fooNames.size() == 2
'''
}
+
+ @Override // GROOVY-9955
+ void testStaticPropertyWithInheritanceFromAnotherSourceUnit() {
+ assertScript '''
+ import groovy.transform.stc.FieldsAndPropertiesSTCTest.Public
+ //assert Public.answer == 42
+ assert Public.CONST == 'XX'
+ assert Public.VALUE == null
+ Public.VALUE = 'YY'
+ assert Public.VALUE == 'YY'
+ Public.@VALUE = 'ZZ'
+ assert Public.@VALUE == 'ZZ'
+ '''
+ }
}