You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/05/30 15:18:30 UTC

[groovy] 01/11: minor refactor

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 8bd69f4630028952a8db8b990909535ccad2e2ee
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed May 27 14:38:19 2020 -0500

    minor refactor
    
    (cherry picked from commit ae351e38f96a172bd14bc3d94ea6a86d17270c8c)
---
 .../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;