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 2019/11/26 17:23:37 UTC
[groovy] branch master updated: refactor to move property-only
field checking to visitPropertyExpression
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 3f37f60 refactor to move property-only field checking to visitPropertyExpression
3f37f60 is described below
commit 3f37f60c7d0b4393c3d1f34c4880cb02c7296f62
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Nov 26 11:23:17 2019 -0600
refactor to move property-only field checking to visitPropertyExpression
replace usesSuper(PropertyExpression) with isSuperExpression(Expression)
---
.../groovy/classgen/AsmClassGenerator.java | 207 ++++++++-------------
1 file changed, 78 insertions(+), 129 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index da3d97c..680aa30 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -913,104 +913,27 @@ public class AsmClassGenerator extends ClassGenerator {
}
private void visitAttributeOrProperty(PropertyExpression expression, MethodCallerMultiAdapter adapter) {
- MethodVisitor mv = controller.getMethodVisitor();
-
- Expression objectExpression = expression.getObjectExpression();
ClassNode classNode = controller.getClassNode();
+ String propertyName = expression.getPropertyAsString();
+ Expression objectExpression = expression.getObjectExpression();
- //TODO (blackdrag): this if branch needs a rework. There should be no direct method calls be produced, the
- // handling of this/super could be much simplified (see visitAttributeExpression), the field accessibility check
- // could be moved directly into the search, which would also no longer require the GroovyBugError then
- // the outer class field access seems to be without any tests (if there are tests for that, then the code
- // here is dead code)
- if (isThisOrSuper(objectExpression)) {
- // let's use the field expression if it's available
- String name = expression.getPropertyAsString();
- if (name != null) {
- FieldNode field = null;
- boolean privateSuperField = false;
- if (isSuperExpression(objectExpression)) {
- field = classNode.getSuperClass().getDeclaredField(name);
- if (field != null && field.isPrivate()) {
- privateSuperField = true;
- }
- } else if (expression.isImplicitThis() || !controller.isInClosure()) {
- field = classNode.getDeclaredField(name);
- ClassNode outer = classNode.getOuterClass();
- if (field == null && outer != null) {
- FieldNode outerClassField;
- do {
- outerClassField = outer.getDeclaredField(name);
- if (outerClassField!=null && outerClassField.isStatic() && outerClassField.isFinal()) {
- if (outer != classNode.getOuterClass() && outerClassField.isPrivate()) {
- throw new GroovyBugError("Trying to access private constant field [" + outerClassField.getDeclaringClass() + "#" + outerClassField.getName() + "] from inner class");
- }
- PropertyExpression pexp = new PropertyExpression(
- new ClassExpression(outer),
- expression.getProperty()
- );
- pexp.getObjectExpression().setSourcePosition(objectExpression);
- pexp.visit(controller.getAcg());
- return;
- }
- outer = outer.getSuperClass();
- } while (outer != null);
- }
- if (field == null
- && expression instanceof AttributeExpression
- && isThisExpression(objectExpression)
- && controller.isStaticContext()) {
- // GROOVY-6183
- ClassNode current = classNode.getSuperClass();
- while (field == null && current != null) {
- field = current.getDeclaredField(name);
- current = current.getSuperClass();
- }
- if (field != null && (field.isProtected() || field.isPublic())) {
- visitFieldExpression(new FieldExpression(field));
- return;
- }
- }
- }
- if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
- visitFieldExpression(new FieldExpression(field));
- return;
- }
- if (isSuperExpression(objectExpression)) {
- String prefix;
- if (controller.getCompileStack().isLHS()) {
- setPropertyOfSuperClass(classNode, expression, mv);
- return;
- } else {
- prefix = "get";
- }
- String propName = prefix + capitalize(name);
- visitMethodCallExpression(new MethodCallExpression(objectExpression, propName, MethodCallExpression.NO_ARGUMENTS));
- return;
- }
- }
- }
-
- String propName = expression.getPropertyAsString();
- // TODO: add support for super here too
- if (expression.getObjectExpression() instanceof ClassExpression && "this".equals(propName)) {
+ if (objectExpression instanceof ClassExpression && "this".equals(propertyName)) {
// we have something like A.B.this, and need to make it
// into this.this$0.this$0, where this.this$0 returns
// A.B and this.this$0.this$0 return A.
ClassNode type = objectExpression.getType();
- ClassNode iterType = classNode;
- if (controller.getCompileStack().isInSpecialConstructorCall() && classNode.getOuterClass() != null) {
- boolean staticInnerClass = classNode.isStaticClass();
+ if (controller.getCompileStack().isInSpecialConstructorCall() && type.equals(classNode.getOuterClass())) {
// Outer.this in a special constructor call
- if (classNode.getOuterClass().equals(type)) {
- ConstructorNode ctor = controller.getConstructorNode();
- Expression receiver = !staticInnerClass ? new VariableExpression(ctor.getParameters()[0]) : new ClassExpression(type);
- receiver.setSourcePosition(expression);
- receiver.visit(this);
- return;
- }
+ ConstructorNode ctor = controller.getConstructorNode();
+ Expression receiver = !classNode.isStaticClass() ? new VariableExpression(ctor.getParameters()[0]) : new ClassExpression(type);
+ receiver.setSourcePosition(expression);
+ receiver.visit(this);
+ return;
}
+
+ MethodVisitor mv = controller.getMethodVisitor();
mv.visitVarInsn(ALOAD, 0);
+ ClassNode iterType = classNode;
while (!iterType.equals(type)) {
String ownerName = BytecodeHelper.getClassInternalName(iterType);
if (iterType.getOuterClass() == null) break;
@@ -1018,7 +941,7 @@ public class AsmClassGenerator extends ClassGenerator {
iterType = iterType.getOuterClass();
if (thisField == null) {
// closure within inner class
- while (iterType.isDerivedFrom(ClassHelper.CLOSURE_TYPE)) {
+ while (iterType.isDerivedFrom(ClassHelper.CLOSURE_TYPE) && iterType.implementsInterface(ClassHelper.GENERATED_CLOSURE_Type)) {
// GROOVY-8881: cater for closures within closures - getThisObject is already outer class of all closures
iterType = iterType.getOuterClass();
}
@@ -1040,14 +963,14 @@ public class AsmClassGenerator extends ClassGenerator {
return;
}
- if (propName != null) {
+ if (propertyName != null) {
// TODO: spread safe should be handled inside
if (adapter == getProperty && !expression.isSpreadSafe()) {
- controller.getCallSiteWriter().makeGetPropertySite(objectExpression, propName, expression.isSafe(), expression.isImplicitThis());
+ controller.getCallSiteWriter().makeGetPropertySite(objectExpression, propertyName, expression.isSafe(), expression.isImplicitThis());
} else if (adapter == getGroovyObjectProperty && !expression.isSpreadSafe()) {
- controller.getCallSiteWriter().makeGroovyObjectGetPropertySite(objectExpression, propName, expression.isSafe(), expression.isImplicitThis());
+ controller.getCallSiteWriter().makeGroovyObjectGetPropertySite(objectExpression, propertyName, expression.isSafe(), expression.isImplicitThis());
} else {
- controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, propName, adapter);
+ controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, propertyName, adapter);
}
} else {
controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, null, adapter);
@@ -1113,30 +1036,71 @@ public class AsmClassGenerator extends ClassGenerator {
return objectExpressionType.isDerivedFromGroovyObject();
}
- private boolean isThisOrSuperInStaticContext(Expression objectExpression) {
- return !controller.isInClosure() && controller.isStaticContext() && isThisOrSuper(objectExpression);
- }
-
@Override
public void visitPropertyExpression(PropertyExpression expression) {
Expression objectExpression = expression.getObjectExpression();
OperandStack operandStack = controller.getOperandStack();
int mark = operandStack.getStackLength() - 1;
- MethodCallerMultiAdapter adapter;
- if (controller.getCompileStack().isLHS()) {
- if (isGroovyObject(objectExpression) && !isThisOrSuperInStaticContext(objectExpression)) {
- adapter = setGroovyObjectProperty;
- } else {
- adapter = setProperty;
+ boolean visited = false;
+
+ if (isThisOrSuper(objectExpression)) {
+ String name = expression.getPropertyAsString();
+ if (name != null) {
+ FieldNode field = null;
+ boolean privateSuperField = false;
+ ClassNode classNode = controller.getClassNode();
+
+ if (isSuperExpression(objectExpression)) {
+ field = classNode.getSuperClass().getDeclaredField(name);
+ privateSuperField = (field != null && field.isPrivate());
+ } else if (expression.isImplicitThis() || !controller.isInClosure()) {
+ field = classNode.getDeclaredField(name);
+
+ ClassNode outer = classNode.getOuterClass();
+ if (field == null && outer != null) {
+ do {
+ FieldNode outerClassField = outer.getDeclaredField(name);
+ if (outerClassField != null && outerClassField.isStatic() && outerClassField.isFinal()) {
+ if (outerClassField.isPrivate() && classNode.getOuterClass() != outer) {
+ throw new GroovyBugError("Trying to access private field [" + outerClassField.getDeclaringClass() + "#" + outerClassField.getName() + "] from inner class");
+ }
+ PropertyExpression staticOuterField = new PropertyExpression(new ClassExpression(outer), expression.getProperty());
+ staticOuterField.getObjectExpression().setSourcePosition(objectExpression);
+ staticOuterField.visit(controller.getAcg());
+ return;
+ }
+ outer = outer.getSuperClass();
+ } while (outer != null);
+ }
+ }
+
+ if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
+ visitFieldExpression(new FieldExpression(field));
+ visited = true;
+ } else if (isSuperExpression(objectExpression)) {
+ if (controller.getCompileStack().isLHS()) {
+ setPropertyOfSuperClass(classNode, expression, controller.getMethodVisitor());
+ } else {
+ visitMethodCallExpression(new MethodCallExpression(objectExpression, "get" + capitalize(name), MethodCallExpression.NO_ARGUMENTS));
+ }
+ visited = true;
+ }
}
- } else {
- if (isGroovyObject(objectExpression) && !isThisOrSuperInStaticContext(objectExpression)) {
- adapter = getGroovyObjectProperty;
+ }
+
+ if (!visited) {
+ boolean useMetaObjectProtocol = isGroovyObject(objectExpression)
+ && (!isThisOrSuper(objectExpression) || !controller.isStaticContext() || controller.isInClosure());
+
+ MethodCallerMultiAdapter adapter;
+ if (controller.getCompileStack().isLHS()) {
+ adapter = useMetaObjectProtocol ? setGroovyObjectProperty : setProperty;
} else {
- adapter = getProperty;
+ adapter = useMetaObjectProtocol ? getGroovyObjectProperty : getProperty;
}
+ visitAttributeOrProperty(expression, adapter);
}
- visitAttributeOrProperty(expression, adapter);
+
if (controller.getCompileStack().isLHS()) {
operandStack.remove(operandStack.getStackLength() - mark);
} else {
@@ -1151,18 +1115,13 @@ public class AsmClassGenerator extends ClassGenerator {
int mark = operandStack.getStackLength() - 1;
boolean visited = false;
- // TODO: checking for isThisOrSuper is enough for AttributeExpression, but if this is moved into
- // visitAttributeOrProperty to handle attributes and properties equally, then the extended check should be done
if (isThisOrSuper(objectExpression)) {
- // let's use the field expression if it's available
String name = expression.getPropertyAsString();
if (name != null) {
ClassNode classNode = controller.getClassNode();
FieldNode field = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
if (field != null) {
- FieldExpression fldExp = new FieldExpression(field);
- fldExp.setSourcePosition(expression.getProperty());
- visitFieldExpression(fldExp);
+ visitFieldExpression(new FieldExpression(field));
visited = true;
}
}
@@ -1172,12 +1131,12 @@ public class AsmClassGenerator extends ClassGenerator {
MethodCallerMultiAdapter adapter;
if (controller.getCompileStack().isLHS()) {
adapter = setField;
- if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
- if (usesSuper(expression)) adapter = setFieldOnSuper;
+ if (isSuperExpression(objectExpression)) adapter = setFieldOnSuper;
+ else if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
} else {
adapter = getField;
- if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
- if (usesSuper(expression)) adapter = getFieldOnSuper;
+ if (isSuperExpression(objectExpression)) adapter = getFieldOnSuper;
+ else if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
}
visitAttributeOrProperty(expression, adapter);
}
@@ -1189,16 +1148,6 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private static boolean usesSuper(PropertyExpression pe) {
- Expression objExp = pe.getObjectExpression();
- if (objExp instanceof VariableExpression) {
- VariableExpression varExp = (VariableExpression) objExp;
- String variable = varExp.getName();
- return variable.equals("super");
- }
- return false;
- }
-
@Override
public void visitFieldExpression(FieldExpression expression) {
FieldNode field = expression.getField();