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:41 UTC
[groovy] 01/01: GROOVY-9700: SC: transform variable assignment to
direct setter call
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 '''