From 3a10ea4f886b4c12526226dd94573c5f28278b94 Mon Sep 17 00:00:00 2001 From: Georgy Bronnikov Date: Mon, 23 Nov 2020 17:06:21 +0300 Subject: [PATCH] JVM_IR: synchronize code generation for inline functions --- .../FirBlackBoxCodegenTestGenerated.java | 6 ++ .../kotlin/backend/jvm/JvmBackendContext.kt | 2 + .../backend/jvm/codegen/ClassCodegen.kt | 71 ++++++++++--------- .../codegen/box/functions/mutualInline.kt | 11 +++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../IrJsCodegenBoxES6TestGenerated.java | 5 ++ .../IrJsCodegenBoxTestGenerated.java | 5 ++ .../semantics/JsCodegenBoxTestGenerated.java | 5 ++ .../IrCodegenBoxWasmTestGenerated.java | 5 ++ 11 files changed, 93 insertions(+), 34 deletions(-) create mode 100644 compiler/testData/codegen/box/functions/mutualInline.kt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index d95c02bd570..9d72ca2dd92 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -15612,6 +15612,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/functions/max.kt"); } + @Test + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @Test @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt index 775fe2f0d72..76deb542ad3 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt @@ -132,6 +132,8 @@ class JvmBackendContext( internal val continuationClassesVarsCountByType: MutableMap> = hashMapOf() + val inlineMethodGenerationLock = Any() + init { state.mapInlineClass = { descriptor -> typeMapper.mapType(referenceClass(descriptor).defaultType) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt index 0b104c33502..ad2f6e3b57d 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt @@ -22,6 +22,7 @@ import org.jetbrains.kotlin.config.LanguageVersionSettings import org.jetbrains.kotlin.descriptors.DescriptorVisibilities import org.jetbrains.kotlin.descriptors.DescriptorVisibility import org.jetbrains.kotlin.descriptors.Modality +import org.jetbrains.kotlin.ir.IrLock import org.jetbrains.kotlin.ir.builders.declarations.addFunction import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.descriptors.toIrBasedDescriptor @@ -48,8 +49,8 @@ import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull import org.jetbrains.kotlin.utils.addToStdlib.safeAs import org.jetbrains.org.objectweb.asm.* import org.jetbrains.org.objectweb.asm.commons.Method -import org.jetbrains.org.objectweb.asm.tree.MethodNode import java.io.File +import java.util.concurrent.ConcurrentHashMap interface MetadataSerializer { fun serialize(metadata: MetadataSource): Pair? @@ -258,33 +259,6 @@ class ClassCodegen private constructor( return listOf(File(entry.name)) } - companion object { - fun getOrCreate( - irClass: IrClass, - context: JvmBackendContext, - // The `parentFunction` is only set for classes nested inside of functions. This is usually safe, since there is no - // way to refer to (inline) members of such a class from outside of the function unless the function in question is - // itself declared as inline. In that case, the function will be compiled before we can refer to the nested class. - // - // The one exception to this rule are anonymous objects defined as members of a class. These are nested inside of the - // class initializer, but can be referred to from anywhere within the scope of the class. That's why we have to ensure - // that all references to classes inside of have a non-null `parentFunction`. - parentFunction: IrFunction? = irClass.parent.safeAs()?.takeIf { - it.origin == JvmLoweredDeclarationOrigin.CLASS_STATIC_INITIALIZER - }, - ): ClassCodegen = - context.classCodegens.getOrPut(irClass) { ClassCodegen(irClass, context, parentFunction) }.also { - assert(parentFunction == null || it.parentFunction == parentFunction) { - "inconsistent parent function for ${irClass.render()}:\n" + - "New: ${parentFunction!!.render()}\n" + - "Old: ${it.parentFunction?.render()}" - } - } - - private fun JvmClassSignature.hasInvalidName() = - name.splitToSequence('/').any { identifier -> identifier.any { it in JvmSimpleNameBacktickChecker.INVALID_CHARS } } - } - private fun generateField(field: IrField) { val fieldType = typeMapper.mapType(field) val fieldSignature = @@ -324,7 +298,7 @@ class ClassCodegen private constructor( } } - private val generatedInlineMethods = mutableMapOf() + private val generatedInlineMethods = ConcurrentHashMap() fun generateMethodNode(method: IrFunction): SMAPAndMethodNode { if (!method.isInline && !method.isSuspendCapturingCrossinline()) { @@ -335,11 +309,13 @@ class ClassCodegen private constructor( // multiple times if declared in a `finally` block - should they be cached? return FunctionCodegen(method, this).generate() } - val (node, smap) = generatedInlineMethods.getOrPut(method) { FunctionCodegen(method, this).generate() } - val copy = with(node) { MethodNode(Opcodes.API_VERSION, access, name, desc, signature, exceptions.toTypedArray()) } - node.instructions.resetLabels() - node.accept(copy) - return SMAPAndMethodNode(copy, smap) + + // Only allow generation of one inline method at a time, to avoid deadlocks when files call inline methods of each other. + val (node, smap) = + generatedInlineMethods[method] ?: synchronized(context.inlineMethodGenerationLock) { + generatedInlineMethods.getOrPut(method) { FunctionCodegen(method, this).generate() } + } + return SMAPAndMethodNode(cloneMethodNode(node), smap) } private fun generateMethod(method: IrFunction, classSMAP: SourceMapper, delegatedPropertyOptimizer: DelegatedPropertyOptimizer?) { @@ -483,6 +459,33 @@ class ClassCodegen private constructor( else OtherOrigin(psiElement, descriptor) } + + companion object { + fun getOrCreate( + irClass: IrClass, + context: JvmBackendContext, + // The `parentFunction` is only set for classes nested inside of functions. This is usually safe, since there is no + // way to refer to (inline) members of such a class from outside of the function unless the function in question is + // itself declared as inline. In that case, the function will be compiled before we can refer to the nested class. + // + // The one exception to this rule are anonymous objects defined as members of a class. These are nested inside of the + // class initializer, but can be referred to from anywhere within the scope of the class. That's why we have to ensure + // that all references to classes inside of have a non-null `parentFunction`. + parentFunction: IrFunction? = irClass.parent.safeAs()?.takeIf { + it.origin == JvmLoweredDeclarationOrigin.CLASS_STATIC_INITIALIZER + }, + ): ClassCodegen = + context.classCodegens.getOrPut(irClass) { ClassCodegen(irClass, context, parentFunction) }.also { + assert(parentFunction == null || it.parentFunction == parentFunction) { + "inconsistent parent function for ${irClass.render()}:\n" + + "New: ${parentFunction!!.render()}\n" + + "Old: ${it.parentFunction?.render()}" + } + } + + private fun JvmClassSignature.hasInvalidName() = + name.splitToSequence('/').any { identifier -> identifier.any { it in JvmSimpleNameBacktickChecker.INVALID_CHARS } } + } } private fun IrClass.getFlags(languageVersionSettings: LanguageVersionSettings): Int = diff --git a/compiler/testData/codegen/box/functions/mutualInline.kt b/compiler/testData/codegen/box/functions/mutualInline.kt new file mode 100644 index 00000000000..cd99a4da050 --- /dev/null +++ b/compiler/testData/codegen/box/functions/mutualInline.kt @@ -0,0 +1,11 @@ +// FILE: A.kt + +inline fun a(): String = b2() + +// FILE: B.kt + +inline fun b1(): String = a() + +inline fun b2(): String = "OK" + +fun box() = b1() \ No newline at end of file diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 394e3725b59..bc6581273e3 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -15612,6 +15612,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/functions/max.kt"); } + @Test + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @Test @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index a7e09d0eeac..5414593c1e5 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -15612,6 +15612,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/functions/max.kt"); } + @Test + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @Test @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 3d1940fdcdd..5565e1cb62d 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -12916,6 +12916,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/functions/max.kt"); } + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { runTest("compiler/testData/codegen/box/functions/nothisnoclosure.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java index 62fbcac6447..b112b2632b2 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java @@ -11394,6 +11394,11 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes runTest("compiler/testData/codegen/box/functions/localReturnInsideFunctionExpression.kt"); } + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { runTest("compiler/testData/codegen/box/functions/nothisnoclosure.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index d3b92601d73..aa5bf66bb6a 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -10851,6 +10851,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/functions/localReturnInsideFunctionExpression.kt"); } + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { runTest("compiler/testData/codegen/box/functions/nothisnoclosure.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 25a2cb619f7..98a81ca5fdd 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -10851,6 +10851,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/functions/localReturnInsideFunctionExpression.kt"); } + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { runTest("compiler/testData/codegen/box/functions/nothisnoclosure.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java index 16d925c73cc..57c5749c722 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java @@ -5512,6 +5512,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest runTest("compiler/testData/codegen/box/functions/localReturnInsideFunctionExpression.kt"); } + @TestMetadata("mutualInline.kt") + public void testMutualInline() throws Exception { + runTest("compiler/testData/codegen/box/functions/mutualInline.kt"); + } + @TestMetadata("nothisnoclosure.kt") public void testNothisnoclosure() throws Exception { runTest("compiler/testData/codegen/box/functions/nothisnoclosure.kt");