From d6ed93b2b873d6af1e08c345343d916be3df3f01 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 3 Jan 2020 11:43:59 +0100 Subject: [PATCH] JVM IR: fail when SyntheticAccessorLowering adds accessor to other files This would help for example in debugging the issue fixed in the previous commit. The only problem by now where this lowering tried to add accessors to foreign files was reproduced for interfaces inheriting from Cloneable. There, we generate a DefaultImpls bridge that calls protected method `clone` from java.lang.Cloneable.DefaultImpls. This makes no sense, but the old backend behaves the same. Instead of generating accessor for it in JVM IR, we now see all DefaultImpls bridges as public as a workaround. (The fact that assertion no longer fails here is checked e.g. by box/reflection/mapping/methodsFromObject.kt.) --- .../kotlin/backend/common/ir/IrUtils.kt | 5 ++-- .../jvm/descriptors/JvmDeclarationFactory.kt | 7 ++++- .../jvm/lower/SyntheticAccessorLowering.kt | 28 ++++++++++--------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt index 77a4cbe2087..29e0d076151 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt @@ -11,9 +11,9 @@ import org.jetbrains.kotlin.backend.common.deepCopyWithVariables import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.descriptors.annotations.Annotations import org.jetbrains.kotlin.ir.IrElement -import org.jetbrains.kotlin.ir.IrStatement import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET import org.jetbrains.kotlin.ir.builders.Scope import org.jetbrains.kotlin.ir.declarations.* @@ -514,6 +514,7 @@ fun createStaticFunctionWithReceivers( dispatchReceiverType: IrType? = oldFunction.dispatchReceiverParameter?.type, origin: IrDeclarationOrigin = oldFunction.origin, modality: Modality = Modality.FINAL, + visibility: Visibility = oldFunction.visibility, copyMetadata: Boolean = true ): IrSimpleFunction { val descriptor = (oldFunction.descriptor as? DescriptorWithContainerSource)?.let { @@ -524,7 +525,7 @@ fun createStaticFunctionWithReceivers( origin, IrSimpleFunctionSymbolImpl(descriptor), name, - oldFunction.visibility, + visibility, modality, oldFunction.returnType, isInline = oldFunction.isInline, diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt index 21e427c5fa3..fecd8035365 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt @@ -210,7 +210,12 @@ class JvmDeclarationFactory( else -> JvmLoweredDeclarationOrigin.DEFAULT_IMPLS_BRIDGE }, // Old backend doesn't generate ACC_FINAL on DefaultImpls methods. - modality = Modality.OPEN + modality = Modality.OPEN, + // Interface functions are always public, with one exception: clone in Cloneable, which is protected. However, Cloneable + // has no DefaultImpls, so this merely replicates the incorrect behavior of the old backend. We should rather not generate + // a bridge to clone when interface inherits from Cloneable at all. Below, we force everything, including those bridges, + // to be public so that we won't try to generate synthetic accessor for them. + visibility = Visibilities.PUBLIC ).also { it.copyAttributes(interfaceFun) } } } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt index f369b063f8f..313ce67de55 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt @@ -39,13 +39,21 @@ import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.utils.addToStdlib.safeAs internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElementTransformerVoidWithContext(), FileLoweringPass { - private val pendingTransformations = mutableListOf>() + private val pendingAccessorsToAdd = mutableListOf() private val inlineLambdaToCallSite = mutableMapOf() override fun lower(irFile: IrFile) { inlineLambdaToCallSite.putAll(IrInlineReferenceLocator.scan(context, irFile).lambdaToCallSite) irFile.transformChildrenVoid(this) - pendingTransformations.forEach { it() } + + for (accessor in pendingAccessorsToAdd) { + assert(accessor.file == irFile) { + "SyntheticAccessorLowering should not attempt to modify other files!\n" + + "While lowering this file: ${irFile.render()}\n" + + "Trying to add this accessor: ${accessor.render()}" + } + (accessor.parent as IrDeclarationContainer).declarations.add(accessor) + } } private val functionMap = mutableMapOf() @@ -68,11 +76,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle !expression.symbol.isAccessible(withSuper, thisSymbol) -> functionMap.getOrPut(expression.symbol) { when (val symbol = expression.symbol) { - is IrConstructorSymbol -> symbol.owner.makeConstructorAccessor().also { accessor -> - pendingTransformations.add { - (accessor.parent as IrDeclarationContainer).declarations.add(accessor) - } - }.symbol + is IrConstructorSymbol -> symbol.owner.makeConstructorAccessor().also(pendingAccessorsToAdd::add).symbol is IrSimpleFunctionSymbol -> symbol.owner.makeSimpleFunctionAccessor(expression as IrCall).symbol else -> error("Unknown subclass of IrFunctionSymbol") } @@ -101,9 +105,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle override fun visitConstructor(declaration: IrConstructor): IrStatement { if (declaration.isOrShouldBeHidden) { - pendingTransformations.add { - declaration.parentAsClass.declarations.add(handleHiddenConstructor(declaration)) - } + pendingAccessorsToAdd.add(handleHiddenConstructor(declaration)) declaration.visibility = Visibilities.PRIVATE } @@ -241,7 +243,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle isSuspend = source.isSuspend // synthetic accessors of suspend functions are handled in codegen }.also { accessor -> accessor.parent = parent - pendingTransformations.add { (accessor.parent as IrDeclarationContainer).declarations.add(accessor) } + pendingAccessorsToAdd.add(accessor) accessor.copyTypeParametersFrom(source, JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR) accessor.copyValueParametersToStatic(source, JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR, dispatchReceiverType) @@ -273,7 +275,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle returnType = fieldSymbol.owner.type }.also { accessor -> accessor.parent = fieldSymbol.owner.accessorParent() - pendingTransformations.add { (accessor.parent as IrDeclarationContainer).declarations.add(accessor) } + pendingAccessorsToAdd.add(accessor) if (!fieldSymbol.owner.isStatic) { accessor.addValueParameter( @@ -309,7 +311,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle returnType = context.irBuiltIns.unitType }.also { accessor -> accessor.parent = fieldSymbol.owner.accessorParent() - pendingTransformations.add { (accessor.parent as IrDeclarationContainer).declarations.add(accessor) } + pendingAccessorsToAdd.add(accessor) if (!fieldSymbol.owner.isStatic) { accessor.addValueParameter(