groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sh...@apache.org
Subject 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
Date Sun, 15 Nov 2015 07:55:55 GMT
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 <shil.sinha@gmail.com>
Authored: Mon Nov 2 13:54:25 2015 -0500
Committer: Shil Sinha <shils@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 {


Mime
View raw message