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:57 UTC
[groovy] branch GROOVY_2_5_X updated (ec1cdcd -> 42798a8)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from ec1cdcd GROOVY-9906: check index before comparing parameters
new 880879a GROOVY-9955: access static fields using object expression type not owner
new 4c42e55 GROOVY-9955: invoke static method using object expression type not owner
new 42798a8 GROOVY-9967: makeGroovyObjectGetPropertySite: leverage property owner
The 3 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:
.../groovy/classgen/asm/InvocationWriter.java | 93 ++++++++--------
.../classgen/asm/sc/StaticInvocationWriter.java | 52 ++++-----
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 121 ++++++++++-----------
.../stc/FieldsAndPropertiesSTCTest.groovy | 26 ++++-
.../transform/stc/TypeInferenceSTCTest.groovy | 52 ++++++++-
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 16 ++-
.../classgen/asm/sc/bugs/Groovy6276Bug.groovy | 70 ++++++------
7 files changed, 256 insertions(+), 174 deletions(-)
[groovy] 02/03: GROOVY-9955: invoke static method using object
expression type not owner
Posted by em...@apache.org.
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'
+ '''
+ }
}
[groovy] 01/03: GROOVY-9955: access static fields using object
expression type not owner
Posted by em...@apache.org.
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 880879ab3895fed7c39f5ff476fbc3beed0850dc
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Feb 26 11:15:07 2021 -0600
GROOVY-9955: access static fields using object expression type not owner
- restore direct static field write for "Type.@name = ..."
Conflicts:
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
---
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 107 +++++++++++----------
.../stc/FieldsAndPropertiesSTCTest.groovy | 24 ++++-
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 2 +-
3 files changed, 79 insertions(+), 54 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index f26a825..33329c6 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -84,6 +84,7 @@ import static org.codehaus.groovy.ast.ClassHelper.make;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.chooseBestMethod;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDGMMethodsByNameAndArguments;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf;
@@ -131,42 +132,13 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
makeDynamicGetProperty(receiver, propertyName, safe);
return;
}
- TypeChooser typeChooser = controller.getTypeChooser();
- ClassNode classNode = controller.getClassNode();
- ClassNode receiverType = receiver.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
- if (receiverType==null) {
- receiverType = typeChooser.resolveType(receiver, classNode);
- }
- Object type = receiver.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
- if (type==null && receiver instanceof VariableExpression) {
- Variable variable = ((VariableExpression) receiver).getAccessedVariable();
- if (variable instanceof Expression) {
- type = ((Expression) variable).getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
- }
- }
- if (type!=null) {
- // in case a "flow type" is found, it is preferred to use it instead of
- // the declaration type
- receiverType = (ClassNode) type;
- }
- boolean isClassReceiver = false;
- if (isClassClassNodeWrappingConcreteType(receiverType)) {
- isClassReceiver = true;
- receiverType = receiverType.getGenericsTypes()[0].getType();
- }
-
- if (isPrimitiveType(receiverType)) {
- // GROOVY-6590: wrap primitive types
- receiverType = getWrapper(receiverType);
- }
-
- MethodVisitor mv = controller.getMethodVisitor();
-
- if (receiverType.isArray() && propertyName.equals("length")) {
+ boolean[] isClassReceiver = new boolean[1];
+ ClassNode receiverType = getPropertyOwnerType(receiver, isClassReceiver);
+ if (receiverType.isArray() && "length".equals(propertyName)) {
receiver.visit(controller.getAcg());
- ClassNode arrayGetReturnType = typeChooser.resolveType(receiver, classNode);
+ ClassNode arrayGetReturnType = controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
controller.getOperandStack().doGroovyCast(arrayGetReturnType);
- mv.visitInsn(ARRAYLENGTH);
+ controller.getMethodVisitor().visitInsn(ARRAYLENGTH);
controller.getOperandStack().replace(int_TYPE);
return;
} else if (
@@ -189,7 +161,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
if (!isStaticProperty && (receiverType.implementsInterface(MAP_TYPE) || MAP_TYPE.equals(receiverType))) {
// for maps, replace map.foo with map.get('foo')
- writeMapDotProperty(receiver, propertyName, mv, safe);
+ writeMapDotProperty(receiver, propertyName, safe);
return;
}
if (makeGetPropertyWithGetter(receiver, receiverType, propertyName, safe, implicitThis)) return;
@@ -199,7 +171,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
if (makeGetPropertyWithGetter(receiver, receiver.getType(), propertyName, safe, implicitThis)) return;
if (makeGetPrivateFieldWithBridgeMethod(receiver, receiver.getType(), propertyName, safe, implicitThis)) return;
}
- if (isClassReceiver) {
+ if (isClassReceiver[0]) {
// we are probably looking for a property of the class
if (makeGetPropertyWithGetter(receiver, CLASS_Type, propertyName, safe, implicitThis)) return;
if (makeGetField(receiver, CLASS_Type, propertyName, safe, false)) return;
@@ -261,8 +233,8 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
}
}
- if (!isStaticProperty && (receiverType.implementsInterface(LIST_TYPE) || LIST_TYPE.equals(receiverType))) {
- writeListDotProperty(receiver, propertyName, mv, safe);
+ if (!isStaticProperty && isOrImplements(receiverType, LIST_TYPE)) {
+ writeListDotProperty(receiver, propertyName, safe);
return;
}
@@ -284,9 +256,11 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
mce.visit(controller.getAcg());
}
- private void writeMapDotProperty(final Expression receiver, final String methodName, final MethodVisitor mv, final boolean safe) {
+ private void writeMapDotProperty(final Expression receiver, final String propertyName, final boolean safe) {
receiver.visit(controller.getAcg()); // load receiver
+ MethodVisitor mv = controller.getMethodVisitor();
+
Label exit = new Label();
if (safe) {
Label doGet = new Label();
@@ -298,7 +272,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
receiver.visit(controller.getAcg());
}
- mv.visitLdcInsn(methodName); // load property name
+ mv.visitLdcInsn(propertyName);
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Map", "get", "(Ljava/lang/Object;)Ljava/lang/Object;", true);
if (safe) {
mv.visitLabel(exit);
@@ -306,7 +280,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
controller.getOperandStack().replace(OBJECT_TYPE);
}
- private void writeListDotProperty(final Expression receiver, final String methodName, final MethodVisitor mv, final boolean safe) {
+ private void writeListDotProperty(final Expression receiver, final String propertyName, final boolean safe) {
ClassNode componentType = receiver.getNodeMetaData(StaticCompilationMetadataKeys.COMPONENT_TYPE);
if (componentType==null) {
componentType = OBJECT_TYPE;
@@ -316,6 +290,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
// for (e in list) { result.add (e.foo) }
// result
CompileStack compileStack = controller.getCompileStack();
+ MethodVisitor mv = controller.getMethodVisitor();
Label exit = new Label();
if (safe) {
@@ -372,7 +347,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
public ClassNode getType() {
return finalComponentType;
}
- }, methodName);
+ }, propertyName);
pexp.visit(controller.getAcg());
controller.getOperandStack().box();
controller.getOperandStack().remove(1);
@@ -566,7 +541,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
ClassNode replacementType = field.getOriginType();
OperandStack operandStack = controller.getOperandStack();
if (field.isStatic()) {
- mv.visitFieldInsn(GETSTATIC, BytecodeHelper.getClassInternalName(field.getOwner()), fieldName, BytecodeHelper.getTypeDescription(replacementType));
+ mv.visitFieldInsn(GETSTATIC, BytecodeHelper.getClassInternalName(receiverType), fieldName, BytecodeHelper.getTypeDescription(replacementType));
operandStack.push(replacementType);
} else {
if (implicitThis) {
@@ -857,29 +832,57 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
super.fallbackAttributeOrPropertySite(expression, objectExpression, name, adapter);
}
+ private ClassNode getPropertyOwnerType(final Expression receiver, final boolean... isClassReceiver) {
+ Object inferredType = receiver.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+ if (inferredType == null && receiver instanceof VariableExpression) {
+ Variable variable = ((VariableExpression) receiver).getAccessedVariable();
+ if (variable instanceof Expression) {
+ inferredType = ((Expression) variable).getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+ }
+ }
+ ClassNode receiverType;
+ if (inferredType instanceof ClassNode) {
+ // in case a "flow type" is found, it is preferred to use it instead of the declaration type
+ receiverType = (ClassNode) inferredType;
+ } else {
+ receiverType = receiver.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
+ if (receiverType == null) {
+ receiverType = controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
+ }
+ }
+ if (isClassClassNodeWrappingConcreteType(receiverType)) {
+ if (isClassReceiver.length > 0) isClassReceiver[0] = true;
+ receiverType = receiverType.getGenericsTypes()[0].getType();
+ }
+ if (isPrimitiveType(receiverType)) {
+ // GROOVY-6590: wrap primitive types
+ receiverType = getWrapper(receiverType);
+ }
+ return receiverType;
+ }
+
// this is just a simple set field handling static and non-static, but not Closure and inner classes
- private boolean setField(PropertyExpression expression, Expression objectExpression, ClassNode rType, String name) {
+ private boolean setField(final PropertyExpression expression, final Expression objectExpression, final ClassNode receiverType, final String name) {
if (expression.isSafe()) return false;
- FieldNode fn = AsmClassGenerator.getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(controller.getClassNode(), rType, name, false);
- if (fn==null) return false;
+ FieldNode fn = AsmClassGenerator.getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(controller.getClassNode(), receiverType, name, false);
+ if (fn == null) return false;
OperandStack stack = controller.getOperandStack();
stack.doGroovyCast(fn.getType());
MethodVisitor mv = controller.getMethodVisitor();
- String ownerName = BytecodeHelper.getClassInternalName(fn.getOwner());
if (!fn.isStatic()) {
controller.getCompileStack().pushLHS(false);
objectExpression.visit(controller.getAcg());
controller.getCompileStack().popLHS();
- if (!rType.equals(stack.getTopOperand())) {
- BytecodeHelper.doCast(mv, rType);
- stack.replace(rType);
+ if (!receiverType.equals(stack.getTopOperand())) {
+ BytecodeHelper.doCast(mv, receiverType);
+ stack.replace(receiverType);
}
stack.swap();
- mv.visitFieldInsn(PUTFIELD, ownerName, name, BytecodeHelper.getTypeDescription(fn.getType()));
+ mv.visitFieldInsn(PUTFIELD, BytecodeHelper.getClassInternalName(fn.getOwner()), name, BytecodeHelper.getTypeDescription(fn.getType()));
stack.remove(1);
} else {
- mv.visitFieldInsn(PUTSTATIC, ownerName, name, BytecodeHelper.getTypeDescription(fn.getType()));
+ mv.visitFieldInsn(PUTSTATIC, BytecodeHelper.getClassInternalName(receiverType), name, BytecodeHelper.getTypeDescription(fn.getType()));
}
//mv.visitInsn(ACONST_NULL);
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 5e100f7..cd4d50c 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -19,6 +19,7 @@
package groovy.transform.stc
import groovy.transform.NotYetImplemented
+import groovy.transform.PackageScope
/**
* Unit tests for static type checking : fields and properties.
@@ -222,6 +223,19 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-9955
+ void testStaticPropertyWithInheritanceFromAnotherSourceUnit() {
+ assertScript '''
+ import groovy.transform.stc.FieldsAndPropertiesSTCTest.Public
+ assert Public.CONST == 'XX'
+ assert Public.VALUE == null
+ Public.VALUE = 'YY'
+ assert Public.VALUE == 'YY'
+ Public.@VALUE = 'ZZ'
+ assert Public.@VALUE == 'ZZ'
+ '''
+ }
+
void testMethodUsageForProperty() {
assertScript '''
class Foo {
@@ -1052,4 +1066,12 @@ import org.codehaus.groovy.ast.stmt.AssertStatement
static class BaseClass2 extends BaseClass {
}
-}
\ No newline at end of file
+
+ @PackageScope static class PackagePrivate {
+ public static final String CONST = 'XX'
+ public static String VALUE
+ }
+
+ static class Public extends PackagePrivate {
+ }
+}
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 9c37137..cafcf92 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -302,7 +302,7 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
assert B.m() == 0
'''
def b = astTrees['B'][1]
- assert b.contains('GETSTATIC p/A.x')
+ assert b.contains('GETSTATIC B.x')
assert !b.contains('INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.getGroovyObjectProperty')
}
[groovy] 03/03: GROOVY-9967: makeGroovyObjectGetPropertySite:
leverage property owner
Posted by em...@apache.org.
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 42798a8bc60fa71d926ed2bf75e40f97ff9fcdf4
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Mar 29 09:25:43 2021 -0500
GROOVY-9967: makeGroovyObjectGetPropertySite: leverage property owner
Conflicts:
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
---
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 14 ++---
.../transform/stc/TypeInferenceSTCTest.groovy | 52 +++++++++++++++-
.../classgen/asm/sc/bugs/Groovy6276Bug.groovy | 70 ++++++++++------------
3 files changed, 87 insertions(+), 49 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index 33329c6..da059f7 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -416,21 +416,17 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
}
@Override
- public void makeGroovyObjectGetPropertySite(final Expression receiver, final String methodName, final boolean safe, final boolean implicitThis) {
- TypeChooser typeChooser = controller.getTypeChooser();
- ClassNode classNode = controller.getClassNode();
- ClassNode receiverType = typeChooser.resolveType(receiver, classNode);
- if (receiver instanceof VariableExpression && ((VariableExpression) receiver).isThisExpression() && !controller.isInClosure()) {
- receiverType = classNode;
+ public void makeGroovyObjectGetPropertySite(final Expression receiver, final String propertyName, final boolean safe, final boolean implicitThis) {
+ ClassNode receiverType = controller.getClassNode();
+ if (!AsmClassGenerator.isThisExpression(receiver) || controller.isInClosure()) {
+ receiverType = getPropertyOwnerType(receiver); // GROOVY-9967, et al.
}
- String propertyName = methodName;
if (implicitThis) {
if (controller.getInvocationWriter() instanceof StaticInvocationWriter) {
MethodCallExpression currentCall = ((StaticInvocationWriter) controller.getInvocationWriter()).getCurrentCall();
if (currentCall != null && currentCall.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER) != null) {
- propertyName = currentCall.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
- String[] props = propertyName.split("\\.");
+ String[] props = currentCall.<String>getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER).split("\\.");
BytecodeExpression thisLoader = new BytecodeExpression() {
@Override
public void visit(final MethodVisitor mv) {
diff --git a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
index 2ae9b95..cabdcc0 100644
--- a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
+++ b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
@@ -252,7 +252,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testInstanceOfInferenceWithField() {
+ void testInstanceOfInferenceWithProperty1() {
assertScript '''
class A {
int x
@@ -264,7 +264,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testInstanceOfInferenceWithFieldAndAssignment() {
+ void testInstanceOfInferenceWithProperty2() {
shouldFailWithMessages '''
class A {
int x
@@ -276,7 +276,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
''', 'Cannot assign value of type java.lang.String to variable of type int'
}
- void testInstanceOfInferenceWithMissingField() {
+ void testInstanceOfInferenceWithMissingProperty() {
shouldFailWithMessages '''
class A {
int x
@@ -288,6 +288,52 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
''', 'No such property: y for class: A'
}
+ // GROOVY-9967
+ void testInstanceOfInferenceWithSubclassProperty() {
+ assertScript '''
+ class A {
+ int i
+ }
+ class B extends A {
+ String s = 'foo'
+ }
+
+ String scenario1(x) {
+ (x instanceof String) ? x.toLowerCase() : 'bar'
+ }
+ String scenario2(B x) {
+ x.s
+ }
+ String scenario2a(B x) {
+ x.getS()
+ }
+ String scenario3(B x) {
+ (x instanceof B) ? x.s : 'bar'
+ }
+ String scenario3a(B x) {
+ (x instanceof B) ? x.getS() : 'bar'
+ }
+ String scenario4(A x) {
+ (x instanceof B) ? x.s : 'bar' // Access to A#s is forbidden
+ }
+ String scenario4a(A x) {
+ (x instanceof B) ? x.getS() : 'bar'
+ }
+
+ assert scenario1(null) == 'bar'
+ assert scenario1('Foo') == 'foo'
+ assert scenario2(new B()) == 'foo'
+ assert scenario2a(new B()) == 'foo'
+ assert scenario3(new B()) == 'foo'
+ assert scenario3a(new B()) == 'foo'
+
+ assert scenario4(new A()) == 'bar'
+ assert scenario4(new B()) == 'foo'
+ assert scenario4a(new A()) == 'bar'
+ assert scenario4a(new B()) == 'foo'
+ '''
+ }
+
void testShouldNotFailWithWith() {
assertScript '''
class A {
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy6276Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy6276Bug.groovy
index 51fa3ce..171d006 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy6276Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy6276Bug.groovy
@@ -16,56 +16,52 @@
* specific language governing permissions and limitations
* under the License.
*/
-
-
-
-
-
-
package org.codehaus.groovy.classgen.asm.sc.bugs
import groovy.transform.stc.StaticTypeCheckingTestCase
import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
-class Groovy6276Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+final class Groovy6276Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+
void testOuterClassMethodCall() {
- assertScript '''class Outer {
- private int outerField = 1
+ assertScript '''
+ class Outer {
+ private int outerField = 1
- private int outerMethod() { 2 }
- int outerProperty = 3
- class Inner {
- void assertions() {
- assert outerField == 1 // #1
- assert outerMethod() == 2 // #2
- assert outerProperty == 3 // #3
- assert getOuterProperty() == 3 // #4
- }
- }
+ private int outerMethod() { 2 }
+ int outerProperty = 3
+ class Inner {
+ void assertions() {
+ assert outerField == 1 // #1
+ assert outerMethod() == 2 // #2
+ assert outerProperty == 3 // #3
+ assert getOuterProperty() == 3 // #4
+ }
+ }
- void test() {
- new Inner().assertions()
- }
-}
+ void test() {
+ new Inner().assertions()
+ }
+ }
-new Outer().test()
- '''
+ new Outer().test()
+ '''
}
void testAccessPrivateMethodFromClosure() {
assertScript '''
- class Outer {
- private int foo(int x) {
- 2*x
- }
+ class Outer {
+ private int foo(int x) {
+ 2*x
+ }
- public int bar() {
- (Integer) [1,2,3].collect {
- foo(it)
- }.sum()
- }
- }
- new Outer().bar()
-'''
+ int bar() {
+ (Integer) [1,2,3].collect {
+ foo(it)
+ }.sum()
+ }
+ }
+ new Outer().bar()
+ '''
}
}