From 8809abdbacca472e684071f45de6308afc38c93e Mon Sep 17 00:00:00 2001 From: pyos Date: Thu, 7 May 2020 14:48:07 +0200 Subject: [PATCH] JVM_IR: do not handle Nothing in suspend tail call bridges Else they wouldn't be tail call, would they? --- .../ir/FirBlackBoxCodegenTestGenerated.java | 5 +++ .../KotlinNothingValueExceptionLowering.kt | 10 +++--- .../jetbrains/kotlin/backend/jvm/JvmLower.kt | 5 +-- .../backend/jvm/codegen/ExpressionCodegen.kt | 5 +-- .../box/coroutines/tailCallToNothing.kt | 34 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 +++ .../LightAnalysisModeTestGenerated.java | 5 +++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 +++ .../IrJsCodegenBoxTestGenerated.java | 5 +++ .../semantics/JsCodegenBoxTestGenerated.java | 5 +++ 10 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 compiler/testData/codegen/box/coroutines/tailCallToNothing.kt diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index e17f01c3271..1aed21bf4a3 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -6706,6 +6706,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt", "kotlin.coroutines"); } + @TestMetadata("tailCallToNothing.kt") + public void testTailCallToNothing() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallToNothing.kt"); + } + @TestMetadata("tryCatchFinallyWithHandleResult.kt") public void testTryCatchFinallyWithHandleResult_1_3() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/tryCatchFinallyWithHandleResult.kt", "kotlin.coroutines"); diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/KotlinNothingValueExceptionLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/KotlinNothingValueExceptionLowering.kt index 2d040c8403e..a047b4982c5 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/KotlinNothingValueExceptionLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/KotlinNothingValueExceptionLowering.kt @@ -19,11 +19,13 @@ import org.jetbrains.kotlin.ir.types.isNothing import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid -class KotlinNothingValueExceptionLowering(val backendContext: CommonBackendContext) : BodyLoweringPass { +class KotlinNothingValueExceptionLowering( + val backendContext: CommonBackendContext, val skip: (IrDeclaration) -> Boolean = { false } +) : BodyLoweringPass { override fun lower(irBody: IrBody, container: IrDeclaration) { - irBody.transformChildrenVoid( - Transformer((container as IrSymbolDeclaration<*>).symbol) - ) + if (!skip(container)) { + irBody.transformChildrenVoid(Transformer((container as IrSymbolDeclaration<*>).symbol)) + } } private inner class Transformer(val parent: IrSymbol) : IrElementTransformerVoid() { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt index c14873d48f7..c6e615b909d 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt @@ -11,6 +11,7 @@ import org.jetbrains.kotlin.backend.common.lower.* import org.jetbrains.kotlin.backend.common.lower.loops.forLoopsPhase import org.jetbrains.kotlin.backend.common.lower.optimizations.foldConstantLoweringPhase import org.jetbrains.kotlin.backend.common.phaser.* +import org.jetbrains.kotlin.backend.jvm.codegen.shouldContainSuspendMarkers import org.jetbrains.kotlin.backend.jvm.lower.* import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.descriptors.Visibility @@ -262,8 +263,8 @@ private val tailrecPhase = makeIrFilePhase( description = "Handle tailrec calls" ) -private val kotlinNothingValueExceptionPhase = makeIrFilePhase( - ::KotlinNothingValueExceptionLowering, +private val kotlinNothingValueExceptionPhase = makeIrFilePhase( + { context -> KotlinNothingValueExceptionLowering(context) { it is IrFunction && !it.shouldContainSuspendMarkers() } }, name = "KotlinNothingValueException", description = "Throw proper exception for calls returning value of type 'kotlin.Nothing'" ) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt index e6e2777c9a9..de051cc8e7a 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt @@ -449,12 +449,9 @@ class ExpressionCodegen( } return when { - expression.type.isNothing() -> { - unitValue - } expression is IrConstructorCall -> MaterialValue(this, asmType, expression.type) - expression.type.isUnit() && irFunction.shouldContainSuspendMarkers() -> { + (expression.type.isNothing() || expression.type.isUnit()) && irFunction.shouldContainSuspendMarkers() -> { // NewInference allows casting `() -> T` to `() -> Unit`. A CHECKCAST here will fail. // Also, if the callee is a suspend function with a suspending tail call, the next `resumeWith` // will continue from here, but the value passed to it might not have been `Unit`. An exception diff --git a/compiler/testData/codegen/box/coroutines/tailCallToNothing.kt b/compiler/testData/codegen/box/coroutines/tailCallToNothing.kt new file mode 100644 index 00000000000..5a61be5e250 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/tailCallToNothing.kt @@ -0,0 +1,34 @@ +// IGNORE_BACKEND_FIR: JVM_IR +// WITH_RUNTIME +// WITH_COROUTINES +import helpers.* +import kotlin.coroutines.* +import kotlin.coroutines.intrinsics.* + +suspend fun suspendThenThrow(): Nothing { + suspendCoroutineUninterceptedOrReturn { + it.resume(Unit) + COROUTINE_SUSPENDED + } + throw RuntimeException() +} + +suspend fun foo(x: Int = 1): Nothing = suspendThenThrow() + +interface I { + suspend fun bar(): Nothing +} + +class C : I by (object : I { + override suspend fun bar(): Nothing = suspendThenThrow() +}) + +var result = "" + +fun box(): String { + suspend { + try { foo() } catch (e: RuntimeException) { result += "O" } + try { C().bar() } catch (e: RuntimeException) { result += "K" } + }.startCoroutine(EmptyContinuation) + return result +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index bb73965512e..d5afae6d5f9 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -7136,6 +7136,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt", "kotlin.coroutines"); } + @TestMetadata("tailCallToNothing.kt") + public void testTailCallToNothing() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallToNothing.kt"); + } + @TestMetadata("tryCatchFinallyWithHandleResult.kt") public void testTryCatchFinallyWithHandleResult_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/tryCatchFinallyWithHandleResult.kt", "kotlin.coroutines.experimental"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 8ee9039f272..79da70e2975 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -7136,6 +7136,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt", "kotlin.coroutines"); } + @TestMetadata("tailCallToNothing.kt") + public void testTailCallToNothing() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallToNothing.kt"); + } + @TestMetadata("tryCatchFinallyWithHandleResult.kt") public void testTryCatchFinallyWithHandleResult_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/tryCatchFinallyWithHandleResult.kt", "kotlin.coroutines.experimental"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 65aacfb4635..0515a82770b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -6706,6 +6706,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt", "kotlin.coroutines"); } + @TestMetadata("tailCallToNothing.kt") + public void testTailCallToNothing() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallToNothing.kt"); + } + @TestMetadata("tryCatchFinallyWithHandleResult.kt") public void testTryCatchFinallyWithHandleResult_1_3() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/tryCatchFinallyWithHandleResult.kt", "kotlin.coroutines"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index 82e193a4b88..b56514e4b10 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -5656,6 +5656,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt", "kotlin.coroutines"); } + @TestMetadata("tailCallToNothing.kt") + public void testTailCallToNothing() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallToNothing.kt"); + } + @TestMetadata("tryCatchFinallyWithHandleResult.kt") public void testTryCatchFinallyWithHandleResult_1_3() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/tryCatchFinallyWithHandleResult.kt", "kotlin.coroutines"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 61be49410c4..cb28757f05f 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -5656,6 +5656,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt", "kotlin.coroutines"); } + @TestMetadata("tailCallToNothing.kt") + public void testTailCallToNothing() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallToNothing.kt"); + } + @TestMetadata("tryCatchFinallyWithHandleResult.kt") public void testTryCatchFinallyWithHandleResult_1_3() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/tryCatchFinallyWithHandleResult.kt", "kotlin.coroutines");