Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@groovy.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: sunlan@apache.org
To: commits@groovy.apache.org
Message-Id: <c4e2862ab18844448f26d858ea22c228@git.apache.org>
Subject: groovy git commit: GROOVY-8229: nested try-catch-finally handling
 inside Closures generates wrong bytecode
Date: Fri, 16 Jun 2017 00:10:47 +0000 (UTC)
archived-at: Fri, 16 Jun 2017 00:10:49 -0000

Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_5_X 8e6cccdbc -> 2cd09b563


GROOVY-8229: nested try-catch-finally handling inside Closures generates wrong bytecode

(cherry picked from commit 0406787)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/2cd09b56
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/2cd09b56
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/2cd09b56

Branch: refs/heads/GROOVY_2_5_X
Commit: 2cd09b563835f26fa1b3ba2ed36e74ba9c9a77d2
Parents: 8e6cccd
Author: sunlan <sunlan@apache.org>
Authored: Thu Jun 15 22:46:21 2017 +0800
Committer: sunlan <sunlan@apache.org>
Committed: Fri Jun 16 08:10:38 2017 +0800

----------------------------------------------------------------------
 .../groovy/classgen/asm/CompileStack.java       | 28 +++++++-----
 src/test/groovy/bugs/Groovy8229Bug.groovy       | 46 ++++++++++++++++++++
 2 files changed, 63 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/2cd09b56/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java b/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java
index ba856bc..2d7d8b1 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java
@@ -29,7 +29,13 @@ import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 
-import java.util.*;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.ListIterator;
+import java.util.Map;
 
 /**
  * This class is a helper for AsmClassGenerator. It manages
@@ -567,7 +573,7 @@ public class CompileStack implements Opcodes {
         return answer;
     }
 
-    private void makeLocalVariablesOffset(Parameter[] paras,boolean isInStaticContext) {
+    private void makeLocalVariablesOffset(Parameter[] paras, boolean isInStaticContext) {
         resetVariableIndex(isInStaticContext);
 
         for (Parameter para : paras) {
@@ -633,11 +639,11 @@ public class CompileStack implements Opcodes {
 
     private static void pushInitValue(ClassNode type, MethodVisitor mv) {
         if (ClassHelper.isPrimitiveType(type)) {
-            if (type==ClassHelper.long_TYPE) {
+            if (type== ClassHelper.long_TYPE) {
                 mv.visitInsn(LCONST_0);
-            } else if (type==ClassHelper.double_TYPE) {
+            } else if (type== ClassHelper.double_TYPE) {
                 mv.visitInsn(DCONST_0);
-            } else if (type==ClassHelper.float_TYPE) {
+            } else if (type== ClassHelper.float_TYPE) {
                 mv.visitInsn(FCONST_0);
             } else {
                 mv.visitLdcInsn(0);
@@ -706,7 +712,7 @@ public class CompileStack implements Opcodes {
      */
     private void makeNextVariableID(ClassNode type, boolean useReferenceDirectly) {
         currentVariableIndex = nextVariableIndex;
-        if ((type==ClassHelper.long_TYPE || type==ClassHelper.double_TYPE) && !useReferenceDirectly) {
+        if ((type== ClassHelper.long_TYPE || type== ClassHelper.double_TYPE) && !useReferenceDirectly) {
             nextVariableIndex++;
         }
         nextVariableIndex++;
@@ -767,10 +773,12 @@ public class CompileStack implements Opcodes {
         applyBlockRecorder(blocks);
     }
 
+
     private void applyBlockRecorder(List<BlockRecorder> blocks) {
-        if (blocks.isEmpty() || blocks.size()==visitedBlocks.size()) return;
+        if (blocks.isEmpty() || blocks.size() == visitedBlocks.size()) return;
 
         MethodVisitor mv = controller.getMethodVisitor();
+        Label newStart = new Label();
 
         for (BlockRecorder fb : blocks) {
             if (visitedBlocks.contains(fb)) continue;
@@ -785,13 +793,11 @@ public class CompileStack implements Opcodes {
             // here to avoid double visiting of finally statements
             fb.excludedStatement.run();
 
-            Label newStart = new Label();
             fb.startRange(newStart);
-
-            mv.visitInsn(NOP);
-            mv.visitLabel(newStart);
         }
 
+        mv.visitInsn(NOP);
+        mv.visitLabel(newStart);
     }
 
     public void applyBlockRecorder() {

http://git-wip-us.apache.org/repos/asf/groovy/blob/2cd09b56/src/test/groovy/bugs/Groovy8229Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8229Bug.groovy b/src/test/groovy/bugs/Groovy8229Bug.groovy
new file mode 100644
index 0000000..eac96a2
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8229Bug.groovy
@@ -0,0 +1,46 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+class Groovy8229Bug extends GroovyTestCase {
+    void testFinallyBlockInClosureCalledOnce() {
+        assert shouldFail(
+                '''
+                class TryCatchProblem {
+                    static int count = 0
+                    static void main(args) {
+                        def cl = {
+                            try {
+                                try {
+                                    assert count == 0
+                                } catch (Throwable e) { }
+                            } finally {
+                                check()
+                            }
+                        }
+                        cl()
+                    }
+                    static void check() {
+                        throw new UnsupportedOperationException("check call count: ${++count}")
+                    }
+                }
+            '''
+        ) == 'check call count: 1'
+    }
+}