Properly generate exception table for try/catch block

KT-3549 'Finally' block not run when re-throwing exception KT-3867 When catched exception occurs in finnaly block it invokes additional catch and itself #KT-3549 Fixed #KT-3867 Fixed
2013-08-15 17:22:20 +04:00
parent eaf0c2cb84
commit d0f042ba93
6 changed files with 345 additions and 60 deletions
@@ -145,13 +145,18 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
    }

    static class FinallyBlockStackElement extends BlockStackElement {
+        List<Label> gaps = new ArrayList();
+
        final JetTryExpression expression;

        FinallyBlockStackElement(JetTryExpression expression) {
            this.expression = expression;
        }
-    }

+        private void addGapLabel(Label label){
+            gaps.add(label);
+        }
+    }

    public ExpressionCodegen(
            @NotNull MethodVisitor v,
@@ -1139,45 +1144,26 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem

    @Override
    public StackValue visitBreakExpression(JetBreakExpression expression, StackValue receiver) {
-        JetSimpleNameExpression labelElement = expression.getTargetLabel();
-
-        for (int i = blockStackElements.size() - 1; i >= 0; --i) {
-            BlockStackElement stackElement = blockStackElements.get(i);
-            if (stackElement instanceof FinallyBlockStackElement) {
-                FinallyBlockStackElement finallyBlockStackElement = (FinallyBlockStackElement) stackElement;
-                JetTryExpression jetTryExpression = finallyBlockStackElement.expression;
-                //noinspection ConstantConditions
-                gen(jetTryExpression.getFinallyBlock().getFinalExpression(), Type.VOID_TYPE);
-            }
-            else if (stackElement instanceof LoopBlockStackElement) {
-                LoopBlockStackElement loopBlockStackElement = (LoopBlockStackElement) stackElement;
-                //noinspection ConstantConditions
-                if (labelElement == null ||
-                    loopBlockStackElement.targetLabel != null &&
-                    labelElement.getReferencedName().equals(loopBlockStackElement.targetLabel.getReferencedName())) {
-                    v.goTo(loopBlockStackElement.breakLabel);
-                    return StackValue.none();
-                }
-            }
-            else {
-                throw new UnsupportedOperationException();
-            }
-        }
-
-        throw new UnsupportedOperationException();
+        return visitBreakOrContinueExpression(expression, receiver, true);
    }

    @Override
    public StackValue visitContinueExpression(JetContinueExpression expression, StackValue receiver) {
+        return visitBreakOrContinueExpression(expression, receiver, false);
+    }
+
+    @NotNull
+    private StackValue visitBreakOrContinueExpression(@NotNull JetLabelQualifiedExpression expression, StackValue receiver, boolean isBreak) {
+        assert expression instanceof JetContinueExpression || expression instanceof JetBreakExpression;
+
        JetSimpleNameExpression labelElement = expression.getTargetLabel();

        for (int i = blockStackElements.size() - 1; i >= 0; --i) {
            BlockStackElement stackElement = blockStackElements.get(i);
            if (stackElement instanceof FinallyBlockStackElement) {
                FinallyBlockStackElement finallyBlockStackElement = (FinallyBlockStackElement) stackElement;
-                JetTryExpression jetTryExpression = finallyBlockStackElement.expression;
                //noinspection ConstantConditions
-                gen(jetTryExpression.getFinallyBlock().getFinalExpression(), Type.VOID_TYPE);
+                genFinallyBlockOrGoto(finallyBlockStackElement, false, null);
            }
            else if (stackElement instanceof LoopBlockStackElement) {
                LoopBlockStackElement loopBlockStackElement = (LoopBlockStackElement) stackElement;
@@ -1185,7 +1171,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
                if (labelElement == null ||
                    loopBlockStackElement.targetLabel != null &&
                    labelElement.getReferencedName().equals(loopBlockStackElement.targetLabel.getReferencedName())) {
-                    v.goTo(loopBlockStackElement.continueLabel);
+                    v.goTo(isBreak ? loopBlockStackElement.breakLabel : loopBlockStackElement.continueLabel);
                    return StackValue.none();
                }
            }
@@ -1528,14 +1514,47 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
            BlockStackElement stackElement = blockStackElements.get(i);
            if (stackElement instanceof FinallyBlockStackElement) {
                FinallyBlockStackElement finallyBlockStackElement = (FinallyBlockStackElement) stackElement;
-                JetTryExpression jetTryExpression = finallyBlockStackElement.expression;
-                blockStackElements.pop();
-                //noinspection ConstantConditions
-                gen(jetTryExpression.getFinallyBlock().getFinalExpression(), Type.VOID_TYPE);
-                blockStackElements.push(finallyBlockStackElement);
+                genFinallyBlockOrGoto(finallyBlockStackElement, true, null);
            }
-            else {
+            else if (stackElement instanceof LoopBlockStackElement) {
                break;
+            } else {
+                throw new UnsupportedOperationException();
+            }
+        }
+    }
+
+    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();
+            }
+
+            Label finallyStart = new Label();
+            v.mark(finallyStart);
+            finallyBlockStackElement.addGapLabel(finallyStart);
+
+            //noinspection ConstantConditions
+            gen(jetTryExpression.getFinallyBlock().getFinalExpression(), Type.VOID_TYPE);
+        }
+
+        if (tryCatchBlockEnd != null) {
+            v.goTo(tryCatchBlockEnd);
+        }
+
+        if (finallyBlockStackElement != null) {
+            Label finallyEnd = new Label();
+            v.mark(finallyEnd);
+            finallyBlockStackElement.addGapLabel(finallyEnd);
+
+            if (doPop) {
+                blockStackElements.push(finallyBlockStackElement);
            }
        }
    }
