You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by sh...@apache.org on 2015/11/30 19:33:47 UTC
incubator-groovy git commit: GROOVY-7688 Spread safe method call
receivers with side effects should not be visited twice *
StaticInvocationWriter#makeCall: use temporary variables for spread safe
receiver expressions which are not variable or constant ex
Repository: incubator-groovy
Updated Branches:
refs/heads/master 4e3f1703e -> 9a565bd72
GROOVY-7688 Spread safe method call receivers with side effects should not be visited twice
* StaticInvocationWriter#makeCall: use temporary variables for spread safe receiver expressions which are not variable or constant expressions
* StaticTypesStatementWriter#writeOptimizedForEachLoop: remove temporary variables from compileStack after their use
Closes #199
Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/9a565bd7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/9a565bd7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/9a565bd7
Branch: refs/heads/master
Commit: 9a565bd72dd6d3b7a6ee98ed89e8ee4beba05bb3
Parents: 4e3f170
Author: Shil Sinha <sh...@apache.org>
Authored: Sat Nov 28 12:42:54 2015 -0500
Committer: Shil Sinha <sh...@apache.org>
Committed: Mon Nov 30 13:22:55 2015 -0500
----------------------------------------------------------------------
.../classgen/asm/sc/StaticInvocationWriter.java | 21 +++++++++++---------
.../asm/sc/StaticTypesStatementWriter.java | 3 +++
...ArraysAndCollectionsStaticCompileTest.groovy | 18 +++++++++++++++++
3 files changed, 33 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/9a565bd7/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 0745125..b9f65a2 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -446,12 +446,10 @@ public class StaticInvocationWriter extends InvocationWriter {
}
// if call is spread safe, replace it with a for in loop
if (spreadSafe && origin instanceof MethodCallExpression) {
- // List literals should not be visited twice, avoid by using a temporary variable for the receiver
- if (receiver instanceof ListExpression) {
- TemporaryVariableExpression tmp = new TemporaryVariableExpression(receiver);
- makeCall(origin, tmp, message, arguments, adapter, true, true, false);
- tmp.remove(controller);
- return;
+ // receiver expressions with side effects should not be visited twice, avoid by using a temporary variable
+ Expression tmpReceiver = receiver;
+ if (!(receiver instanceof VariableExpression) && !(receiver instanceof ConstantExpression)) {
+ tmpReceiver = new TemporaryVariableExpression(receiver);
}
MethodVisitor mv = controller.getMethodVisitor();
CompileStack compileStack = controller.getCompileStack();
@@ -475,13 +473,13 @@ public class StaticInvocationWriter extends InvocationWriter {
declr.visit(controller.getAcg());
operandStack.pop();
// if (receiver != null)
- receiver.visit(controller.getAcg());
+ tmpReceiver.visit(controller.getAcg());
Label ifnull = compileStack.createLocalLabel("ifnull_" + counter);
mv.visitJumpInsn(IFNULL, ifnull);
operandStack.remove(1); // receiver consumed by if()
Label nonull = compileStack.createLocalLabel("nonull_" + counter);
mv.visitLabel(nonull);
- ClassNode componentType = StaticTypeCheckingVisitor.inferLoopElementType(typeChooser.resolveType(receiver, classNode));
+ ClassNode componentType = StaticTypeCheckingVisitor.inferLoopElementType(typeChooser.resolveType(tmpReceiver, classNode));
Parameter iterator = new Parameter(componentType, "for$it$" + counter);
VariableExpression iteratorAsVar = new VariableExpression(iterator);
MethodCallExpression origMCE = (MethodCallExpression) origin;
@@ -503,7 +501,7 @@ public class StaticInvocationWriter extends InvocationWriter {
// for (e in receiver) { result.add(e?.method(arguments) }
ForStatement stmt = new ForStatement(
iterator,
- receiver,
+ tmpReceiver,
new ExpressionStatement(add)
);
stmt.visit(controller.getAcg());
@@ -513,6 +511,11 @@ public class StaticInvocationWriter extends InvocationWriter {
// end of if/else
// return result list
result.visit(controller.getAcg());
+
+ // cleanup temporary variables
+ if (tmpReceiver instanceof TemporaryVariableExpression) {
+ ((TemporaryVariableExpression) tmpReceiver).remove(controller);
+ }
} else if (safe && origin instanceof MethodCallExpression) {
// wrap call in an IFNULL check
MethodVisitor mv = controller.getMethodVisitor();
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/9a565bd7/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
index dc41b7e..9a4f997 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
@@ -145,6 +145,9 @@ public class StaticTypesStatementWriter extends StatementWriter {
mv.visitLabel(breakLabel);
+ compileStack.removeVar(loopIdx);
+ compileStack.removeVar(arrayLen);
+ compileStack.removeVar(array);
}
private void loadFromArray(MethodVisitor mv, BytecodeVariable variable, int array, int iteratorIdx) {
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/9a565bd7/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
index 4d9cb64..fbb4f3c 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
@@ -110,6 +110,24 @@ class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsSTCTest
'''
}
+ //GROOVY-7688
+ void testSpreadSafeMethodCallReceiversWithSideEffectsShouldNotBeVisitedTwice() {
+ try {
+ assertScript '''
+ class Foo {
+ static void test() {
+ def list = ['a', 'b']
+ def lengths = list.toList()*.length()
+ assert lengths == [1, 1]
+ }
+ }
+ Foo.test()
+ '''
+ } finally {
+ assert astTrees['Foo'][1].count('DefaultGroovyMethods.toList') == 1
+ }
+ }
+
@Override
void testForInLoop() {
try {