From 7bc521ab928d5a9d03b0f1844d4e6d9741831f65 Mon Sep 17 00:00:00 2001 From: Artem Kobzar Date: Tue, 12 Sep 2023 15:18:16 +0000 Subject: [PATCH] [K/JS] Reset exceptionState inside coroutines with a finally block ^KT-58685 Fixed --- .../common/lower/FinallyBlocksLowering.kt | 10 +++- .../lower/coroutines/StateMachineBuilder.kt | 9 +++- .../testData/codegen/box/classes/kt496.kt | 3 -- .../js/test/fir/FirJsBoxTestGenerated.java | 6 +++ .../js/test/fir/FirJsES6BoxTestGenerated.java | 6 +++ .../js/test/ir/IrBoxJsES6TestGenerated.java | 6 +++ .../js/test/ir/IrBoxJsTestGenerated.java | 6 +++ .../testData/box/coroutines/tryFinally.kt | 46 +++++++++++++++++++ 8 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 js/js.translator/testData/box/coroutines/tryFinally.kt diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FinallyBlocksLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FinallyBlocksLowering.kt index 11fdf90049a..7409e462ad3 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FinallyBlocksLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FinallyBlocksLowering.kt @@ -27,6 +27,8 @@ import org.jetbrains.kotlin.ir.util.setDeclarationsParent import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid import org.jetbrains.kotlin.name.Name +object FINALLY_EXPRESSION : IrStatementOriginImpl("FINALLY_EXPRESSION") + class FinallyBlocksLowering(val context: CommonBackendContext, private val throwableType: IrType): FileLoweringPass, IrElementTransformerVoidWithContext() { private interface HighLevelJump { @@ -271,13 +273,17 @@ class FinallyBlocksLowering(val context: CommonBackendContext, private val throw +irReturnableBlock(symbol, type) { +value } - +copy(finallyExpression) + +irComposite(resultType = context.irBuiltIns.unitType, origin = FINALLY_EXPRESSION) { + +copy(finallyExpression) + } } else -> irBlock(value, null, type) { val tmp = createTmpVariable(irReturnableBlock(symbol, type) { +irReturn(symbol, value) }) - +copy(finallyExpression) + +irComposite(resultType = context.irBuiltIns.unitType, origin = FINALLY_EXPRESSION) { + +copy(finallyExpression) + } +irGet(tmp) } } diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/coroutines/StateMachineBuilder.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/coroutines/StateMachineBuilder.kt index 1e559bc4e1c..037e2ddbf34 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/coroutines/StateMachineBuilder.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/coroutines/StateMachineBuilder.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.ir.backend.js.lower.coroutines import org.jetbrains.kotlin.backend.common.ir.isPure +import org.jetbrains.kotlin.backend.common.lower.FINALLY_EXPRESSION import org.jetbrains.kotlin.backend.common.peek import org.jetbrains.kotlin.backend.common.pop import org.jetbrains.kotlin.backend.common.push @@ -396,6 +397,13 @@ class StateMachineBuilder( private fun wrap(expression: IrExpression, variable: IrVariableSymbol) = JsIrBuilder.buildSetVariable(variable, expression, unit) + override fun visitComposite(expression: IrComposite) { + if (expression.origin == FINALLY_EXPRESSION) { + catchBlockStack.peek()?.let(::setupExceptionState) + } + super.visitComposite(expression) + } + override fun visitWhen(expression: IrWhen) { if (expression !in suspendableNodes) return addStatement(expression) @@ -709,7 +717,6 @@ class StateMachineBuilder( catchBlockStack.pop() updateState(tryState.catchState) - setupExceptionState(enclosingCatch) var rethrowNeeded = true diff --git a/compiler/testData/codegen/box/classes/kt496.kt b/compiler/testData/codegen/box/classes/kt496.kt index 84f12258dae..e4c31c86a27 100644 --- a/compiler/testData/codegen/box/classes/kt496.kt +++ b/compiler/testData/codegen/box/classes/kt496.kt @@ -1,6 +1,3 @@ -// IGNORE_BACKEND_K2: WASM -// ^ KT-59800 K2 + Wasm: test failure after changing `finally` block generation - fun test1() : Boolean { try { return true diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java index 9b318abab28..2ce218326af 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java @@ -961,6 +961,12 @@ public class FirJsBoxTestGenerated extends AbstractFirJsBoxTest { public void testSuspendMethodWithSuperCall() throws Exception { runTest("js/js.translator/testData/box/coroutines/suspendMethodWithSuperCall.kt"); } + + @Test + @TestMetadata("tryFinally.kt") + public void testTryFinally() throws Exception { + runTest("js/js.translator/testData/box/coroutines/tryFinally.kt"); + } } @Nested diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsES6BoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsES6BoxTestGenerated.java index e120bddc7e6..462ff3fdfd7 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsES6BoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsES6BoxTestGenerated.java @@ -961,6 +961,12 @@ public class FirJsES6BoxTestGenerated extends AbstractFirJsES6BoxTest { public void testSuspendMethodWithSuperCall() throws Exception { runTest("js/js.translator/testData/box/coroutines/suspendMethodWithSuperCall.kt"); } + + @Test + @TestMetadata("tryFinally.kt") + public void testTryFinally() throws Exception { + runTest("js/js.translator/testData/box/coroutines/tryFinally.kt"); + } } @Nested diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java index 07c323db025..fdf13861500 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java @@ -961,6 +961,12 @@ public class IrBoxJsES6TestGenerated extends AbstractIrBoxJsES6Test { public void testSuspendMethodWithSuperCall() throws Exception { runTest("js/js.translator/testData/box/coroutines/suspendMethodWithSuperCall.kt"); } + + @Test + @TestMetadata("tryFinally.kt") + public void testTryFinally() throws Exception { + runTest("js/js.translator/testData/box/coroutines/tryFinally.kt"); + } } @Nested diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java index 7ab57f16c2c..a1ba4965903 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java @@ -961,6 +961,12 @@ public class IrBoxJsTestGenerated extends AbstractIrBoxJsTest { public void testSuspendMethodWithSuperCall() throws Exception { runTest("js/js.translator/testData/box/coroutines/suspendMethodWithSuperCall.kt"); } + + @Test + @TestMetadata("tryFinally.kt") + public void testTryFinally() throws Exception { + runTest("js/js.translator/testData/box/coroutines/tryFinally.kt"); + } } @Nested diff --git a/js/js.translator/testData/box/coroutines/tryFinally.kt b/js/js.translator/testData/box/coroutines/tryFinally.kt new file mode 100644 index 00000000000..8f578fed0d1 --- /dev/null +++ b/js/js.translator/testData/box/coroutines/tryFinally.kt @@ -0,0 +1,46 @@ +import kotlin.coroutines.* +import kotlin.coroutines.intrinsics.* + +var isLocked = false + +fun builder(c: suspend () -> Unit) { + c.startCoroutine(object : Continuation { + override val context = EmptyCoroutineContext + override fun resumeWith(result: Result) {} + }) +} + +private suspend fun lock() { + isLocked = true +} + +private fun unlock() { + if (isLocked) { + isLocked = false + } else { + throw IllegalStateException("Not locked") + } +} + +private suspend inline fun withLock(block: () -> T): T { + lock() + try { + return block() + } finally { + unlock() + } +} + +fun box(): String { + var error: Exception? = null + builder { + try { + withLock {} + throw Exception("seems fine") + } catch (e: Exception) { + error = e + } + } + + return if (error !is IllegalStateException) "OK" else "fail" +} \ No newline at end of file