@@ -3452,13 +3471,13 @@ The "returned" value of try expression with no finally is either the last expres

        Label tryEnd = new Label();
        v.mark(tryEnd);
-        if (finallyBlock != null) {
-            blockStackElements.pop();
-            gen(finallyBlock.getFinalExpression(), Type.VOID_TYPE);
-            blockStackElements.push(finallyBlockStackElement);
-        }
+
+        //do it before finally block generation
+        List<Label> tryBlockRegions = getCatchIntervals(finallyBlockStackElement, tryStart, tryEnd);
+
        Label end = new Label();
-        v.goTo(end);
+
+        genFinallyBlockOrGoto(finallyBlockStackElement, true, end);

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

            myFrameMap.leave(descriptor);

-            if (finallyBlock != null) {
-                blockStackElements.pop();
-                gen(finallyBlock.getFinalExpression(), Type.VOID_TYPE);
-                blockStackElements.push(finallyBlockStackElement);
-            }
+            genFinallyBlockOrGoto(finallyBlockStackElement, true, i != size - 1 || finallyBlock != null ? end : null);

-            if (i != size - 1 || finallyBlock != null) {
-                v.goTo(end);
-            }
-
-            v.visitTryCatchBlock(tryStart, tryEnd, clauseStart, descriptorType.getInternalName());
+            generateExceptionTable(clauseStart, tryBlockRegions, descriptorType.getInternalName());
        }

