From 800cc633478f68cde97d1ac24df3c4dfd6a711ec Mon Sep 17 00:00:00 2001 From: Mikhael Bogdanov Date: Fri, 3 Nov 2017 14:08:10 +0100 Subject: [PATCH] Remove external finally block interval on splitting during inline #KT-20433 Fixed --- .../inline/CoveringTryCatchNodeProcessor.kt | 13 +++-- .../codegen/inline/SourceCompilerForInline.kt | 2 +- .../codegen/inline/TryBlockClustering.kt | 10 ++++ .../nonLocalReturns/tryFinally/kt20433.kt | 47 ++++++++++++++++++ .../nonLocalReturns/tryFinally/kt20433_2.kt | 47 ++++++++++++++++++ .../tryFinally/kt20433_2_void.kt | 47 ++++++++++++++++++ .../tryFinally/kt20433_void.kt | 48 +++++++++++++++++++ .../IrBlackBoxInlineCodegenTestGenerated.java | 24 ++++++++++ ...otlinAgainstInlineKotlinTestGenerated.java | 24 ++++++++++ .../BlackBoxInlineCodegenTestGenerated.java | 24 ++++++++++ ...otlinAgainstInlineKotlinTestGenerated.java | 24 ++++++++++ .../NonLocalReturnsTestGenerated.java | 24 ++++++++++ 12 files changed, 330 insertions(+), 4 deletions(-) create mode 100644 compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt create mode 100644 compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt create mode 100644 compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt create mode 100644 compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt index 3b8c35147b2..2d9fa07d12d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt @@ -22,8 +22,8 @@ import org.jetbrains.org.objectweb.asm.tree.* import java.util.* abstract class CoveringTryCatchNodeProcessor(parameterSize: Int) { - val tryBlocksMetaInfo: IntervalMetaInfo = IntervalMetaInfo() - val localVarsMetaInfo: IntervalMetaInfo = IntervalMetaInfo() + val tryBlocksMetaInfo: IntervalMetaInfo = IntervalMetaInfo(this) + val localVarsMetaInfo: IntervalMetaInfo = IntervalMetaInfo(this) var nextFreeLocalIndex: Int = parameterSize private set @@ -84,24 +84,27 @@ abstract class CoveringTryCatchNodeProcessor(parameterSize: Int) { } } -class IntervalMetaInfo> { +class IntervalMetaInfo>(private val processor: CoveringTryCatchNodeProcessor) { val intervalStarts = LinkedListMultimap.create() val intervalEnds = LinkedListMultimap.create() val allIntervals: ArrayList = arrayListOf() val currentIntervals: MutableSet = linkedSetOf() fun addNewInterval(newInfo: T) { + newInfo.verify(processor) intervalStarts.put(newInfo.startLabel, newInfo) intervalEnds.put(newInfo.endLabel, newInfo) allIntervals.add(newInfo) } private fun remapStartLabel(oldStart: LabelNode, remapped: T) { + remapped.verify(processor) intervalStarts.remove(oldStart, remapped) intervalStarts.put(remapped.startLabel, remapped) } private fun remapEndLabel(oldEnd: LabelNode, remapped: T) { + remapped.verify(processor) intervalEnds.remove(oldEnd, remapped) intervalEnds.put(remapped.endLabel, remapped) } @@ -110,6 +113,10 @@ class IntervalMetaInfo> { return currentIntervals.map { split(it, by, keepStart) } } + fun splitAndRemoveCurrentIntervals(by: Interval, keepStart: Boolean) { + currentIntervals.map { splitAndRemoveInterval(it, by, keepStart) } + } + fun processCurrent(curIns: LabelNode, directOrder: Boolean) { getInterval(curIns, directOrder).forEach { val added = currentIntervals.add(it) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt index 5c199791971..a9d716858de 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt @@ -366,7 +366,7 @@ class PsiSourceCompilerForInline(private val codegen: ExpressionCodegen, overrid insertNodeBefore(finallyNode, intoNode, curInstr) val splitBy = SimpleInterval(start.info as LabelNode, extension.finallyIntervalEnd) - processor.tryBlocksMetaInfo.splitCurrentIntervals(splitBy, true) + processor.tryBlocksMetaInfo.splitAndRemoveCurrentIntervals(splitBy, true) //processor.getLocalVarsMetaInfo().splitAndRemoveIntervalsFromCurrents(splitBy); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt index 14b00cad736..ecd19139e41 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.codegen.inline +import org.jetbrains.kotlin.codegen.optimization.common.InsnSequence import org.jetbrains.org.objectweb.asm.tree.LabelNode import org.jetbrains.org.objectweb.asm.tree.TryCatchBlockNode @@ -35,6 +36,12 @@ interface Interval { /*note that some intervals are mutable */ fun isEmpty(): Boolean = startLabel == endLabel + + fun verify(processor: CoveringTryCatchNodeProcessor) { + assert (processor.instructionIndex(startLabel) <= processor.instructionIndex(endLabel)) { + "Try block body starts after body end: ${processor.instructionIndex(startLabel)} > ${processor.instructionIndex(endLabel)}" + } + } } interface SplittableInterval : Interval { @@ -77,6 +84,9 @@ class TryCatchBlockNodeInfo( } } +val TryCatchBlockNodeInfo.bodyInstuctions + get() = InsnSequence(startLabel, endLabel) + class TryCatchBlockNodePosition( val nodeInfo: TryCatchBlockNodeInfo, var position: TryCatchPosition diff --git a/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt new file mode 100644 index 00000000000..fe95b3d1f13 --- /dev/null +++ b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt @@ -0,0 +1,47 @@ +// FILE: 1.kt +//WITH_RUNTIME +package test + +public inline fun T.mylet(block: (T) -> R): R { + return block(this) +} + +// FILE: 2.kt +import test.* + +var message = "" + +fun foo(root: String): String { + try { + return root.let { _ -> + try { + if (!random()) { + return root + } + return "fail $root" + } catch (e: Exception) { + message += "${e.message} " + } + "fail" + } + } finally { + message += "Finally block" + } +} + +var exception = false + +fun random() = if (exception) error("Exception") else false + +fun box(): String { + var okResult = foo("OK") + if (okResult != "OK") return "fail 1: $okResult" + if (message != "Finally block") return "fail 2: $message" + + message = "" + exception = true + if (foo("OK") != "fail") return "fail 3: ${foo("OK")}" + if (message != "Exception Finally block" ) return "fail 4: $message" + + return okResult +} diff --git a/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt new file mode 100644 index 00000000000..1214eb64235 --- /dev/null +++ b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt @@ -0,0 +1,47 @@ +// FILE: 1.kt +//WITH_RUNTIME +package test + +public inline fun T.mylet(block: (T) -> R): R { + return block(this) +} + +// FILE: 2.kt +import test.* + +var message = "" + +fun foo(root: String): String { + try { + return root.let { _ -> + try { + if (!random()) { + return root + } + return "fail $root" + } catch (e: Exception) { + message += "Exception $e" + } + "fail" + } + } finally { + message += "Finally block" + } +} + +var fail = false + +fun random() = fail + +fun box(): String { + var okResult = foo("OK") + if (okResult != "OK") return "fail 1: $okResult" + if (message != "Finally block") return "fail 2: $message" + + message = "" + fail = true + if (foo("OK") != "fail OK") return "fail 3: ${foo("OK")}" + if (message != "Finally block" ) return "fail 4: $message" + + return okResult +} diff --git a/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt new file mode 100644 index 00000000000..2988a95f25b --- /dev/null +++ b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt @@ -0,0 +1,47 @@ +// FILE: 1.kt +//WITH_RUNTIME +package test + +public inline fun T.mylet(block: (T) -> R): R { + return block(this) +} + +// FILE: 2.kt +import test.* + +var message = "" + +fun foo(root: String) { + try { + return root.let { _ -> + try { + if (!random()) { + message += root + return + } + message += "fail $root" + } catch (e: Exception) { + message += "Exception $e" + } + "fail" + } + } finally { + message += " Finally block" + } +} + +var fail = false + +fun random() = fail + +fun box(): String { + foo("OK") + if (message != "OK Finally block") return "fail 1: $message" + + message = "" + fail = true + foo("OK") + if (message != "fail OK Finally block" ) return "fail 2: $message" + + return "OK" +} diff --git a/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt new file mode 100644 index 00000000000..c8be6a91d48 --- /dev/null +++ b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt @@ -0,0 +1,48 @@ +// FILE: 1.kt +//WITH_RUNTIME +package test + +public inline fun T.mylet(block: (T) -> R): R { + return block(this) +} + +// FILE: 2.kt +import test.* + +var message = "" + +fun foo(root: String) { + try { + root.let { _ -> + try { + if (!random()) { + message += root + return + } + message += "fail $root" + return + } catch (e: Exception) { + message += "${e.message}" + } + "fail" + } + } finally { + message += " Finally block" + } +} + +var exception = false + +fun random() = if (exception) error("Exception") else false + +fun box(): String { + foo("OK") + if (message != "OK Finally block") return "fail 1: $message" + + message = "" + exception = true + foo("OK") + if (message != "Exception Finally block" ) return "fail 2: $message" + + return "OK" +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java index c5befc65851..35e3c2045dd 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java @@ -1876,6 +1876,30 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("kt20433.kt") + public void testKt20433() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2.kt") + public void testKt20433_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2_void.kt") + public void testKt20433_2_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_void.kt") + public void testKt20433_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt"); + doTest(fileName); + } + @TestMetadata("kt6956.kt") public void testKt6956() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt6956.kt"); diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java index 69d5d6182f7..ecbed87601d 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java @@ -1876,6 +1876,30 @@ public class IrCompileKotlinAgainstInlineKotlinTestGenerated extends AbstractIrC KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("kt20433.kt") + public void testKt20433() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2.kt") + public void testKt20433_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2_void.kt") + public void testKt20433_2_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_void.kt") + public void testKt20433_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt"); + doTest(fileName); + } + @TestMetadata("kt6956.kt") public void testKt6956() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt6956.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java index f1352b73f2c..e8ee9f3a891 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -1876,6 +1876,30 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("kt20433.kt") + public void testKt20433() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2.kt") + public void testKt20433_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2_void.kt") + public void testKt20433_2_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_void.kt") + public void testKt20433_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt"); + doTest(fileName); + } + @TestMetadata("kt6956.kt") public void testKt6956() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt6956.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index 342ee1b8040..6abfcb97863 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -1876,6 +1876,30 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("kt20433.kt") + public void testKt20433() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2.kt") + public void testKt20433_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2_void.kt") + public void testKt20433_2_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_void.kt") + public void testKt20433_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt"); + doTest(fileName); + } + @TestMetadata("kt6956.kt") public void testKt6956() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt6956.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NonLocalReturnsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NonLocalReturnsTestGenerated.java index c9b6356821d..e8e93868b82 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NonLocalReturnsTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NonLocalReturnsTestGenerated.java @@ -143,6 +143,30 @@ public class NonLocalReturnsTestGenerated extends AbstractNonLocalReturnsTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true); } + @TestMetadata("kt20433.kt") + public void testKt20433() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2.kt") + public void testKt20433_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_2_void.kt") + public void testKt20433_2_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_2_void.kt"); + doTest(fileName); + } + + @TestMetadata("kt20433_void.kt") + public void testKt20433_void() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt20433_void.kt"); + doTest(fileName); + } + @TestMetadata("kt6956.kt") public void testKt6956() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/kt6956.kt");