From 791be7c2dccc3ead9d9a3bb5b9248a0ecf4d7806 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 5 Nov 2020 13:08:03 +0100 Subject: [PATCH] JVM IR: improve ABI of properties in multifile facades When copying top level declarations from multifile parts to facades, also copy corresponding properties. This allows to keep their annotations, which are later used in codegen (for example, to generate ACC_DEPRECATED on property accessors), and allows to get rid of the hack where the JVM name of the property accessor was computed prematurely via methodSignatureMapper. Also, don't copy `$annotations` methods for non-const properties to facades because the old backend only generates them in parts (which might be a separate problem, see KT-27644). #KT-40262 Fixed --- .../kotlin/fir/backend/FirMetadataSource.kt | 6 +- .../jvm/lower/GenerateMultifileFacades.kt | 61 ++++++++++++++----- .../kotlin/ir/declarations/MetadataSource.kt | 12 +++- .../functionInMultifileClass_ir.txt | 4 +- .../deprecated/deprecatedInMultifileClass.kt | 22 +++++++ .../deprecated/deprecatedInMultifileClass.txt | 21 +++++++ .../codegen/BytecodeListingTestGenerated.java | 5 ++ .../ir/IrBytecodeListingTestGenerated.java | 5 ++ 8 files changed, 115 insertions(+), 21 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.kt create mode 100644 compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.txt diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirMetadataSource.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirMetadataSource.kt index 8ab522514da..55a27c9ae95 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirMetadataSource.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirMetadataSource.kt @@ -31,5 +31,7 @@ sealed class FirMetadataSource : MetadataSource { class Function(override val fir: FirFunction<*>) : FirMetadataSource(), MetadataSource.Function - class Property(override val fir: FirProperty) : FirMetadataSource(), MetadataSource.Property -} \ No newline at end of file + class Property(override val fir: FirProperty) : FirMetadataSource(), MetadataSource.Property { + override val isConst: Boolean get() = fir.isConst + } +} diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/GenerateMultifileFacades.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/GenerateMultifileFacades.kt index 1e57ce543e4..8e52efbd827 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/GenerateMultifileFacades.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/GenerateMultifileFacades.kt @@ -17,6 +17,7 @@ import org.jetbrains.kotlin.backend.common.phaser.makeCustomPhase import org.jetbrains.kotlin.backend.jvm.JvmBackendContext import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin import org.jetbrains.kotlin.backend.jvm.codegen.fileParent +import org.jetbrains.kotlin.backend.jvm.codegen.isSyntheticMethodForProperty import org.jetbrains.kotlin.config.JvmAnalysisFlags import org.jetbrains.kotlin.descriptors.DescriptorVisibilities import org.jetbrains.kotlin.descriptors.Modality @@ -27,6 +28,7 @@ import org.jetbrains.kotlin.ir.builders.* import org.jetbrains.kotlin.ir.builders.declarations.addConstructor import org.jetbrains.kotlin.ir.builders.declarations.buildClass import org.jetbrains.kotlin.ir.builders.declarations.buildFun +import org.jetbrains.kotlin.ir.builders.declarations.buildProperty import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.declarations.impl.IrFileImpl import org.jetbrains.kotlin.ir.expressions.* @@ -39,7 +41,6 @@ import org.jetbrains.kotlin.ir.visitors.IrElementTransformer import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid import org.jetbrains.kotlin.load.java.JavaDescriptorVisibilities -import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.inline.INLINE_ONLY_ANNOTATION_FQ_NAME import org.jetbrains.kotlin.resolve.jvm.JvmClassName @@ -140,19 +141,23 @@ private fun generateMultifileFacades( context.multifileFacadeForPart[partClass.attributeOwnerId as IrClass] = jvmClassName context.multifileFacadeClassForPart[partClass.attributeOwnerId as IrClass] = facadeClass - moveFieldsOfConstProperties(partClass, facadeClass) - + val correspondingProperties = CorrespondingPropertyCache(context, facadeClass) for (member in partClass.declarations) { if (member !is IrSimpleFunction) continue + + val correspondingProperty = member.correspondingPropertySymbol?.owner if (member.hasAnnotation(INLINE_ONLY_ANNOTATION_FQ_NAME) || - member.correspondingPropertySymbol?.owner?.hasAnnotation(INLINE_ONLY_ANNOTATION_FQ_NAME) == true + correspondingProperty?.hasAnnotation(INLINE_ONLY_ANNOTATION_FQ_NAME) == true ) continue - val newMember = member.createMultifileDelegateIfNeeded(context, facadeClass, shouldGeneratePartHierarchy) + val newMember = + member.createMultifileDelegateIfNeeded(context, facadeClass, correspondingProperties, shouldGeneratePartHierarchy) if (newMember != null) { functionDelegates[member] = newMember } } + + moveFieldsOfConstProperties(partClass, facadeClass, correspondingProperties) } file @@ -181,11 +186,16 @@ private fun modifyMultifilePartsForHierarchy(context: JvmBackendContext, parts: return parts.last() } -private fun moveFieldsOfConstProperties(partClass: IrClass, facadeClass: IrClass) { +private fun moveFieldsOfConstProperties(partClass: IrClass, facadeClass: IrClass, correspondingProperties: CorrespondingPropertyCache) { partClass.declarations.transformFlat { member -> if (member is IrField && member.shouldMoveToFacade()) { member.patchDeclarationParents(facadeClass) facadeClass.declarations.add(member) + member.correspondingPropertySymbol?.let { oldPropertySymbol -> + val newProperty = correspondingProperties.getOrCopyProperty(oldPropertySymbol.owner) + member.correspondingPropertySymbol = newProperty.symbol + newProperty.backingField = member + } emptyList() } else null } @@ -199,25 +209,31 @@ private fun IrField.shouldMoveToFacade(): Boolean { private fun IrSimpleFunction.createMultifileDelegateIfNeeded( context: JvmBackendContext, facadeClass: IrClass, + correspondingProperties: CorrespondingPropertyCache, shouldGeneratePartHierarchy: Boolean ): IrSimpleFunction? { val target = this if (DescriptorVisibilities.isPrivate(visibility) || name == StaticInitializersLowering.clinitName || - origin == JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR + origin == JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR || + // $annotations methods in the facade are only needed for const properties. + (isSyntheticMethodForProperty && (metadata as? MetadataSource.Property)?.isConst != true) ) return null val function = context.irFactory.buildFun { updateFrom(target) isFakeOverride = shouldGeneratePartHierarchy - name = if (shouldGeneratePartHierarchy) { - target.name - } else { - // Generating a bridge. If the bridge is targeting a property getter - // we need to take care to get the name right. The bridge is not a - // property getter itself. - Name.identifier(context.methodSignatureMapper.mapFunctionName(target)) + name = target.name + } + + val targetProperty = correspondingPropertySymbol?.owner + if (targetProperty != null) { + val newProperty = correspondingProperties.getOrCopyProperty(targetProperty) + function.correspondingPropertySymbol = newProperty.symbol + when (target.valueParameters.size) { + 0 -> newProperty.getter = function + 1 -> newProperty.setter = function } } @@ -250,6 +266,23 @@ private fun IrSimpleFunction.createMultifileDelegateIfNeeded( return function } +private class CorrespondingPropertyCache(private val context: JvmBackendContext, private val facadeClass: IrClass) { + private var cache: MutableMap? = null + + fun getOrCopyProperty(from: IrProperty): IrProperty { + val cache = cache ?: mutableMapOf().also { cache = it } + return cache.getOrPut(from) { + context.irFactory.buildProperty { + updateFrom(from) + name = from.name + }.apply { + parent = facadeClass + copyAnnotationsFrom(from) + } + } + } +} + private class UpdateFunctionCallSites( private val functionDelegates: MutableMap ) : FileLoweringPass, IrElementTransformer { diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/declarations/MetadataSource.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/declarations/MetadataSource.kt index f96927c50a6..e3e11a6d565 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/declarations/MetadataSource.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/declarations/MetadataSource.kt @@ -14,7 +14,9 @@ interface MetadataSource { interface File : MetadataSource interface Class : MetadataSource interface Function : MetadataSource - interface Property : MetadataSource + interface Property : MetadataSource { + val isConst: Boolean + } } sealed class DescriptorMetadataSource : MetadataSource { @@ -30,8 +32,12 @@ sealed class DescriptorMetadataSource : MetadataSource { class Function(override val descriptor: FunctionDescriptor) : DescriptorMetadataSource(), MetadataSource.Function - class Property(override val descriptor: PropertyDescriptor) : DescriptorMetadataSource(), MetadataSource.Property + class Property(override val descriptor: PropertyDescriptor) : DescriptorMetadataSource(), MetadataSource.Property { + override val isConst: Boolean get() = descriptor.isConst + } class LocalDelegatedProperty(override val descriptor: VariableDescriptorWithAccessors) : DescriptorMetadataSource(), - MetadataSource.Property + MetadataSource.Property { + override val isConst: Boolean get() = descriptor.isConst + } } diff --git a/compiler/testData/codegen/bytecodeListing/defaultArguments/functionInMultifileClass_ir.txt b/compiler/testData/codegen/bytecodeListing/defaultArguments/functionInMultifileClass_ir.txt index b6a33ff0961..176aaa4bc26 100644 --- a/compiler/testData/codegen/bytecodeListing/defaultArguments/functionInMultifileClass_ir.txt +++ b/compiler/testData/codegen/bytecodeListing/defaultArguments/functionInMultifileClass_ir.txt @@ -5,8 +5,8 @@ public final class A { public final static method internal(@org.jetbrains.annotations.NotNull p0: java.lang.String): void public synthetic static method internalInline$default(p0: java.lang.String, p1: int, p2: int, p3: java.lang.Object): void public final static method internalInline(@org.jetbrains.annotations.NotNull p0: java.lang.String, p1: int): void - synthetic static method private$A__TestKt$default(p0: java.lang.String, p1: int, p2: java.lang.Object): void - synthetic static method privateInline$A__TestKt$default(p0: java.lang.String, p1: int, p2: int, p3: java.lang.Object): void + synthetic static method private$default(p0: java.lang.String, p1: int, p2: java.lang.Object): void + synthetic static method privateInline$default(p0: java.lang.String, p1: int, p2: int, p3: java.lang.Object): void public synthetic static method public$default(p0: java.lang.String, p1: int, p2: java.lang.Object): void public final static method public(@org.jetbrains.annotations.NotNull p0: java.lang.String): void public synthetic static method publicInline$default(p0: java.lang.String, p1: int, p2: int, p3: java.lang.Object): void diff --git a/compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.kt b/compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.kt new file mode 100644 index 00000000000..d711fdfb500 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.kt @@ -0,0 +1,22 @@ +// WITH_RUNTIME +// FILE: part.kt + +@file:JvmMultifileClass +@file:JvmName("A") + +package test + +@Deprecated("") +val str: String + get() = "" + +@Deprecated("") +fun f() {} + +@Deprecated("") +val Int.ext: Int get() = this + +var Int.extA: Int + get() = this + @Deprecated("") + set(value) {} diff --git a/compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.txt b/compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.txt new file mode 100644 index 00000000000..69d960c5e50 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.txt @@ -0,0 +1,21 @@ +@kotlin.Metadata +public final class test/A { + // source: 'part.kt' + public deprecated final static @kotlin.Deprecated method f(): void + public deprecated final static method getExt(p0: int): int + public final static method getExtA(p0: int): int + public deprecated final static @org.jetbrains.annotations.NotNull method getStr(): java.lang.String + public deprecated final static @kotlin.Deprecated method setExtA(p0: int, p1: int): void +} + +@kotlin.Metadata +synthetic final class test/A__PartKt { + // source: 'part.kt' + public deprecated final static @kotlin.Deprecated method f(): void + public synthetic deprecated static @kotlin.Deprecated method getExt$annotations(p0: int): void + public deprecated final static method getExt(p0: int): int + public final static method getExtA(p0: int): int + public synthetic deprecated static @kotlin.Deprecated method getStr$annotations(): void + public deprecated final static @org.jetbrains.annotations.NotNull method getStr(): java.lang.String + public deprecated final static @kotlin.Deprecated method setExtA(p0: int, p1: int): void +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java index c971df54263..9514486b157 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java @@ -738,6 +738,11 @@ public class BytecodeListingTestGenerated extends AbstractBytecodeListingTest { runTest("compiler/testData/codegen/bytecodeListing/deprecated/deprecatedEnumEntryFields.kt"); } + @TestMetadata("deprecatedInMultifileClass.kt") + public void testDeprecatedInMultifileClass() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.kt"); + } + @TestMetadata("deprecatedLateinitVar.kt") public void testDeprecatedLateinitVar() throws Exception { runTest("compiler/testData/codegen/bytecodeListing/deprecated/deprecatedLateinitVar.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java index 07612850dc8..13127f578f9 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java @@ -708,6 +708,11 @@ public class IrBytecodeListingTestGenerated extends AbstractIrBytecodeListingTes runTest("compiler/testData/codegen/bytecodeListing/deprecated/deprecatedEnumEntryFields.kt"); } + @TestMetadata("deprecatedInMultifileClass.kt") + public void testDeprecatedInMultifileClass() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/deprecated/deprecatedInMultifileClass.kt"); + } + @TestMetadata("deprecatedLateinitVar.kt") public void testDeprecatedLateinitVar() throws Exception { runTest("compiler/testData/codegen/bytecodeListing/deprecated/deprecatedLateinitVar.kt");