-        if (finallyBlock != null) {
-            Label finallyStart = new Label();
-            v.mark(finallyStart);

+        //for default catch clause
+        if (finallyBlock != null) {
+            Label defaultCatchStart = new Label();
+            v.mark(defaultCatchStart);
            int savedException = myFrameMap.enterTemp(JAVA_THROWABLE_TYPE);
            v.store(savedException, JAVA_THROWABLE_TYPE);
+            Label defaultCatchEnd = new Label();
+            v.mark(defaultCatchEnd);

-            blockStackElements.pop();
-            gen(finallyBlock.getFinalExpression(), Type.VOID_TYPE);
-            blockStackElements.push(finallyBlockStackElement);
+            //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);
+
+
+            genFinallyBlockOrGoto(finallyBlockStackElement, true, null);

            v.load(savedException, JAVA_THROWABLE_TYPE);
            myFrameMap.leaveTemp(JAVA_THROWABLE_TYPE);

            v.athrow();

-            v.visitTryCatchBlock(tryStart, tryEnd, finallyStart, null);
+            generateExceptionTable(defaultCatchStart, defaultCatchRegions, null);
        }

        markLineNumber(expression);
@@ -3529,6 +3546,25 @@ 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) {
+        for (int i = 0; i < catchedRegions.size(); i += 2) {
+            Label startRegion = catchedRegions.get(i);
+            Label endRegion = catchedRegions.get(i+1);
+            v.visitTryCatchBlock(startRegion, endRegion, catchStart, exception);
+        }
+    }
+
+
+    private List<Label> getCatchIntervals(FinallyBlockStackElement finallyBlockStackElement, Label blockStart, 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);
+        blockRegions.addAll(gapsInBlock);
+        blockRegions.add(blockEnd);
+        return blockRegions;
+    }
+
    @Override
    public StackValue visitBinaryWithTypeRHSExpression(JetBinaryExpressionWithTypeRHS expression, StackValue receiver) {
        JetSimpleNameExpression operationSign = expression.getOperationReference();
@@ -0,0 +1,41 @@
+fun test1() : String {
+    var s = "";
+    try {
+        try {
+            s += "Try";
+            throw Exception()
+        } catch (x : Exception) {
+            s += "Catch";
+            throw x
+        } finally {
+            s += "Finally";
+        }
+    } catch (x : Exception) {
+        return s
+    }
+}
+
+fun test2() : String {
+    var s = "";
+
+    try {
+        s += "Try";
+        throw Exception()
+    } catch (x : Exception) {
+        s += "Catch";
+    } finally {
+        s += "Finally";
+    }
+
+    return s
+}
+
+
+
+fun box() : String {
+    if (test1() != "TryCatchFinally") return "fail1: ${test1()}"
+
+    if (test2() != "TryCatchFinally") return "fail2: ${test2()}"
+
+    return "OK"
+}
@@ -0,0 +1,46 @@
+fun fail() = if (true) throw RuntimeException() else 1
+
+fun test1(): String {
+    var r = ""
+    try {
+        try {
+            r += "Try"
+            return r
+        } catch (e: RuntimeException) {
+            r += "Catch"
+            return r
+        }
+        finally {
+            r += "Finally"
+            fail()
+        }
+    } catch (e: RuntimeException) {
+        return r
+    }
+}
+
+fun test2(): String {
+    var r = ""
+    try {
+        try {
+            r += "Try"
+        } catch (e: RuntimeException) {
+            r += "Catch"
+        }
+        finally {
+            r += "Finally"
+            fail()
+        }
+    } catch (e: RuntimeException) {
+        return r
+    }
+    return r + "Fail"
+}
+
+fun box(): String {
+   if (test1() != "TryFinally") return "fail1: ${test1()}"
+
+   if (test2() != "TryFinally") return "fail2: ${test2()}"
+
+   return "OK"
+}
@@ -0,0 +1,93 @@
+fun unsupportedEx() = if (true) throw UnsupportedOperationException()
+fun runtimeEx() = if (true) throw RuntimeException()
+
+fun test1() : String {
+    var s = "";
+    try {
+        try {
+            s += "Try";
+            unsupportedEx()
+        } catch (x : UnsupportedOperationException) {
+            s += "Catch";
+            runtimeEx()
+        } catch (e: RuntimeException) {
+            s += "WrongCatch"
+        }
+    } catch (x : RuntimeException) {
+        return s
+    }
+    return s + "Failed"
+}
+
+fun test1WithFinally() : String {
+    var s = "";
+    try {
+        try {
+            s += "Try";
+            unsupportedEx()
+        } catch (x : UnsupportedOperationException) {
+            s += "Catch";
+            runtimeEx()
+        } catch (e: RuntimeException) {
+            s += "WrongCatch"
+        } finally {
+            s += "Finally"
+        }
+    } catch (x : RuntimeException) {
+        return s
+    }
+    return s + "Failed"
+}
+
+fun test2() : String {
+    var s = "";
+    try {
+        try {
+            s += "Try";
+            unsupportedEx()
+            return s
+        } catch (x : UnsupportedOperationException) {
+            s += "Catch";
+            runtimeEx()
+            return s
+        } catch (e: RuntimeException) {
+            s += "WrongCatch"
+        }
+    } catch (x : RuntimeException) {
+        return s
+    }
+    return s + "Failed"
+}
+
+fun test2WithFinally() : String {
+    var s = "";
+    try {
+        try {
+            s += "Try";
+            unsupportedEx()
+            return s
+        } catch (x : UnsupportedOperationException) {
+            s += "Catch";
+            runtimeEx()
+            return s
+        } catch (e: RuntimeException) {
+            s += "WrongCatch"
+        } finally {
+            s += "Finally"
+        }
+    } catch (x : RuntimeException) {
+        return s
+    }
+    return s + "Failed"
+}
+
+
+
+fun box() : String {
+    if (test1() != "TryCatch") return "fail1: ${test1()}"
+    if (test1WithFinally() != "TryCatchFinally") return "fail2: ${test1WithFinally()}"
+
+    if (test2() != "TryCatch") return "fail3: ${test2()}"
+    if (test2WithFinally() != "TryCatchFinally") return "fail4: ${test2WithFinally()}"
+    return "OK"
+}
@@ -0,0 +1,40 @@
+fun unsupportedEx() = if (true) throw UnsupportedOperationException()
+fun runtimeEx() = if (true) throw RuntimeException()
+
+fun test1WithFinally() : String {
+    var s = "";
+    try {
+        try {
+            s += "Try";
+            unsupportedEx()
+        } finally {
+            s += "Finally"
+        }
+    } catch (x : RuntimeException) {
+        return s
+    }
+    return s + "Failed"
+}
+
+
+fun test2WithFinally() : String {
+    var s = "";
+    try {
+        try {
+            s += "Try";
+            unsupportedEx()
+            return s
+        } finally {
+            s += "Finally"
+        }
+    } catch (x : RuntimeException) {
+        return s
+    }
+}
+
+fun box() : String {
+    if (test1WithFinally() != "TryFinally") return "fail2: ${test1WithFinally()}"
+
+    if (test2WithFinally() != "TryFinally") return "fail4: ${test2WithFinally()}"
+    return "OK"
+}
@@ -31,7 +31,7 @@ import org.jetbrains.jet.codegen.generated.AbstractBlackBoxCodegenTest;
 /** This class is generated by {@link org.jetbrains.jet.generators.tests.GenerateTests}. DO NOT MODIFY MANUALLY */
@SuppressWarnings("all")
@TestMetadata("compiler/testData/codegen/box")
-@InnerTestClasses({BlackBoxCodegenTestGenerated.Arrays.class, BlackBoxCodegenTestGenerated.Bridges.class, BlackBoxCodegenTestGenerated.CallableReference.class, BlackBoxCodegenTestGenerated.Casts.class, BlackBoxCodegenTestGenerated.Classes.class, BlackBoxCodegenTestGenerated.Closures.class, BlackBoxCodegenTestGenerated.ControlStructures.class, BlackBoxCodegenTestGenerated.DefaultArguments.class, BlackBoxCodegenTestGenerated.DelegatedProperty.class, BlackBoxCodegenTestGenerated.Elvis.class, BlackBoxCodegenTestGenerated.Enum.class, BlackBoxCodegenTestGenerated.ExclExcl.class, BlackBoxCodegenTestGenerated.ExtensionFunctions.class, BlackBoxCodegenTestGenerated.ExtensionProperties.class, BlackBoxCodegenTestGenerated.FieldRename.class, BlackBoxCodegenTestGenerated.Functions.class, BlackBoxCodegenTestGenerated.InnerNested.class, BlackBoxCodegenTestGenerated.Instructions.class, BlackBoxCodegenTestGenerated.Intrinsics.class, BlackBoxCodegenTestGenerated.Labels.class, BlackBoxCodegenTestGenerated.LocalClasses.class, BlackBoxCodegenTestGenerated.MultiDecl.class, BlackBoxCodegenTestGenerated.Namespace.class, BlackBoxCodegenTestGenerated.Objects.class, BlackBoxCodegenTestGenerated.OperatorConventions.class, BlackBoxCodegenTestGenerated.PrimitiveTypes.class, BlackBoxCodegenTestGenerated.Properties.class, BlackBoxCodegenTestGenerated.Reflection.class, BlackBoxCodegenTestGenerated.SafeCall.class, BlackBoxCodegenTestGenerated.SamConstructors.class, BlackBoxCodegenTestGenerated.Strings.class, BlackBoxCodegenTestGenerated.Super.class, BlackBoxCodegenTestGenerated.Traits.class, BlackBoxCodegenTestGenerated.TypeInfo.class, BlackBoxCodegenTestGenerated.Unit.class, BlackBoxCodegenTestGenerated.Vararg.class, BlackBoxCodegenTestGenerated.When.class})
+@InnerTestClasses({BlackBoxCodegenTestGenerated.Arrays.class, BlackBoxCodegenTestGenerated.Bridges.class, BlackBoxCodegenTestGenerated.CallableReference.class, BlackBoxCodegenTestGenerated.Casts.class, BlackBoxCodegenTestGenerated.Classes.class, BlackBoxCodegenTestGenerated.Closures.class, BlackBoxCodegenTestGenerated.ControlStructures.class, BlackBoxCodegenTestGenerated.DefaultArguments.class, BlackBoxCodegenTestGenerated.DelegatedProperty.class, BlackBoxCodegenTestGenerated.Elvis.class, BlackBoxCodegenTestGenerated.Enum.class, BlackBoxCodegenTestGenerated.ExclExcl.class, BlackBoxCodegenTestGenerated.ExtensionFunctions.class, BlackBoxCodegenTestGenerated.ExtensionProperties.class, BlackBoxCodegenTestGenerated.FieldRename.class, BlackBoxCodegenTestGenerated.Finally.class, BlackBoxCodegenTestGenerated.Functions.class, BlackBoxCodegenTestGenerated.InnerNested.class, BlackBoxCodegenTestGenerated.Instructions.class, BlackBoxCodegenTestGenerated.Intrinsics.class, BlackBoxCodegenTestGenerated.Labels.class, BlackBoxCodegenTestGenerated.LocalClasses.class, BlackBoxCodegenTestGenerated.MultiDecl.class, BlackBoxCodegenTestGenerated.Namespace.class, BlackBoxCodegenTestGenerated.Objects.class, BlackBoxCodegenTestGenerated.OperatorConventions.class, BlackBoxCodegenTestGenerated.PrimitiveTypes.class, BlackBoxCodegenTestGenerated.Properties.class, BlackBoxCodegenTestGenerated.Reflection.class, BlackBoxCodegenTestGenerated.SafeCall.class, BlackBoxCodegenTestGenerated.SamConstructors.class, BlackBoxCodegenTestGenerated.Strings.class, BlackBoxCodegenTestGenerated.Super.class, BlackBoxCodegenTestGenerated.Traits.class, BlackBoxCodegenTestGenerated.TypeInfo.class, BlackBoxCodegenTestGenerated.Unit.class, BlackBoxCodegenTestGenerated.Vararg.class, BlackBoxCodegenTestGenerated.When.class})
 public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
    public void testAllFilesPresentInBox() throws Exception {
        JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("compiler/testData/codegen/box"), Pattern.compile("^(.+)\\.kt$"), true);
@@ -2096,6 +2096,34 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
        
    }
    
