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 2022/09/25 11:00:32 UTC

[groovy] 01/02: GROOVY-10757: STC: don't write inference metadata to `PropertyNode`

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

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

commit e820f2dfc1d78396ba075bf697a46a6091d1a472
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Sep 25 05:17:44 2022 -0500

    GROOVY-10757: STC: don't write inference metadata to `PropertyNode`
---
 .../VariableExpressionTransformer.java             | 86 ++++++++++++++--------
 .../transform/stc/StaticTypeCheckingVisitor.java   | 14 ++--
 2 files changed, 60 insertions(+), 40 deletions(-)

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 978fcec160..95298d3947 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
@@ -20,16 +20,18 @@ package org.codehaus.groovy.transform.sc.transformers;
 
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.PropertyExpression;
 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.callThisX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
 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
@@ -37,56 +39,76 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
  */
 public class VariableExpressionTransformer {
 
-    public Expression transformVariableExpression(final VariableExpression expr) {
-        Expression trn = tryTransformDelegateToProperty(expr);
-        if (trn == null) {
-            trn = tryTransformPrivateFieldAccess(expr);
+    public Expression transformVariableExpression(final VariableExpression ve) {
+        Expression xe = tryTransformImplicitReceiver(ve);
+        if (xe == null) {
+            xe = tryTransformPrivateFieldAccess(ve);
         }
-        return trn != null ? trn : expr;
+        if (xe == null) {
+            xe = tryTransformDirectMethodTarget(ve);
+        }
+        if (xe != null) {
+            return xe;
+        }
+        return ve;
     }
 
-    private static Expression tryTransformDelegateToProperty(final VariableExpression expr) {
+    private static Expression tryTransformImplicitReceiver(final VariableExpression ve) {
         // 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 || val.equals(expr.getName())) return null;
+        Object val = ve.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
+        if (val == null || val.equals(ve.getName())) return null;
 
         // 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 variable expression
-        receiver.setLineNumber(expr.getLineNumber());
-        receiver.setColumnNumber(expr.getColumnNumber());
+        Expression receiver = new VariableExpression("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 variable expression
+        receiver.setColumnNumber(ve.getColumnNumber());
+        receiver.setLineNumber(ve.getLineNumber());
 
-        PropertyExpression pexp = propX(receiver, expr.getName());
-        pexp.getProperty().setSourcePosition(expr);
-        pexp.copyNodeMetaData(expr);
-        pexp.setImplicitThis(true);
+        PropertyExpression pe = propX(receiver, ve.getName());
+        pe.getProperty().setSourcePosition(ve);
+        pe.setImplicitThis(true);
+        pe.copyNodeMetaData(ve);
 
-        ClassNode owner = expr.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
+        ClassNode owner = ve.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
         if (owner != null) {
             receiver.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, owner);
             receiver.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, val);
         }
-        pexp.removeNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
+        pe.removeNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
 
-        return pexp;
+        return pe;
     }
 
-    private static Expression tryTransformPrivateFieldAccess(final VariableExpression expr) {
-        FieldNode field = expr.getNodeMetaData(StaticTypesMarker.PV_FIELDS_ACCESS);
+    private static Expression tryTransformPrivateFieldAccess(final VariableExpression ve) {
+        FieldNode field = ve.getNodeMetaData(StaticTypesMarker.PV_FIELDS_ACCESS);
+        if (field == null) {
+            field = ve.getNodeMetaData(StaticTypesMarker.PV_FIELDS_MUTATION);
+        }
         if (field == null) {
-            field = expr.getNodeMetaData(StaticTypesMarker.PV_FIELDS_MUTATION);
+            return null;
         }
-        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
-            PropertyExpression pexp = !field.isStatic() ? thisPropX(true, expr.getName()) : propX(classX(field.getDeclaringClass()), 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);
-            return pexp;
+
+        // access to a private field from a section of code that normally doesn't have access to it, like a closure block or an inner class
+        PropertyExpression pe = !field.isStatic() ? thisPropX(true, ve.getName()) : propX(classX(field.getDeclaringClass()), ve.getName());
+        // store the declaring class so that the class writer knows that it will have to call a bridge method
+        pe.getObjectExpression().putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, field.getDeclaringClass());
+        pe.putNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE, field.getOriginType());
+        pe.getProperty().setSourcePosition(ve);
+        return pe;
+    }
+
+    private static Expression tryTransformDirectMethodTarget(final VariableExpression ve) {
+        MethodNode dmct = ve.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
+        // NOTE: BinaryExpressionTransformer handles the setter
+        if (dmct == null || dmct.getParameters().length != 0) {
+            return null;
         }
-        return null;
+
+        MethodCallExpression mce = callThisX(dmct.getName());
+        mce.getMethod().setSourcePosition(ve);
+        mce.setMethodTarget(dmct);
+        return mce;
     }
 }
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index a31020a956..d4d27ac07e 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4397,22 +4397,20 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             VariableExpression var = (VariableExpression) exp;
             Variable accessedVariable = var.getAccessedVariable();
             if (accessedVariable instanceof VariableExpression) {
-                if (accessedVariable != exp)
+                if (accessedVariable != var)
                     storeType((VariableExpression) accessedVariable, cn);
-            } else if (accessedVariable instanceof Parameter
-                    || accessedVariable instanceof PropertyNode
-                    && ((PropertyNode) accessedVariable).getField().isSynthetic()) {
-                ((AnnotatedNode) accessedVariable).putNodeMetaData(INFERRED_TYPE, cn);
+            } else if (accessedVariable instanceof Parameter) {
+                ((Parameter) accessedVariable).putNodeMetaData(INFERRED_TYPE, cn);
             }
             if (cn != null && var.isClosureSharedVariable()) {
-                List<ClassNode> assignedTypes = typeCheckingContext.closureSharedVariablesAssignmentTypes.computeIfAbsent(var, k -> new LinkedList<ClassNode>());
+                List<ClassNode> assignedTypes = typeCheckingContext.closureSharedVariablesAssignmentTypes.computeIfAbsent(var, k -> new LinkedList<>());
                 assignedTypes.add(cn);
             }
             if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
-                List<ClassNode> temporaryTypesForExpression = getTemporaryTypesForExpression(exp);
+                List<ClassNode> temporaryTypesForExpression = getTemporaryTypesForExpression(var);
                 if (temporaryTypesForExpression != null && !temporaryTypesForExpression.isEmpty()) {
                     // a type inference has been made on a variable whose type was defined in an instanceof block
-                    // we erase available information with the new type
+                    // erase available information with the new type
                     temporaryTypesForExpression.clear();
                 }
             }