From 50797dba8d65170fb6e150b9c6e420c6a4128d12 Mon Sep 17 00:00:00 2001 From: pyos Date: Tue, 6 Jul 2021 10:57:48 +0200 Subject: [PATCH] JVM: do not use `crossinline` flag when inlining assertions Crossinline lambdas *can* be inlined into objects, but don't *have* to; the correct place should be determined from the context, not from the parameter. --- .../kotlin/codegen/ExpressionCodegen.java | 2 +- .../kotlin/codegen/inline/LambdaInfo.kt | 8 +- .../kotlin/codegen/inline/MethodInliner.kt | 26 ++-- .../kotlin/codegen/inline/PsiInlineCodegen.kt | 8 +- ...FirBlackBoxInlineCodegenTestGenerated.java | 6 + .../backend/jvm/codegen/IrInlineCodegen.kt | 4 +- .../assert/jvmCrossinlineRedundant.kt | 128 ++++++++++++++++++ .../BlackBoxInlineCodegenTestGenerated.java | 6 + ...otlinAgainstInlineKotlinTestGenerated.java | 6 + .../IrBlackBoxInlineCodegenTestGenerated.java | 6 + ...otlinAgainstInlineKotlinTestGenerated.java | 6 + ...JvmIrAgainstOldBoxInlineTestGenerated.java | 6 + ...JvmOldAgainstIrBoxInlineTestGenerated.java | 6 + 13 files changed, 191 insertions(+), 27 deletions(-) create mode 100644 compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 4c117c29385..47fface0da8 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -1264,7 +1264,7 @@ public class ExpressionCodegen extends KtVisitor impleme // Thus, do not generate it. Otherwise, it leads to VerifyError on run-time. boolean isCrossinlineLambda = (callGenerator instanceof PsiInlineCodegen) && Objects.requireNonNull(((PsiInlineCodegen) callGenerator).getActiveLambda(), - "no active lambda found").isCrossInline; + "no active lambda found").isCrossInline(); if (!isCrossinlineLambda) { v.aconst(null); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt index a81441ccc78..e2150eb45ef 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt @@ -16,8 +16,7 @@ import org.jetbrains.org.objectweb.asm.tree.FieldInsnNode interface FunctionalArgument -abstract class LambdaInfo(@JvmField val isCrossInline: Boolean) : FunctionalArgument { - +abstract class LambdaInfo : FunctionalArgument { abstract val isBoundCallableReference: Boolean abstract val isSuspend: Boolean @@ -66,7 +65,7 @@ abstract class LambdaInfo(@JvmField val isCrossInline: Boolean) : FunctionalArgu object NonInlineableArgumentForInlineableParameterCalledInSuspend : FunctionalArgument object NonInlineableArgumentForInlineableSuspendParameter : FunctionalArgument -abstract class ExpressionLambda(isCrossInline: Boolean) : LambdaInfo(isCrossInline) { +abstract class ExpressionLambda : LambdaInfo() { fun generateLambdaBody(sourceCompiler: SourceCompilerForInline) { node = sourceCompiler.generateLambdaBody(this, reifiedTypeParametersUsages) node.node.preprocessSuspendMarkers(forInline = true, keepFakeContinuation = false) @@ -76,11 +75,10 @@ abstract class ExpressionLambda(isCrossInline: Boolean) : LambdaInfo(isCrossInli abstract class DefaultLambda( final override val lambdaClassType: Type, capturedArgs: Array, - isCrossinline: Boolean, val offset: Int, val needReification: Boolean, sourceCompiler: SourceCompilerForInline -) : LambdaInfo(isCrossinline) { +) : LambdaInfo() { final override val isSuspend get() = false // TODO: it should probably be true sometimes, but it never was final override val isBoundCallableReference: Boolean diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt index 8ff78548c02..ee13aff47e0 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt @@ -558,21 +558,17 @@ class MethodInliner( ) } else if (fieldInsnNode.isCheckAssertionsStatus()) { fieldInsnNode.owner = inlineCallSiteInfo.ownerClassName - if (inliningContext.isInliningLambda) { - if (inliningContext.lambdaInfo!!.isCrossInline) { - assert(inliningContext.parent?.parent is RegeneratedClassContext) { - "$inliningContext grandparent shall be RegeneratedClassContext but got ${inliningContext.parent?.parent}" - } - inliningContext.parent!!.parent!!.generateAssertField = true - } else { - assert(inliningContext.parent != null) { - "$inliningContext parent shall not be null" - } - inliningContext.parent!!.generateAssertField = true - } - } else { - inliningContext.generateAssertField = true - } + when { + // In inline function itself: + inliningContext.parent == null -> inliningContext + // In method of regenerated object - field should already exist: + inliningContext.parent is RegeneratedClassContext -> inliningContext.parent + // In lambda inlined into the root function: + inliningContext.parent.parent == null -> inliningContext.parent + // In lambda inlined into a method of a regenerated object: + else -> inliningContext.parent.parent as? RegeneratedClassContext + ?: throw AssertionError("couldn't find class for \$assertionsDisabled (context = $inliningContext)") + }.generateAssertField = true } } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/PsiInlineCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/PsiInlineCodegen.kt index 3646f4d87ea..13882029da0 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/PsiInlineCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/PsiInlineCodegen.kt @@ -181,7 +181,7 @@ class PsiInlineCodegen( } } - var activeLambda: LambdaInfo? = null + var activeLambda: PsiExpressionLambda? = null private set private fun putClosureParametersOnStack(next: PsiExpressionLambda, receiverValue: StackValue?) { @@ -210,9 +210,9 @@ private val FunctionDescriptor.explicitParameters class PsiExpressionLambda( expression: KtExpression, private val state: GenerationState, - isCrossInline: Boolean, + val isCrossInline: Boolean, override val isBoundCallableReference: Boolean -) : ExpressionLambda(isCrossInline) { +) : ExpressionLambda() { override val lambdaClassType: Type override val invokeMethod: Method @@ -308,7 +308,7 @@ class PsiDefaultLambda( offset: Int, needReification: Boolean, sourceCompiler: SourceCompilerForInline -) : DefaultLambda(lambdaClassType, capturedArgs, parameterDescriptor.isCrossinline, offset, needReification, sourceCompiler) { +) : DefaultLambda(lambdaClassType, capturedArgs, offset, needReification, sourceCompiler) { private val invokeMethodDescriptor: FunctionDescriptor override val invokeMethodParameters: List diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxInlineCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxInlineCodegenTestGenerated.java index f5a3cac843a..c63524e4df7 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxInlineCodegenTestGenerated.java @@ -882,6 +882,12 @@ public class FirBlackBoxInlineCodegenTestGenerated extends AbstractFirBlackBoxIn runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineLambdaDeclarationSiteOnly.kt"); } + @Test + @TestMetadata("jvmCrossinlineRedundant.kt") + public void testJvmCrossinlineRedundant() throws Exception { + runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt"); + } + @Test @TestMetadata("jvmCrossinlineSAMDeclarationSite.kt") public void testJvmCrossinlineSAMDeclarationSite() throws Exception { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt index 7f24017d2a9..75ed99ba084 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt @@ -133,7 +133,7 @@ class IrExpressionLambdaImpl( codegen: ExpressionCodegen, val reference: IrFunctionReference, irValueParameter: IrValueParameter -) : ExpressionLambda(irValueParameter.isCrossinline), IrExpressionLambda { +) : ExpressionLambda(), IrExpressionLambda { override val isExtensionLambda: Boolean = irValueParameter.type.isExtensionFunctionType val function: IrFunction @@ -194,7 +194,7 @@ class IrDefaultLambda( offset: Int, needReification: Boolean, sourceCompiler: IrSourceCompilerForInline -) : DefaultLambda(lambdaClassType, capturedArgs, irValueParameter.isCrossinline, offset, needReification, sourceCompiler) { +) : DefaultLambda(lambdaClassType, capturedArgs, offset, needReification, sourceCompiler) { private val typeArguments: MutableList diff --git a/compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt b/compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt new file mode 100644 index 00000000000..bf8813408a4 --- /dev/null +++ b/compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt @@ -0,0 +1,128 @@ +// NO_CHECK_LAMBDA_INLINING +// WITH_RUNTIME +// TARGET_BACKEND: JVM +// ASSERTIONS_MODE: jvm +// FILE: inline.kt + +package test + +inline fun call(crossinline c: () -> Unit) { + c() +} + +// FILE: inlineSite.kt + +import test.* + +interface Checker { + fun checkTrue(): Boolean + fun checkFalse(): Boolean + fun checkTrueWithMessage(): Boolean + fun checkFalseWithMessage(): Boolean +} + +class ShouldBeDisabled : Checker { + override fun checkTrue(): Boolean { + var hit = false + val l = { hit = true; true } + call { + assert(l()) + } + return hit + } + + override fun checkFalse(): Boolean { + var hit = false + val l = { hit = true; false } + call { + assert(l()) + } + return hit + } + + override fun checkTrueWithMessage(): Boolean { + var hit = false + val l = { hit = true; true } + call { + assert(l()) { "BOOYA" } + } + return hit + } + + override fun checkFalseWithMessage(): Boolean { + var hit = false + val l = { hit = true; false } + call { + assert(l()) { "BOOYA" } + } + return hit + } +} + +class ShouldBeEnabled : Checker { + override fun checkTrue(): Boolean { + var hit = false + val l = { hit = true; true } + call { + assert(l()) + } + return hit + } + + override fun checkFalse(): Boolean { + var hit = false + val l = { hit = true; false } + call { + assert(l()) + } + return hit + } + + override fun checkTrueWithMessage(): Boolean { + var hit = false + val l = { hit = true; true } + call { + assert(l()) { "BOOYA" } + } + return hit + } + + override fun checkFalseWithMessage(): Boolean { + var hit = false + val l = { hit = true; false } + call { + assert(l()) { "BOOYA" } + } + return hit + } +} + +fun setDesiredAssertionStatus(v: Boolean): Checker { + val loader = Checker::class.java.classLoader + loader.setClassAssertionStatus("ShouldBeEnabled", true) + loader.setClassAssertionStatus("ShouldBeDisabled", false) + val c = loader.loadClass(if (v) "ShouldBeEnabled" else "ShouldBeDisabled") + return c.newInstance() as Checker +} + +fun box(): String { + var c = setDesiredAssertionStatus(false) + if (c.checkTrue()) return "FAIL 0" + if (c.checkTrueWithMessage()) return "FAIL 1" + if (c.checkFalse()) return "FAIL 2" + if (c.checkFalseWithMessage()) return "FAIL 3" + c = setDesiredAssertionStatus(true) + if (!c.checkTrue()) return "FAIL 4" + if (!c.checkTrueWithMessage()) return "FAIL 5" + try { + c.checkFalse() + return "FAIL 6" + } catch (ignore: AssertionError) { + } + try { + c.checkFalseWithMessage() + return "FAIL 7" + } catch (ignore: AssertionError) { + } + return "OK" +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxInlineCodegenTestGenerated.java index 8de7be176d5..bdc9a054f18 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -882,6 +882,12 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineLambdaDeclarationSiteOnly.kt"); } + @Test + @TestMetadata("jvmCrossinlineRedundant.kt") + public void testJvmCrossinlineRedundant() throws Exception { + runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt"); + } + @Test @TestMetadata("jvmCrossinlineSAMDeclarationSite.kt") public void testJvmCrossinlineSAMDeclarationSite() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index 790dcb57a31..c1f4f7fce41 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -882,6 +882,12 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineLambdaDeclarationSiteOnly.kt"); } + @Test + @TestMetadata("jvmCrossinlineRedundant.kt") + public void testJvmCrossinlineRedundant() throws Exception { + runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt"); + } + @Test @TestMetadata("jvmCrossinlineSAMDeclarationSite.kt") public void testJvmCrossinlineSAMDeclarationSite() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxInlineCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxInlineCodegenTestGenerated.java index 37c65142b57..e53208ff3d9 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxInlineCodegenTestGenerated.java @@ -882,6 +882,12 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineLambdaDeclarationSiteOnly.kt"); } + @Test + @TestMetadata("jvmCrossinlineRedundant.kt") + public void testJvmCrossinlineRedundant() throws Exception { + runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt"); + } + @Test @TestMetadata("jvmCrossinlineSAMDeclarationSite.kt") public void testJvmCrossinlineSAMDeclarationSite() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrCompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrCompileKotlinAgainstInlineKotlinTestGenerated.java index 3554c0785ea..dafddadcfd3 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrCompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrCompileKotlinAgainstInlineKotlinTestGenerated.java @@ -882,6 +882,12 @@ public class IrCompileKotlinAgainstInlineKotlinTestGenerated extends AbstractIrC runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineLambdaDeclarationSiteOnly.kt"); } + @Test + @TestMetadata("jvmCrossinlineRedundant.kt") + public void testJvmCrossinlineRedundant() throws Exception { + runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt"); + } + @Test @TestMetadata("jvmCrossinlineSAMDeclarationSite.kt") public void testJvmCrossinlineSAMDeclarationSite() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmIrAgainstOldBoxInlineTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmIrAgainstOldBoxInlineTestGenerated.java index f3cb9911317..9a7fd74cba4 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmIrAgainstOldBoxInlineTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmIrAgainstOldBoxInlineTestGenerated.java @@ -882,6 +882,12 @@ public class JvmIrAgainstOldBoxInlineTestGenerated extends AbstractJvmIrAgainstO runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineLambdaDeclarationSiteOnly.kt"); } + @Test + @TestMetadata("jvmCrossinlineRedundant.kt") + public void testJvmCrossinlineRedundant() throws Exception { + runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt"); + } + @Test @TestMetadata("jvmCrossinlineSAMDeclarationSite.kt") public void testJvmCrossinlineSAMDeclarationSite() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmOldAgainstIrBoxInlineTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmOldAgainstIrBoxInlineTestGenerated.java index 08cd7b460d6..6f8fc4b31be 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmOldAgainstIrBoxInlineTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/JvmOldAgainstIrBoxInlineTestGenerated.java @@ -882,6 +882,12 @@ public class JvmOldAgainstIrBoxInlineTestGenerated extends AbstractJvmOldAgainst runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineLambdaDeclarationSiteOnly.kt"); } + @Test + @TestMetadata("jvmCrossinlineRedundant.kt") + public void testJvmCrossinlineRedundant() throws Exception { + runTest("compiler/testData/codegen/boxInline/assert/jvmCrossinlineRedundant.kt"); + } + @Test @TestMetadata("jvmCrossinlineSAMDeclarationSite.kt") public void testJvmCrossinlineSAMDeclarationSite() throws Exception {