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/08/22 02:31:40 UTC

[groovy] branch GROOVY-9700 created (now 0503b00)

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

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


      at 0503b00  GROOVY-9700: SC: transform variable assignment to direct setter call

This branch includes the following new commits:

     new 0503b00  GROOVY-9700: SC: transform variable assignment to direct setter call

The 1 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.



[groovy] 01/01: GROOVY-9700: SC: transform variable assignment to direct setter call

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 0503b00ff9c471540942a7d1345b33da6ce62e3e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Aug 21 21:31:25 2020 -0500

    GROOVY-9700: SC: transform variable assignment to direct setter call
---
 .../transformers/BinaryExpressionTransformer.java  | 59 +++++++++++++++++-----
 .../sc/FieldsAndPropertiesStaticCompileTest.groovy | 30 +++++++++++
 2 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
index 31df52a..86261aa 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
@@ -102,11 +102,31 @@ public class BinaryExpressionTransformer {
             MethodNode directMCT = leftExpression.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
             if (directMCT != null) {
                 Expression left = staticCompilationTransformer.transform(leftExpression);
+                Expression right = staticCompilationTransformer.transform(rightExpression);
                 if (left instanceof PropertyExpression) {
-                    Expression right = staticCompilationTransformer.transform(rightExpression);
-                    return transformPropertyAssignmentToSetterCall((PropertyExpression) left, right, directMCT);
+                    // transform "a.x = val" into "def tmp = val; a.setX(tmp); tmp"
+                    PropertyExpression pe = (PropertyExpression) left;
+                    return transformAssignmentToSetterCall(
+                            pe.getObjectExpression(), // "a"
+                            directMCT, // "setX"
+                            right, // "val"
+                            false,
+                            pe.isSafe(),
+                            pe.getProperty(), // "x"
+                            bin // "a.x = val"
+                    );
+                } else if (left instanceof VariableExpression) {
+                    // transform "x = val" into "def tmp = val; this.setX(tmp); tmp"
+                    return transformAssignmentToSetterCall(
+                            varX("this"),
+                            directMCT, // "setX"
+                            right, // "val"
+                            true,
+                            false,
+                            left, // "x"
+                            bin // "x = val"
+                    );
                 }
-                // TODO: Handle left instanceof VariableExpression and has DIRECT_METHOD_CALL_TARGET?
             }
         } else if (operationType == Types.COMPARE_EQUAL || operationType == Types.COMPARE_NOT_EQUAL) {
             // let's check if one of the operands is the null constant
@@ -357,17 +377,30 @@ public class BinaryExpressionTransformer {
         throw new IllegalArgumentException("Unsupported conversion");
     }
 
-    private static Expression transformPropertyAssignmentToSetterCall(final PropertyExpression leftExpression, final Expression rightExpression, final MethodNode directMCT) {
-        // transform "a.x = b" into "def tmp = b; a.setX(tmp); tmp"
+    /**
+     * Adapter for {@link StaticPropertyAccessHelper#transformToSetterCall}.
+     */
+    private static Expression transformAssignmentToSetterCall(
+            final Expression receiver,
+            final MethodNode setterMethod,
+            final Expression valueExpression,
+            final boolean implicitThis,
+            final boolean safeNavigation,
+            final Expression nameExpression,
+            final Expression binaryExpression) {
+        // expression that will transfer assignment and name positions
+        Expression pos = new PropertyExpression(null, nameExpression);
+        pos.setSourcePosition(binaryExpression);
+
         return StaticPropertyAccessHelper.transformToSetterCall(
-                leftExpression.getObjectExpression(),
-                directMCT,
-                rightExpression,
-                false,
-                leftExpression.isSafe(),
-                false,
-                true, // to be replaced with a proper test whether a return value should be used or not
-                leftExpression
+                receiver,
+                setterMethod,
+                valueExpression,
+                implicitThis,
+                safeNavigation,
+                false, // spreadSafe
+                true, // TODO: replace with a proper test whether a return value is required or not
+                pos
         );
     }
 }
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
index 8a0a33d..3150e2b 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -610,6 +610,36 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
         '''
     }
 
+    // GROOVY-9700
+    void testVariableAssignmentUsesDirectSetterCall() {
+        assertScript '''
+            import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
+
+            class Foo {
+                void setX(Date value) {}
+                void setX(Long value) {}
+            }
+            class Bar extends Foo {
+                @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                    def assignment = node.code.statements[0].expression
+                    assert assignment instanceof ListOfExpressionsExpression
+                        assignment = node.code.statements[1].expression
+                    assert assignment instanceof ListOfExpressionsExpression
+                })
+                void test() {
+                    x = 42L
+                    x = new Date()
+                }
+            }
+            new Bar().test()
+        '''
+
+        def bar = astTrees['Bar'][1]
+        assert bar.contains('INVOKEVIRTUAL Bar.setX (Ljava/lang/Long;)V')
+        assert bar.contains('INVOKEVIRTUAL Bar.setX (Ljava/util/Date;)V')
+        assert !bar.contains('INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.setGroovyObjectProperty ')
+    }
+
     void testPrivateFieldMutationInClosureUsesBridgeMethod() {
         try {
             assertScript '''