Updated exception table generation: try block return moved to uncatched interval

#KT-8148 Fixed

compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java

@@ -1197,16 +1197,20 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
 
     @Override
     public StackValue visitBreakExpression(@NotNull JetBreakExpression expression, StackValue receiver) {
-        return generateBreakOrContinueExpression(expression, true);
+        return generateBreakOrContinueExpression(expression, true, new Label());
     }
 
     @Override
     public StackValue visitContinueExpression(@NotNull JetContinueExpression expression, StackValue receiver) {
-        return generateBreakOrContinueExpression(expression, false);
+        return generateBreakOrContinueExpression(expression, false, new Label());
     }
 
     @NotNull
-    private StackValue generateBreakOrContinueExpression(@NotNull JetExpressionWithLabel expression, boolean isBreak) {
+    private StackValue generateBreakOrContinueExpression(
+            @NotNull JetExpressionWithLabel expression,
+            boolean isBreak,
+            @NotNull Label afterBreakContinueLabel
+    ) {
         assert expression instanceof JetContinueExpression || expression instanceof JetBreakExpression;
 
         if (!blockStackElements.isEmpty()) {
@@ -1215,7 +1219,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
             if (stackElement instanceof FinallyBlockStackElement) {
                 FinallyBlockStackElement finallyBlockStackElement = (FinallyBlockStackElement) stackElement;
                 //noinspection ConstantConditions
-                genFinallyBlockOrGoto(finallyBlockStackElement, null);
+                genFinallyBlockOrGoto(finallyBlockStackElement, null, afterBreakContinueLabel);
             }
             else if (stackElement instanceof LoopBlockStackElement) {
                 LoopBlockStackElement loopBlockStackElement = (LoopBlockStackElement) stackElement;
@@ -1225,6 +1229,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
                     loopBlockStackElement.targetLabel != null &&
                     labelElement.getReferencedName().equals(loopBlockStackElement.targetLabel.getReferencedName())) {
                     v.goTo(isBreak ? loopBlockStackElement.breakLabel : loopBlockStackElement.continueLabel);
+                    v.mark(afterBreakContinueLabel);
                     return StackValue.none();
                 }
             }
@@ -1233,7 +1238,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
             }
 
             blockStackElements.pop();
-            StackValue result = generateBreakOrContinueExpression(expression, isBreak);
+            StackValue result = generateBreakOrContinueExpression(expression, isBreak, afterBreakContinueLabel);
             blockStackElements.push(stackElement);
             return result;
         }
@@ -1754,12 +1759,12 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
         }
     }
 