+    @TestMetadata("compiler/testData/codegen/box/finally")
+    public static class Finally extends AbstractBlackBoxCodegenTest {
+        public void testAllFilesPresentInFinally() throws Exception {
+            JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("compiler/testData/codegen/box/finally"), Pattern.compile("^(.+)\\.kt$"), true);
+        }
+        
+        @TestMetadata("kt3549.kt")
+        public void testKt3549() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/kt3549.kt");
+        }
+        
+        @TestMetadata("kt3867.kt")
+        public void testKt3867() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/kt3867.kt");
+        }
+        
+        @TestMetadata("notChainCatch.kt")
+        public void testNotChainCatch() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/notChainCatch.kt");
+        }
+        
+        @TestMetadata("tryFinally.kt")
+        public void testTryFinally() throws Exception {
+            doTest("compiler/testData/codegen/box/finally/tryFinally.kt");
+        }
+        
+    }
+    
    @TestMetadata("compiler/testData/codegen/box/functions")
    @InnerTestClasses({Functions.Invoke.class, Functions.LocalFunctions.class})
    public static class Functions extends AbstractBlackBoxCodegenTest {
@@ -4211,6 +4239,7 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
        suite.addTestSuite(ExtensionFunctions.class);
        suite.addTestSuite(ExtensionProperties.class);
        suite.addTestSuite(FieldRename.class);
+        suite.addTestSuite(Finally.class);
        suite.addTest(Functions.innerSuite());
        suite.addTestSuite(InnerNested.class);
        suite.addTest(Instructions.innerSuite());