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/15 08:55:55 UTC
incubator-groovy git commit: GROOVY-7656 Statically compiled spread
safe method calls on list literals result in the list expression being
evaluated twice * StatementWriter: Remove temporary variables after their use
Repository: incubator-groovy
Updated Branches:
refs/heads/master e3e9d41a2 -> a1d85c855
GROOVY-7656 Statically compiled spread safe method calls on list literals result in the list expression being evaluated twice
* StatementWriter: Remove temporary variables after their use
Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/a1d85c85
Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/a1d85c85
Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/a1d85c85
Branch: refs/heads/master
Commit: a1d85c855699239e076c8c7bc5c62399a1e3a0e7
Parents: e3e9d41
Author: Shil Sinha <sh...@gmail.com>
Authored: Mon Nov 2 13:54:25 2015 -0500
Committer: Shil Sinha <sh...@apache.org>
Committed: Sun Nov 15 02:54:50 2015 -0500
----------------------------------------------------------------------
.../groovy/classgen/asm/StatementWriter.java | 9 +++++++--
.../classgen/asm/sc/StaticInvocationWriter.java | 8 ++++++++
.../asm/sc/StaticTypesStatementWriter.java | 2 +-
.../ArraysAndCollectionsStaticCompileTest.groovy | 19 +++++++++++++++++++
4 files changed, 35 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a1d85c85/src/main/org/codehaus/groovy/classgen/asm/StatementWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/org/codehaus/groovy/classgen/asm/StatementWriter.java
index 2ef012a..fb755da 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -145,6 +145,7 @@ public class StatementWriter {
mv.visitJumpInsn(GOTO, continueLabel);
mv.visitLabel(breakLabel);
+ compileStack.removeVar(iteratorIdx);
compileStack.pop();
}
@@ -391,7 +392,7 @@ public class StatementWriter {
//store exception
//TODO: maybe define a Throwable and use it here instead of Object
operandStack.push(ClassHelper.OBJECT_TYPE);
- int anyExceptionIndex = compileStack.defineTemporaryVariable("exception", true);
+ final int anyExceptionIndex = compileStack.defineTemporaryVariable("exception", true);
finallyStatement.visit(controller.getAcg());
@@ -400,6 +401,7 @@ public class StatementWriter {
mv.visitInsn(ATHROW);
mv.visitLabel(skipCatchAll);
+ compileStack.removeVar(anyExceptionIndex);
}
private BlockRecorder makeBlockRecorder(final Statement finallyStatement) {
@@ -425,7 +427,7 @@ public class StatementWriter {
// switch does not have a continue label. use its parent's for continue
Label breakLabel = controller.getCompileStack().pushSwitch();
- int switchVariableIndex = controller.getCompileStack().defineTemporaryVariable("switch", true);
+ final int switchVariableIndex = controller.getCompileStack().defineTemporaryVariable("switch", true);
List caseStatements = statement.getCaseStatements();
int caseCount = caseStatements.size();
@@ -444,6 +446,7 @@ public class StatementWriter {
controller.getMethodVisitor().visitLabel(breakLabel);
+ controller.getCompileStack().removeVar(switchVariableIndex);
controller.getCompileStack().pop();
}
@@ -542,6 +545,7 @@ public class StatementWriter {
mv.visitInsn(ATHROW);
mv.visitLabel(synchronizedEnd);
+ compileStack.removeVar(index);
}
public void writeAssert(AssertStatement statement) {
@@ -591,6 +595,7 @@ public class StatementWriter {
int returnValueIdx = controller.getCompileStack().defineTemporaryVariable("returnValue", returnType, true);
controller.getCompileStack().applyBlockRecorder();
operandStack.load(type, returnValueIdx);
+ controller.getCompileStack().removeVar(returnValueIdx);
}
BytecodeHelper.doReturn(mv, returnType);
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a1d85c85/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 12cca0e..0745125 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -37,6 +37,7 @@ import org.codehaus.groovy.syntax.SyntaxException;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys;
import org.codehaus.groovy.transform.sc.StaticCompilationVisitor;
+import org.codehaus.groovy.transform.sc.TemporaryVariableExpression;
import org.codehaus.groovy.transform.stc.ExtensionMethodNode;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
@@ -445,6 +446,13 @@ 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;
+ }
MethodVisitor mv = controller.getMethodVisitor();
CompileStack compileStack = controller.getCompileStack();
TypeChooser typeChooser = controller.getTypeChooser();
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a1d85c85/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 8423dd7..dc41b7e 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
@@ -234,7 +234,7 @@ public class StaticTypesStatementWriter extends StatementWriter {
mv.visitJumpInsn(GOTO, continueLabel);
mv.visitLabel(breakLabel);
-
+ compileStack.removeVar(iteratorIdx);
}
private void writeEnumerationBasedForEachLoop(
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a1d85c85/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 af86990..4a9fbc5 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
@@ -80,6 +80,25 @@ class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsSTCTest
'''
}
+ // GROOVY-7656
+ void testSpreadSafeMethodCallsOnListLiteralShouldNotCreateListTwice() {
+ try {
+ assertScript '''
+ class Foo {
+ static void test() {
+ def list = [1, 2]
+ def lengths = [list << 3]*.size()
+ assert lengths == [3]
+ assert list == [1, 2, 3]
+ }
+ }
+ Foo.test()
+ '''
+ } finally {
+ assert astTrees['Foo'][1].count('ScriptBytecodeAdapter.createList') == 4
+ }
+ }
+
@Override
void testForInLoop() {
try {