This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 1e4b781 GROOVY-5746: SC: one execution of receiver and subscript for
`a[i] += x`
1e4b781 is described below
commit 1e4b7816896724553f2d2ef6f9bf447e39c8e836
Author: Eric Milles <[email protected]>
AuthorDate: Wed Oct 20 14:37:47 2021 -0500
GROOVY-5746: SC: one execution of receiver and subscript for `a[i] += x`
---
.../transform/sc/TemporaryVariableExpression.java | 19 +++++-----
.../transformers/BinaryExpressionTransformer.java | 42 ++++++++++++----------
.../groovy/transform/stc/STCAssignmentTest.groovy | 29 +++++++++++----
3 files changed, 57 insertions(+), 33 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/transform/sc/TemporaryVariableExpression.java
b/src/main/java/org/codehaus/groovy/transform/sc/TemporaryVariableExpression.java
index ef41205..515cbf8 100644
---
a/src/main/java/org/codehaus/groovy/transform/sc/TemporaryVariableExpression.java
+++
b/src/main/java/org/codehaus/groovy/transform/sc/TemporaryVariableExpression.java
@@ -26,6 +26,8 @@ import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.classgen.asm.ExpressionAsVariableSlot;
import org.codehaus.groovy.classgen.asm.WriterController;
+import static
org.codehaus.groovy.transform.stc.StaticTypesMarker.INFERRED_TYPE;
+
/**
* A front-end class for {@link
org.codehaus.groovy.classgen.asm.ExpressionAsVariableSlot} which
* allows defining temporary variables loaded from variable slots directly at
the AST level,
@@ -37,36 +39,37 @@ public class TemporaryVariableExpression extends Expression
{
private final Expression expression;
- private ExpressionAsVariableSlot variable;
+ private ExpressionAsVariableSlot[] variable = {null};
public TemporaryVariableExpression(final Expression expression) {
this.expression = expression;
+ putNodeMetaData(INFERRED_TYPE,
expression.getNodeMetaData(INFERRED_TYPE));
}
@Override
public Expression transformExpression(final ExpressionTransformer
transformer) {
TemporaryVariableExpression result = new
TemporaryVariableExpression(transformer.transform(expression));
result.copyNodeMetaData(this);
+ result.variable = variable;
return result;
}
@Override
public void visit(final GroovyCodeVisitor visitor) {
if (visitor instanceof AsmClassGenerator) {
- if (variable == null) {
- AsmClassGenerator acg = (AsmClassGenerator) visitor;
- WriterController controller = acg.getController();
- variable = new ExpressionAsVariableSlot(controller,
expression);
+ if (variable[0] == null) {
+ WriterController controller = ((AsmClassGenerator)
visitor).getController();
+ variable[0] = new ExpressionAsVariableSlot(controller,
expression);
}
- variable.visit(visitor);
+ variable[0].visit(visitor);
} else {
expression.visit(visitor);
}
}
public void remove(final WriterController controller) {
- controller.getCompileStack().removeVar(variable.getIndex());
- variable = null;
+ controller.getCompileStack().removeVar(variable[0].getIndex());
+ variable[0] = null;
}
@Override
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 6caafa6..2948187 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
@@ -39,6 +39,7 @@ import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.Types;
import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression;
import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys;
+import org.codehaus.groovy.transform.sc.TemporaryVariableExpression;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
@@ -212,37 +213,42 @@ public class BinaryExpressionTransformer {
return expr;
}
- Expression optimized = tryOptimizeCharComparison(left, right, bin);
- if (optimized != null) {
-
optimized.removeNodeMetaData(StaticCompilationMetadataKeys.BINARY_EXP_TARGET);
-
optimized.removeNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
- return optimized;
+ Expression expr = tryOptimizeCharComparison(left, right, bin);
+ if (expr != null) {
+
expr.removeNodeMetaData(StaticCompilationMetadataKeys.BINARY_EXP_TARGET);
+
expr.removeNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
+ return expr;
}
- String name = (String) list[1];
- MethodNode node = (MethodNode) list[0];
- boolean isAssignment = Types.isAssignment(operationType);
- Expression expr = left; // TODO: if (isAssignment) scrub source
offsets from new copy of left?
+ // replace the binary expression with a method call to
ScriptBytecodeAdapter or something else
MethodNode adapter =
StaticCompilationTransformer.BYTECODE_BINARY_ADAPTERS.get(operationType);
if (adapter != null) {
Expression sba =
classX(StaticCompilationTransformer.BYTECODE_ADAPTER_CLASS);
- call = callX(sba, adapter.getName(), args(expr, right));
+ call = callX(sba, adapter.getName(), args(left, right));
call.setMethodTarget(adapter);
} else {
- call = callX(expr, name, args(right));
- call.setMethodTarget(node);
+ call = callX(left, (String) list[1], args(right));
+ call.setMethodTarget((MethodNode) list[0]);
}
call.setImplicitThis(false);
- if (!isAssignment) {
- call.setSourcePosition(bin);
- return call;
+ if (Types.isAssignment(operationType)) { // +=, -=, /=, ...
+ // GROOVY-5746: one execution of receiver and subscript
+ if (left instanceof BinaryExpression) {
+ BinaryExpression be = (BinaryExpression) left;
+ if (be.getOperation().getType() ==
Types.LEFT_SQUARE_BRACKET) {
+ be.setLeftExpression(new
TemporaryVariableExpression(be.getLeftExpression()));
+ be.setRightExpression(new
TemporaryVariableExpression(be.getRightExpression()));
+ }
+ }
+ // call handles the operation, so we must add the assignment
now
+ expr = binX(left, Token.newSymbol(Types.ASSIGN,
operation.getStartLine(), operation.getStartColumn()), call);
+ } else {
+ expr = call;
}
- // case of +=, -=, /=, ...
- // the method represents the operation type only, and we must add
an assignment
- expr = binX(left, Token.newSymbol(Types.ASSIGN,
operation.getStartLine(), operation.getStartColumn()), call);
expr.setSourcePosition(bin);
return expr;
}
+
if (operationType == Types.ASSIGN && leftExpression instanceof
TupleExpression && rightExpression instanceof ListExpression) {
// multiple assignment
ListOfExpressionsExpression cle = new
ListOfExpressionsExpression();
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index 28307ab..7b8a01f 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -142,14 +142,29 @@ class STCAssignmentTest extends
StaticTypeCheckingTestCase {
assertScript '''
class C {
int i
-
- static main(args) {
- def c = new C()
- c.i = 5
- c.i += 10
- assert c.i == 15
- }
}
+ def c = new C(i: 5)
+ def ret = c.i += 10
+
+ assert c.i == 15
+ assert ret == 15
+ '''
+ }
+
+ // GROOVY-5746
+ void testPlusEqualsAndSubscript() {
+ assertScript '''
+ import groovy.transform.Field
+
+ @Field int i = 0
+ int getIndex() { i++ }
+ def list = ['x','y','z']
+ def x = (list[index] += '!')
+ def y = (list[index] += '!')
+
+ assert x == 'x!'
+ assert y == 'y!'
+ assert list.toString() == '[x!, y!, z]'
'''
}
