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 2020/05/27 20:01:01 UTC
[groovy] branch master updated: minor refactor
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 ae351e3 minor refactor
ae351e3 is described below
commit ae351e38f96a172bd14bc3d94ea6a86d17270c8c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed May 27 14:38:19 2020 -0500
minor refactor
---
.../classgen/asm/StatementMetaTypeChooser.java | 15 ++-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 104 +++++++++++----------
.../classgen/asm/sc/StaticTypesTypeChooser.java | 30 +++---
.../VariableExpressionTransformer.java | 43 ++++-----
4 files changed, 103 insertions(+), 89 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementMetaTypeChooser.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementMetaTypeChooser.java
index c02b4f3..819dd17 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementMetaTypeChooser.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementMetaTypeChooser.java
@@ -21,6 +21,7 @@ package org.codehaus.groovy.classgen.asm;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.GenericsType;
import org.codehaus.groovy.ast.Variable;
import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.Expression;
@@ -30,20 +31,28 @@ import org.codehaus.groovy.ast.expr.VariableExpression;
* A {@link TypeChooser} which is aware of statement metadata.
*/
public class StatementMetaTypeChooser implements TypeChooser {
+ @Override
public ClassNode resolveType(final Expression exp, final ClassNode current) {
- if (exp instanceof ClassExpression) return ClassHelper.CLASS_Type;
- OptimizingStatementWriter.StatementMeta meta = exp.getNodeMetaData(OptimizingStatementWriter.StatementMeta.class);
ClassNode type = null;
+ if (exp instanceof ClassExpression) { type = exp.getType();
+ ClassNode classType = ClassHelper.makeWithoutCaching("java.lang.Class");
+ classType.setGenericsTypes(new GenericsType[] {new GenericsType(type)});
+ classType.setRedirect(ClassHelper.CLASS_Type);
+ return classType;
+ }
+
+ OptimizingStatementWriter.StatementMeta meta = exp.getNodeMetaData(OptimizingStatementWriter.StatementMeta.class);
if (meta != null) type = meta.type;
if (type != null) return type;
+
if (exp instanceof VariableExpression) {
VariableExpression ve = (VariableExpression) exp;
if (ve.isClosureSharedVariable()) return ve.getType();
- type = ve.getOriginType();
if (ve.getAccessedVariable() instanceof FieldNode) {
FieldNode fn = (FieldNode) ve.getAccessedVariable();
if (!fn.getDeclaringClass().equals(current)) return fn.getOriginType();
}
+ type = ve.getOriginType();
} else if (exp instanceof Variable) {
Variable v = (Variable) exp;
type = v.getOriginType();
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 313ee38..f909433 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
@@ -75,6 +75,7 @@ import static org.codehaus.groovy.ast.ClassHelper.boolean_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.getUnwrapper;
import static org.codehaus.groovy.ast.ClassHelper.getWrapper;
import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.isGeneratedFunction;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX;
@@ -376,37 +377,48 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
private boolean makeGetPrivateFieldWithBridgeMethod(final Expression receiver, final ClassNode receiverType, final String fieldName, final boolean safe, final boolean implicitThis) {
FieldNode field = receiverType.getField(fieldName);
- ClassNode outerClass = receiverType.getOuterClass();
- if (field == null && implicitThis && outerClass != null && !receiverType.isStaticClass()) {
- Expression pexp;
- if (controller.isInGeneratedFunction()) {
- MethodCallExpression call = callThisX("getThisObject");
- call.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, controller.getOutermostClass());
- call.setImplicitThis(true);
- call.setMethodTarget(CLOSURE_GETTHISOBJECT_METHOD);
- pexp = castX(controller.getOutermostClass(), call);
- } else {
- pexp = propX(classX(outerClass), "this");
- ((PropertyExpression) pexp).setImplicitThis(true);
+ if (field != null) {
+ ClassNode classNode = controller.getClassNode();
+ if (field.isPrivate() && !receiverType.equals(classNode)
+ && (StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(receiverType, classNode)
+ || StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(classNode, receiverType))) {
+ Map<String, MethodNode> accessors = receiverType.redirect().getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_FIELDS_ACCESSORS);
+ if (accessors != null) {
+ MethodNode methodNode = accessors.get(fieldName);
+ if (methodNode != null) {
+ MethodCallExpression call = callX(receiver, methodNode.getName(), args(field.isStatic() ? nullX() : receiver));
+ call.setImplicitThis(implicitThis);
+ call.setMethodTarget(methodNode);
+ call.setSafe(safe);
+ call.visit(controller.getAcg());
+ return true;
+ }
+ }
}
- pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, outerClass);
- pexp.setSourcePosition(receiver);
- return makeGetPrivateFieldWithBridgeMethod(pexp, outerClass, fieldName, safe, true);
- }
- ClassNode classNode = controller.getClassNode();
- if (field != null && field.isPrivate() && !receiverType.equals(classNode)
- && (StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(receiverType, classNode) || StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(classNode, receiverType))) {
- Map<String, MethodNode> accessors = receiverType.redirect().getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_FIELDS_ACCESSORS);
- if (accessors != null) {
- MethodNode methodNode = accessors.get(fieldName);
- if (methodNode != null) {
- MethodCallExpression call = callX(receiver, methodNode.getName(), args(field.isStatic() ? nullX() : receiver));
- call.setImplicitThis(implicitThis);
- call.setMethodTarget(methodNode);
- call.setSafe(safe);
- call.visit(controller.getAcg());
- return true;
+ } else if (implicitThis) {
+ ClassNode outerClass = receiverType.getOuterClass();
+ if (outerClass != null && !receiverType.isStaticClass()) {
+ Expression expr;
+ ClassNode thisType = outerClass;
+ if (controller.isInGeneratedFunction()) {
+ while (isGeneratedFunction(thisType)) {
+ thisType = thisType.getOuterClass();
+ }
+
+ MethodCallExpression call = callThisX("getThisObject");
+ call.setImplicitThis(true);
+ call.setMethodTarget(CLOSURE_GETTHISOBJECT_METHOD);
+ call.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, thisType);
+
+ expr = castX(thisType, call);
+ } else {
+ expr = propX(classX(outerClass), "this");
+ ((PropertyExpression) expr).setImplicitThis(true);
}
+ expr.setSourcePosition(receiver);
+ expr.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, thisType);
+ // try again with "(Outer) getThisObject()" or "Outer.this" as receiver
+ return makeGetPrivateFieldWithBridgeMethod(expr, outerClass, fieldName, safe, true);
}
}
return false;
@@ -414,29 +426,25 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
@Override
public void makeGroovyObjectGetPropertySite(final Expression receiver, final String propertyName, 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.isInGeneratedFunction()) {
- receiverType = classNode;
+ ClassNode receiverType = controller.getClassNode();
+ if (!AsmClassGenerator.isThisExpression(receiver) || controller.isInGeneratedFunction()) {
+ receiverType = controller.getTypeChooser().resolveType(receiver, receiverType);
}
String property = propertyName;
- if (implicitThis) {
- if (controller.getInvocationWriter() instanceof StaticInvocationWriter) {
- MethodCallExpression currentCall = ((StaticInvocationWriter) controller.getInvocationWriter()).getCurrentCall();
- if (currentCall != null && currentCall.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER) != null) {
- property = currentCall.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
- String[] props = property.split("\\.");
- BytecodeExpression thisLoader = bytecodeX(CLOSURE_TYPE, mv -> mv.visitVarInsn(ALOAD, 0));
- PropertyExpression pexp = propX(thisLoader, constX(props[0]), safe);
- for (int i = 1, n = props.length; i < n; i += 1) {
- pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, CLOSURE_TYPE);
- pexp = propX(pexp, props[i]);
- }
- pexp.visit(controller.getAcg());
- return;
+ if (implicitThis && controller.getInvocationWriter() instanceof StaticInvocationWriter) {
+ Expression currentCall = ((StaticInvocationWriter) controller.getInvocationWriter()).getCurrentCall();
+ if (currentCall != null && currentCall.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER) != null) {
+ property = currentCall.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
+ String[] props = property.split("\\.");
+ BytecodeExpression thisLoader = bytecodeX(CLOSURE_TYPE, mv -> mv.visitVarInsn(ALOAD, 0));
+ PropertyExpression pexp = propX(thisLoader, constX(props[0]), safe);
+ for (int i = 1, n = props.length; i < n; i += 1) {
+ pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, CLOSURE_TYPE);
+ pexp = propX(pexp, props[i]);
}
+ pexp.visit(controller.getAcg());
+ return;
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java
index fbcaa94..e0fbb42 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java
@@ -22,6 +22,7 @@ import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.Variable;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.classgen.asm.StatementMetaTypeChooser;
@@ -35,26 +36,27 @@ public class StaticTypesTypeChooser extends StatementMetaTypeChooser {
@Override
public ClassNode resolveType(final Expression exp, final ClassNode current) {
ASTNode target = exp instanceof VariableExpression ? getTarget((VariableExpression) exp) : exp;
+
ClassNode inferredType = target.getNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE);
if (inferredType == null) {
inferredType = target.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
- if (inferredType == null && target instanceof VariableExpression && ((VariableExpression) target).getAccessedVariable() instanceof Parameter) {
- target = (Parameter) ((VariableExpression) target).getAccessedVariable();
- inferredType = ((Parameter) target).getOriginType();
+ if (inferredType == null && target instanceof VariableExpression) {
+ Variable variable = ((VariableExpression) target).getAccessedVariable();
+ if (variable instanceof Parameter) {
+ target = (Parameter) variable;
+ inferredType = variable.getOriginType();
+ }
}
}
- if (inferredType != null) {
- if (ClassHelper.VOID_TYPE == inferredType) {
- // we are in a case of a type inference failure, probably because code was generated
- // it is better to avoid using this
- inferredType = super.resolveType(exp, current);
- }
+ if (inferredType != null && !ClassHelper.VOID_TYPE.equals(inferredType)) {
return inferredType;
}
+
if (target instanceof VariableExpression && ((VariableExpression) target).isThisExpression()) {
// AsmClassGenerator may create "this" expressions that the type checker knows nothing about
return current;
}
+
return super.resolveType(exp, current);
}
@@ -65,9 +67,11 @@ public class StaticTypesTypeChooser extends StatementMetaTypeChooser {
* @param ve the variable expression for which to return the target expression
* @return the target variable expression
*/
- private static VariableExpression getTarget(VariableExpression ve) {
- if (ve.getAccessedVariable() == null || ve.getAccessedVariable() == ve || (!(ve.getAccessedVariable() instanceof VariableExpression)))
- return ve;
- return getTarget((VariableExpression) ve.getAccessedVariable());
+ private static VariableExpression getTarget(final VariableExpression ve) {
+ Variable variable = ve.getAccessedVariable();
+ if (variable != ve && variable instanceof VariableExpression) {
+ return getTarget((VariableExpression) variable);
+ }
+ return ve;
}
}
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
index 84aeb72..d092b92 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
@@ -26,38 +26,38 @@ import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+
/**
* Transformer for VariableExpression the bytecode backend wouldn't be able to
* handle otherwise.
*/
public class VariableExpressionTransformer {
- public Expression transformVariableExpression(VariableExpression expr) {
+ public Expression transformVariableExpression(final VariableExpression expr) {
Expression trn = tryTransformDelegateToProperty(expr);
- if (trn != null) {
- return trn;
- }
- trn = tryTransformPrivateFieldAccess(expr);
- if (trn != null) {
- return trn;
+ if (trn == null) {
+ trn = tryTransformPrivateFieldAccess(expr);
}
- return expr;
+ return trn != null ? trn : expr;
}
- private static Expression tryTransformDelegateToProperty(VariableExpression expr) {
+ private static Expression tryTransformDelegateToProperty(final VariableExpression expr) {
// we need to transform variable expressions that go to a delegate
// to a property expression, as ACG would lose the information in
// processClassVariable before it reaches any makeCall, that could handle it
Object val = expr.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
if (val == null) return null;
- // TODO handle the owner and delegate cases better for nested scenarios and potentially remove the need for the implicit this case
- VariableExpression receiver = new VariableExpression("owner".equals(val) ? (String) val : "delegate".equals(val) ? (String) val : "this");
+ // TODO: handle the owner and delegate cases better for nested scenarios and potentially remove the need for the implicit this case
+ Expression receiver = varX("owner".equals(val) ? (String) val : "delegate".equals(val) ? (String) val : "this");
// GROOVY-9136 -- object expression should not overlap source range of property; property stands in for original varibale expression
receiver.setLineNumber(expr.getLineNumber());
receiver.setColumnNumber(expr.getColumnNumber());
- PropertyExpression pexp = new PropertyExpression(receiver, expr.getName());
+ PropertyExpression pexp = propX(receiver, expr.getName());
pexp.getProperty().setSourcePosition(expr);
pexp.copyNodeMetaData(expr);
pexp.setImplicitThis(true);
@@ -71,25 +71,18 @@ public class VariableExpressionTransformer {
return pexp;
}
- private static Expression tryTransformPrivateFieldAccess(VariableExpression expr) {
+ private static Expression tryTransformPrivateFieldAccess(final VariableExpression expr) {
FieldNode field = expr.getNodeMetaData(StaticTypesMarker.PV_FIELDS_ACCESS);
if (field == null) {
field = expr.getNodeMetaData(StaticTypesMarker.PV_FIELDS_MUTATION);
}
if (field != null) {
- // access to a private field from a section of code that normally doesn't have access to it, like a
- // closure or an inner class
- VariableExpression receiver = new VariableExpression("this");
- PropertyExpression pexp = new PropertyExpression(
- receiver,
- expr.getName()
- );
- pexp.setImplicitThis(true);
+ // access to a private field from a section of code that normally doesn't have access to it, like a closure or an inner class
+ PropertyExpression pexp = thisPropX(true, expr.getName());
+ // store the declaring class so that the class writer knows that it will have to call a bridge method
+ pexp.getObjectExpression().putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, field.getDeclaringClass());
+ pexp.putNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE, field.getOriginType());
pexp.getProperty().setSourcePosition(expr);
- // put the receiver inferred type so that the class writer knows that it will have to call a bridge method
- receiver.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, field.getDeclaringClass());
- // add inferred type information
- pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, field.getOriginType());
return pexp;
}
return null;