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:58 UTC
[groovy] 01/03: GROOVY-9955: access static fields 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 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')
}