-    private void doFinallyOnReturn() {
+    private void doFinallyOnReturn(Label afterReturnLabel) {
         if(!blockStackElements.isEmpty()) {
             BlockStackElement stackElement = blockStackElements.peek();
             if (stackElement instanceof FinallyBlockStackElement) {
                 FinallyBlockStackElement finallyBlockStackElement = (FinallyBlockStackElement) stackElement;
-                genFinallyBlockOrGoto(finallyBlockStackElement, null);
+                genFinallyBlockOrGoto(finallyBlockStackElement, null, afterReturnLabel);
             }
             else if (stackElement instanceof LoopBlockStackElement) {
 
@@ -1768,7 +1773,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
             }
 
             blockStackElements.pop();
-            doFinallyOnReturn();
+            doFinallyOnReturn(afterReturnLabel);
             blockStackElements.push(stackElement);
         }
     }
@@ -1784,7 +1789,8 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
 
     private void genFinallyBlockOrGoto(
             @Nullable FinallyBlockStackElement finallyBlockStackElement,
-            @Nullable Label tryCatchBlockEnd
+            @Nullable Label tryCatchBlockEnd,
+            @Nullable Label afterReturnLabel
     ) {
         if (finallyBlockStackElement != null) {
             assert finallyBlockStackElement.gaps.size() % 2 == 0 : "Finally block gaps are inconsistent";
@@ -1809,8 +1815,10 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
         }
 
         if (finallyBlockStackElement != null) {
-            Label finallyEnd = new Label();
+            Label finallyEnd = afterReturnLabel != null ? afterReturnLabel : new Label();
-            v.mark(finallyEnd);
+            if (afterReturnLabel == null) {
+                v.mark(finallyEnd);
+            }
             finallyBlockStackElement.addGapLabel(finallyEnd);
 
             blockStackElements.push(finallyBlockStackElement);
@@ -1835,28 +1843,30 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
             gen(returnedExpression, returnType);
         }
 
-        generateFinallyBlocksIfNeeded(returnType);
+        Label afterReturnLabel = new Label();
+        generateFinallyBlocksIfNeeded(returnType, afterReturnLabel);
 
         if (isNonLocalReturn) {
             InlineCodegenUtil.generateGlobalReturnFlag(v, nonLocalReturn.labelName);
         }
         v.visitInsn(returnType.getOpcode(Opcodes.IRETURN));
+        v.mark(afterReturnLabel);
 
         return StackValue.none();
     }
 
-    public void generateFinallyBlocksIfNeeded(Type returnType) {
+    public void generateFinallyBlocksIfNeeded(Type returnType, @NotNull Label afterReturnLabel) {
         if (hasFinallyBlocks()) {
             if (!Type.VOID_TYPE.equals(returnType)) {
                 int returnValIndex = myFrameMap.enterTemp(returnType);
                 StackValue.Local localForReturnValue = StackValue.local(returnValIndex, returnType);
                 localForReturnValue.store(StackValue.onStack(returnType), v);
-                doFinallyOnReturn();
+                doFinallyOnReturn(afterReturnLabel);
                 localForReturnValue.put(returnType, v);
                 myFrameMap.leaveTemp(returnType);
             }
             else {
-                doFinallyOnReturn();
+                doFinallyOnReturn(afterReturnLabel);
             }
         }
     }
@@ -3571,7 +3581,7 @@ The "returned" value of try expression with no finally is either the last expres
 
                 Label end = new Label();
 
-                genFinallyBlockOrGoto(finallyBlockStackElement, end);
+                genFinallyBlockOrGoto(finallyBlockStackElement, end, null);
 
                 List<JetCatchClause> clauses = expression.getCatchClauses();
                 for (int i = 0, size = clauses.size(); i < size; i++) {
@@ -3601,7 +3611,7 @@ The "returned" value of try expression with no finally is either the last expres
                     v.visitLocalVariable(descriptor.getName().asString(), descriptorType.getDescriptor(), null, clauseStart, clauseEnd,
                                          index);
 
-                    genFinallyBlockOrGoto(finallyBlockStackElement, i != size - 1 || finallyBlock != null ? end : null);
+                    genFinallyBlockOrGoto(finallyBlockStackElement, i != size - 1 || finallyBlock != null ? end : null, null);
 
                     generateExceptionTable(clauseStart, tryBlockRegions, descriptorType.getInternalName());
                 }
@@ -3621,7 +3631,7 @@ The "returned" value of try expression with no finally is either the last expres
                     List<Label> defaultCatchRegions = getCurrentCatchIntervals(finallyBlockStackElement, tryStart, defaultCatchEnd);
 
 
-                    genFinallyBlockOrGoto(finallyBlockStackElement, null);
+                    genFinallyBlockOrGoto(finallyBlockStackElement, null, null);
 
                     v.load(savedException, JAVA_THROWABLE_TYPE);
                     myFrameMap.leaveTemp(JAVA_THROWABLE_TYPE);

compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.java

@@ -668,7 +668,7 @@ public class InlineCodegen extends CallGenerator {
             MethodInliner.PointForExternalFinallyBlocks extension = extensionPoints.get(curInstr);
             if (extension != null) {
                 Label start = new Label();
-                Label end = new Label();
+                //Label end = new Label();
 
                 MethodNode finallyNode = InlineCodegenUtil.createEmptyMethodNode();
                 finallyNode.visitLabel(start);
@@ -684,12 +684,12 @@ public class InlineCodegen extends CallGenerator {
                     frameMap.enterTemp(Type.INT_TYPE);
                 }
 
-                finallyCodegen.generateFinallyBlocksIfNeeded(extension.returnType);
+                finallyCodegen.generateFinallyBlocksIfNeeded(extension.returnType, extension.labelNode.getLabel());
-                finallyNode.visitLabel(end);
+                //finallyNode.visitLabel(end);
                 //Exception table for external try/catch/finally blocks will be generated in original codegen after exiting this method
                 InlineCodegenUtil.insertNodeBefore(finallyNode, intoNode, curInstr);
 
-                SimpleInterval splitBy = new SimpleInterval((LabelNode) start.info, (LabelNode) end.info);
+                SimpleInterval splitBy = new SimpleInterval((LabelNode) start.info, extension.labelNode);
                 processor.getTryBlocksMetaInfo().splitCurrentIntervals(splitBy, false);
 
                 processor.getLocalVarsMetaInfo().splitCurrentIntervals(splitBy, false);

compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java

@@ -699,8 +699,10 @@ public class MethodInliner {
                 }
 
                 //genetate finally block before nonLocalReturn flag/return/goto
-                result.add(new PointForExternalFinallyBlocks(isLocalReturn ? insnNode : insnNode.getPrevious(), getReturnType(insnNode.getOpcode())
+                LabelNode label = new LabelNode();
-                ));
+                instructions.insert(insnNode, label);
+                result.add(new PointForExternalFinallyBlocks(isLocalReturn ? insnNode : insnNode.getPrevious(), getReturnType(insnNode.getOpcode()),
+                                                             label));
             }
             insnNode = insnNode.getNext();
         }
@@ -714,9 +716,12 @@ public class MethodInliner {
 
         final Type returnType;
 
-        public PointForExternalFinallyBlocks(AbstractInsnNode beforeIns, Type returnType) {
+        final LabelNode labelNode;
+
+        public PointForExternalFinallyBlocks(@NotNull AbstractInsnNode beforeIns, @NotNull Type returnType, @NotNull LabelNode labelNode) {
             this.beforeIns = beforeIns;
             this.returnType = returnType;
+            this.labelNode = labelNode;
         }
 
     }

compiler/testData/codegen/box/controlStructures/kt8148.kt

@@ -0,0 +1,29 @@
+class A(var value: String)
+
+fun box(): String {
+    val a = A("start")
+
+    try {
+        test(a)
+    } catch(e: java.lang.RuntimeException) {
+
+    }
+
+    if (a.value != "start, try, finally1, finally2") return "fail: ${a.value}"
+
+    return "OK"
+}
+
+fun test(a: A) : String {
+    try {
+        try {
+            a.value += ", try"
+            return a.value
+        } finally {
+            a.value += ", finally1"
+        }
+    } finally {
+        a.value += ", finally2"
+        throw RuntimeException("fail")
+    }
+}

compiler/testData/codegen/box/controlStructures/kt8148_break.kt

@@ -0,0 +1,33 @@
+class A(var value: String)
+
+fun box(): String {
+    val a = A("start")
+
+    try {
+        test(a)
+    } catch(e: java.lang.RuntimeException) {
+
+    }
+
+    if (a.value != "start, try, finally1, finally2") return "fail: ${a.value}"
+
+    return "OK"
+}
+
+fun test(a: A) : String {
+    while (true) {
+        try {
+            try {
+                a.value += ", try"
+                break
+            }
+            finally {
+                a.value += ", finally1"
+            }
+        }
+        finally {
+            a.value += ", finally2"
+            throw RuntimeException("fail")
+        }
+    }
+}

compiler/testData/codegen/box/controlStructures/kt8148_continue.kt

@@ -0,0 +1,34 @@
+class A(var value: String)
+
+fun box(): String {
+    val a = A("start")
+
+    try {
+        test(a)
+    } catch(e: java.lang.RuntimeException) {
+
+    }
+
+    if (a.value != "start, try, finally1, finally2") return "fail: ${a.value}"
+
+    return "OK"
+}
+
+fun test(a: A) : String {
+    while (a.value == "start") {
+        try {
+            try {
+                a.value += ", try"
+                continue
+            }
+            finally {
+                a.value += ", finally1"
+            }
+        }
+        finally {
+            a.value += ", finally2"
+            throw RuntimeException("fail")
+        }
+    }
+    return "fail"
+}

compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java

@@ -2143,6 +2143,24 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest(fileName);
         }
 
+        @TestMetadata("kt8148.kt")
+        public void testKt8148() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt8148.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("kt8148_break.kt")
+        public void testKt8148_break() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt8148_break.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("kt8148_continue.kt")
+        public void testKt8148_continue() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt8148_continue.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("kt870.kt")
         public void testKt870() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt870.kt");