Properly process nested try and loop blocks

KT-3894: Loops and finally: finally block executed twice when break and return exists in try/finally block KT-3874: Compilation error on try catch block contains break and continue KT-3869: Loops and finally: outer finally block not run #KT-3869 Fixed #KT-3894 Fixed #KT-3874 Fixed
2013-08-16 13:03:14 +04:00
parent d0f042ba93
commit 185b5013fe
7 changed files with 284 additions and 30 deletions
@@ -1156,17 +1156,17 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
    private StackValue visitBreakOrContinueExpression(@NotNull JetLabelQualifiedExpression expression, StackValue receiver, boolean isBreak) {
        assert expression instanceof JetContinueExpression || expression instanceof JetBreakExpression;

-        JetSimpleNameExpression labelElement = expression.getTargetLabel();
+        if (!blockStackElements.isEmpty()) {
+            BlockStackElement stackElement = blockStackElements.peek();

-        for (int i = blockStackElements.size() - 1; i >= 0; --i) {
-            BlockStackElement stackElement = blockStackElements.get(i);
            if (stackElement instanceof FinallyBlockStackElement) {
                FinallyBlockStackElement finallyBlockStackElement = (FinallyBlockStackElement) stackElement;
                //noinspection ConstantConditions
-                genFinallyBlockOrGoto(finallyBlockStackElement, false, null);
+                genFinallyBlockOrGoto(finallyBlockStackElement, null);
            }
            else if (stackElement instanceof LoopBlockStackElement) {
                LoopBlockStackElement loopBlockStackElement = (LoopBlockStackElement) stackElement;
+                JetSimpleNameExpression labelElement = expression.getTargetLabel();
                //noinspection ConstantConditions
                if (labelElement == null ||
                    loopBlockStackElement.targetLabel != null &&
@@ -1176,11 +1176,18 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
                }
            }
            else {
-                throw new UnsupportedOperationException();
+                throw new UnsupportedOperationException("Wrong BlockStackElement in processing stack");
            }
+
+            blockStackElements.pop();
+            StackValue result = visitBreakOrContinueExpression(expression, receiver, isBreak);
+            blockStackElements.push(stackElement);
+            return result;
+
+
        }

-        throw new UnsupportedOperationException();
+        throw new UnsupportedOperationException("Target label for break/continue not found");
    }

    private StackValue generateSingleBranchIf(
@@ -1510,32 +1517,36 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
    }

    private void doFinallyOnReturn() {
-        for (int i = blockStackElements.size() - 1; i >= 0; --i) {
-            BlockStackElement stackElement = blockStackElements.get(i);
+        if(!blockStackElements.isEmpty()) {
+            BlockStackElement stackElement = blockStackElements.peek();
            if (stackElement instanceof FinallyBlockStackElement) {
                FinallyBlockStackElement finallyBlockStackElement = (FinallyBlockStackElement) stackElement;
-                genFinallyBlockOrGoto(finallyBlockStackElement, true, null);
+                genFinallyBlockOrGoto(finallyBlockStackElement, null);
            }
            else if (stackElement instanceof LoopBlockStackElement) {
-                break;
+
            } else {
-                throw new UnsupportedOperationException();
+                throw new UnsupportedOperationException("Wrong BlockStackElement in processing stack");
            }
+
+            blockStackElements.pop();
+            doFinallyOnReturn();
+            blockStackElements.push(stackElement);
        }
    }

    private void genFinallyBlockOrGoto(
            @Nullable FinallyBlockStackElement finallyBlockStackElement,
-            boolean doPop,
            @Nullable Label tryCatchBlockEnd
    ) {
-        if (finallyBlockStackElement != null) {
-            assert finallyBlockStackElement.gaps.size() % 2 == 0;
-            JetTryExpression jetTryExpression = finallyBlockStackElement.expression;
-            if (doPop) {
-                blockStackElements.pop();
-            }

+        if (finallyBlockStackElement != null) {
+            assert finallyBlockStackElement.gaps.size() % 2 == 0 : "Finally block gaps are inconsistent";
+
+            BlockStackElement topOfStack = blockStackElements.pop();
+            assert topOfStack == finallyBlockStackElement : "Top element of stack doesn't equals processing finally block";
+
+            JetTryExpression jetTryExpression = finallyBlockStackElement.expression;
            Label finallyStart = new Label();
            v.mark(finallyStart);
            finallyBlockStackElement.addGapLabel(finallyStart);
@@ -1553,9 +1564,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
            v.mark(finallyEnd);
            finallyBlockStackElement.addGapLabel(finallyEnd);

-            if (doPop) {
-                blockStackElements.push(finallyBlockStackElement);
-            }
+            blockStackElements.push(finallyBlockStackElement);
        }
    }

@@ -3473,11 +3482,11 @@ The "returned" value of try expression with no finally is either the last expres
        v.mark(tryEnd);

        //do it before finally block generation
-        List<Label> tryBlockRegions = getCatchIntervals(finallyBlockStackElement, tryStart, tryEnd);
+        List<Label> tryBlockRegions = getCurrentCatchIntervals(finallyBlockStackElement, tryStart, tryEnd);

        Label end = new Label();

-        genFinallyBlockOrGoto(finallyBlockStackElement, true, end);
+        genFinallyBlockOrGoto(finallyBlockStackElement, end);

        List<JetCatchClause> clauses = expression.getCatchClauses();
        for (int i = 0, size = clauses.size(); i < size; i++) {
@@ -3501,7 +3510,7 @@ The "returned" value of try expression with no finally is either the last expres

            myFrameMap.leave(descriptor);

-            genFinallyBlockOrGoto(finallyBlockStackElement, true, i != size - 1 || finallyBlock != null ? end : null);
+            genFinallyBlockOrGoto(finallyBlockStackElement, i != size - 1 || finallyBlock != null ? end : null);

            generateExceptionTable(clauseStart, tryBlockRegions, descriptorType.getInternalName());
        }
@@ -3517,11 +3526,11 @@ The "returned" value of try expression with no finally is either the last expres
            v.mark(defaultCatchEnd);

            //do it before finally block generation
-            //javac generates such info for default catch clause too!!!! so defaultCatchEnd as end parameter
-            List<Label> defaultCatchRegions = getCatchIntervals(finallyBlockStackElement, tryStart, defaultCatchEnd);
+            //javac also generates entry in exception table for default catch clause too!!!! so defaultCatchEnd as end parameter
+            List<Label> defaultCatchRegions = getCurrentCatchIntervals(finallyBlockStackElement, tryStart, defaultCatchEnd);


-            genFinallyBlockOrGoto(finallyBlockStackElement, true, null);
+            genFinallyBlockOrGoto(finallyBlockStackElement, null);

            v.load(savedException, JAVA_THROWABLE_TYPE);
            myFrameMap.leaveTemp(JAVA_THROWABLE_TYPE);
@@ -3546,7 +3555,7 @@ The "returned" value of try expression with no finally is either the last expres
        return StackValue.onStack(expectedAsmType);
    }

-    private void generateExceptionTable(Label catchStart, List<Label> catchedRegions, String exception) {
+    private void generateExceptionTable(@NotNull Label catchStart, @NotNull List<Label> catchedRegions, @Nullable String exception) {
        for (int i = 0; i < catchedRegions.size(); i += 2) {
            Label startRegion = catchedRegions.get(i);
            Label endRegion = catchedRegions.get(i+1);
@@ -3555,8 +3564,13 @@ The "returned" value of try expression with no finally is either the last expres
    }


-    private List<Label> getCatchIntervals(FinallyBlockStackElement finallyBlockStackElement, Label blockStart, Label blockEnd) {
-        List<Label> gapsInBlock = finallyBlockStackElement != null ? new ArrayList<Label>(finallyBlockStackElement.gaps) : Collections.<Label>emptyList();
+    private List<Label> getCurrentCatchIntervals(
+            @Nullable FinallyBlockStackElement finallyBlockStackElement,
+            @NotNull Label blockStart,
+            @NotNull Label blockEnd
+    ) {
+        List<Label> gapsInBlock =
+                finallyBlockStackElement != null ? new ArrayList<Label>(finallyBlockStackElement.gaps) : Collections.<Label>emptyList();
        assert gapsInBlock.size() % 2 == 0;
        List<Label> blockRegions = new ArrayList<Label>(gapsInBlock.size() + 2);
        blockRegions.add(blockStart);
@@ -0,0 +1,39 @@
+class MyString {
+    var s = ""
+    fun plus(x : String) : MyString {
+        s += x
+        return this
+    }
+
+    fun toString(): String {
+        return s
+    }
+}
+
+
+fun test1() : MyString {
+    var r = MyString()
+    try {
+        r + "Try1"
+
+        try {
+            r + "Try2"
+            if (true)
+                return r
+        } finally {
+            r + "Finally2"
+            if (true) {
+                return r
+            }
+        }
+
+        return r
+    } finally {
+        r + "Finally1"
+    }
+}
+
+
+fun box(): String {
+  return if (test1().toString() == "Try1Try2Finally2Finally1") "OK" else "fail: ${test1()}"
+}
@@ -0,0 +1,35 @@
+fun test1(): String {
+    var r = ""
+    for (i in 1..2)  {
+        try {
+            r += "O"
+            continue
+        } finally {
+            r += "K"
+            break
+        }
+    }
+    return r
+}
+
+fun test2(): String {
+    var r = ""
+    for (i in 1..2)  {
+        try {
+            r += "O"
+            break
+        } finally {
+            r += "K"
+            continue
+        }
+    }
+    return r
+}
+
+fun box(): String {
+  if (test1() != "OK") return "fail1: ${test1()}"
+
+  if (test2() != "OKOK") return "fail2: ${test2()}"
+
+  return "OK"
+}
@@ -0,0 +1,36 @@
+class MyString {
+    var s = ""
+    fun plus(x : String) : MyString {
+        s += x
+        return this
+    }
+
+    fun toString(): String {
+        return s
+    }
+}
+
+fun test1() : MyString {
+    var r = MyString()
+    while (true) {
+      try {
+          r + "Try"
+
+          if (true) {
+              r + "Break"
+              break
+          }
+
+      } finally {
+          return r + "Finally"
+      }
+    }
+}
+
+
+
+fun box(): String {
+    if (test1().toString() != "TryBreakFinally") return "fail1: ${test1().toString()}"
+
+    return "OK"
+}
@@ -0,0 +1,69 @@
+//KT-3869 Loops and finally: outer finally block not run
+
+class MyString {
+    var s = ""
+    fun plus(x : String) : MyString {
+        s += x
+        return this
+    }
+
+    fun toString(): String {
+        return s
+    }
+}
+
+fun test1() : MyString {
+    var r = MyString()
+    try {
+        r + "Try"
+
+        while(r.toString() != "") {
+            return r + "Loop"
+        }
+
+        return r + "Fail"
+    } finally {
+        r + "Finally"
+    }
+}
+
+fun test2() : MyString {
+    var r = MyString()
+    try {
+        r + "Try"
+
+        do {
+            if (r.toString() != "") {
+                return r + "Loop"
+            }
+        } while (r.toString() != "")
+
+        return r + "Fail"
+    } finally {
+        r + "Finally"
+    }
+}
+
+fun test3() : MyString {
+    var r = MyString()
+    try {
+        r + "Try"
+
+        for(i in 1..2) {
+            r + "Loop"
+            return r
+        }
+
+        return r + "Fail"
+    } finally {
+        r + "Finally"
+    }
+}
+
+fun box(): String {
+    if (test1().toString() != "TryLoopFinally") return "fail1: ${test1()}"
+    if (test2().toString() != "TryLoopFinally") return "fail2: ${test2()}"
+    if (test3().toString() != "TryLoopFinally") return "fail3: ${test3()}"
+
+    return "OK"
+}
@@ -0,0 +1,36 @@
+//test for appropriate
+
+class MyString {
+    var s = ""
+    fun plus(x : String) : MyString {
+        s += x
+        return this
+    }
+
+    fun toString(): String {
+        return s
+    }
+}
+
+
+fun test1() : MyString {
+    var r = MyString()
+    try {
+        r + "Try1"
+        for(i in 1..1) {
+            try {
+                r + "Try2"
+            } finally {
+                return r + "Finally2"
+            }
+        }
+
+    } finally {
+        r + "Finally1"
+    }
+    return r  + "Fail"
+}
+
+fun box(): String {
+  return if (test1().toString() == "Try1Try2Finally2Finally1") "OK" else "fail: ${test1()}"
+}
@@ -2102,6 +2102,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("compiler/testData/codegen/box/finally"), Pattern.compile("^(.+)\\.kt$"), true);
        }
        
+        @TestMetadata("finallyAndFinally.kt")
+        public void testFinallyAndFinally() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/finallyAndFinally.kt");
+        }
+        
        @TestMetadata("kt3549.kt")
        public void testKt3549() throws Exception {
            doTest("compiler/testData/codegen/box/finally/kt3549.kt");
@@ -2112,6 +2117,21 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            doTest("compiler/testData/codegen/box/finally/kt3867.kt");
        }
        
+        @TestMetadata("kt3874.kt")
+        public void testKt3874() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/kt3874.kt");
+        }
+        
+        @TestMetadata("kt3894.kt")
+        public void testKt3894() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/kt3894.kt");
+        }
+        
+        @TestMetadata("loopAndFinally.kt")
+        public void testLoopAndFinally() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/loopAndFinally.kt");
+        }
+        
        @TestMetadata("notChainCatch.kt")
        public void testNotChainCatch() throws Exception {
            doTest("compiler/testData/codegen/box/finally/notChainCatch.kt");
@@ -2122,6 +2142,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            doTest("compiler/testData/codegen/box/finally/tryFinally.kt");
        }
        
+        @TestMetadata("tryLoopTry.kt")
+        public void testTryLoopTry() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/tryLoopTry.kt");
+        }
+        
    }
    
    @TestMetadata("compiler/testData/codegen/box/functions")