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:31 UTC

[groovy] branch GROOVY_3_0_X updated (a4b35c3fe2 -> 00f460f7c4)

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

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


    from a4b35c3fe2 GROOVY-10700: STC: non-static (not just default) interface/trait methods
     new e820f2dfc1 GROOVY-10757: STC: don't write inference metadata to `PropertyNode`
     new 00f460f7c4 GROOVY-10637: SC: parameterized return type for getter method

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../VariableExpressionTransformer.java             | 89 ++++++++++++++--------
 .../transform/stc/StaticTypeCheckingVisitor.java   | 14 ++--
 .../transform/stc/ConstructorsSTCTest.groovy       | 10 +--
 .../groovy/transform/stc/GenericsSTCTest.groovy    |  1 -
 src/test/groovy/transform/stc/MyBean.java          | 13 ++--
 5 files changed, 75 insertions(+), 52 deletions(-)


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

Posted by em...@apache.org.
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();
                 }
             }


[groovy] 02/02: GROOVY-10637: SC: parameterized return type for getter method

Posted by em...@apache.org.
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 00f460f7c471df572ba1a1d6fb2d1340486a678c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu May 26 15:17:52 2022 -0500

    GROOVY-10637: SC: parameterized return type for getter method
---
 .../sc/transformers/VariableExpressionTransformer.java      |  3 +++
 src/test/groovy/transform/stc/ConstructorsSTCTest.groovy    | 10 +++++-----
 src/test/groovy/transform/stc/GenericsSTCTest.groovy        |  1 -
 src/test/groovy/transform/stc/MyBean.java                   | 13 +++++++------
 4 files changed, 15 insertions(+), 12 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 95298d3947..cd90269c40 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
@@ -109,6 +109,9 @@ public class VariableExpressionTransformer {
         MethodCallExpression mce = callThisX(dmct.getName());
         mce.getMethod().setSourcePosition(ve);
         mce.setMethodTarget(dmct);
+        // GROOVY-10637: return type might be parameterized
+        mce.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE,
+         ve.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE));
         return mce;
     }
 }
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index c00eaff84e..b8f36a656f 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -346,17 +346,17 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
     void testConstructJavaBeanFromMap() {
         assertScript '''import groovy.transform.stc.MyBean
 
-        MyBean bean = new MyBean(name:'Cedric')
-        assert bean.name == 'Cedric'
+        MyBean bean = new MyBean<String>(value:'Cedric')
+        assert bean.value == 'Cedric'
         '''
     }
     void testConstructJavaBeanFromMapAndSubclass() {
         assertScript '''import groovy.transform.stc.MyBean
-        class MyBean2 extends MyBean {
+        class MyBean2 extends MyBean<String> {
             int age
         }
-        MyBean2 bean = new MyBean2(name:'Cedric', age:33)
-        assert bean.name == 'Cedric'
+        MyBean2 bean = new MyBean2(value:'Cedric', age:33)
+        assert bean.value == 'Cedric'
         assert bean.age == 33
         '''
     }
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index eb308f3b23..8c97b6ed2e 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -717,7 +717,6 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
     }
 
     // GROOVY-10637
-    @NotYetImplemented
     void testReturnTypeInferenceWithMethodGenerics27() {
         assertScript '''
             class Outer extends groovy.transform.stc.MyBean<Inner> {
diff --git a/src/test/groovy/transform/stc/MyBean.java b/src/test/groovy/transform/stc/MyBean.java
index c50c8e2b65..7471e5b4e2 100644
--- a/src/test/groovy/transform/stc/MyBean.java
+++ b/src/test/groovy/transform/stc/MyBean.java
@@ -21,14 +21,15 @@ package groovy.transform.stc;
 /**
  * A simple Java bean, used by unit test for GROOVY-5578
  */
-public class MyBean {
-    private String name;
+public class MyBean<T> {
 
-    public String getName() {
-        return name;
+    private T value;
+
+    public T getValue() {
+        return value;
     }
 
-    public void setName(final String name) {
-        this.name = name;
+    public void setValue(final T value) {
+        this.value = value;
     }
 }