From b63f9176a391a4b8c7c2ad9d950033244483919a Mon Sep 17 00:00:00 2001 From: Georgy Bronnikov Date: Tue, 22 Oct 2019 16:14:04 +0300 Subject: [PATCH] JVM_IR: repair overriddenSymbols in InterfaceDelegationLowering When creating delegation redirectors for default implementations of interface functions, `overriddenSymbols` should be repaired across the whole module, not just a particular file. --- .../jvm/descriptors/JvmDeclarationFactory.kt | 55 ++++++++- .../jvm/lower/InterfaceDelegationLowering.kt | 115 ++++++------------ .../box/bridges/fakeOverrideMultiFile.kt | 18 +++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 + .../LightAnalysisModeTestGenerated.java | 5 + .../ir/IrBlackBoxCodegenTestGenerated.java | 5 + .../IrJsCodegenBoxTestGenerated.java | 5 + .../semantics/JsCodegenBoxTestGenerated.java | 5 + 8 files changed, 127 insertions(+), 86 deletions(-) create mode 100644 compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt 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 db150453184..9a6cad9cb37 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 @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.backend.jvm.descriptors +import org.jetbrains.kotlin.backend.common.DescriptorsToIrRemapper import org.jetbrains.kotlin.backend.common.descriptors.WrappedClassConstructorDescriptor import org.jetbrains.kotlin.backend.common.descriptors.WrappedClassDescriptor import org.jetbrains.kotlin.backend.common.descriptors.WrappedValueParameterDescriptor @@ -19,12 +20,8 @@ import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET import org.jetbrains.kotlin.ir.builders.declarations.buildField import org.jetbrains.kotlin.ir.builders.setSourceRange import org.jetbrains.kotlin.ir.declarations.* -import org.jetbrains.kotlin.ir.declarations.impl.IrClassImpl -import org.jetbrains.kotlin.ir.declarations.impl.IrConstructorImpl -import org.jetbrains.kotlin.ir.declarations.impl.IrValueParameterImpl -import org.jetbrains.kotlin.ir.symbols.impl.IrClassSymbolImpl -import org.jetbrains.kotlin.ir.symbols.impl.IrConstructorSymbolImpl -import org.jetbrains.kotlin.ir.symbols.impl.IrValueParameterSymbolImpl +import org.jetbrains.kotlin.ir.declarations.impl.* +import org.jetbrains.kotlin.ir.symbols.impl.* import org.jetbrains.kotlin.ir.types.IrType import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.load.java.JavaVisibilities @@ -41,6 +38,7 @@ class JvmDeclarationFactory( private val defaultImplsMethods = HashMap() private val defaultImplsClasses = HashMap() + private val defaultImplsRedirections = HashMap() override fun getFieldForEnumEntry(enumEntry: IrEnumEntry, entryType: IrType): IrField = singletonFieldDeclarations.getOrPut(enumEntry) { @@ -187,4 +185,49 @@ class JvmDeclarationFactory( createImplicitParameterDeclarationWithWrappedDescriptor() } } + + fun getDefaultImplsRedirection(fakeOverride: IrSimpleFunction): IrSimpleFunction = + defaultImplsRedirections.getOrPut(fakeOverride) { + assert(fakeOverride.origin == IrDeclarationOrigin.FAKE_OVERRIDE) + val irClass = fakeOverride.parentAsClass + val descriptor = DescriptorsToIrRemapper.remapDeclaredSimpleFunction(fakeOverride.descriptor) + with(fakeOverride) { + IrFunctionImpl( + UNDEFINED_OFFSET, UNDEFINED_OFFSET, JvmLoweredDeclarationOrigin.DEFAULT_IMPLS_BRIDGE, + IrSimpleFunctionSymbolImpl(descriptor), + name, visibility, modality, returnType, + isInline = isInline, + isExternal = false, + isTailrec = false, + isSuspend = isSuspend, + isExpect = false + ).apply { + descriptor.bind(this) + parent = irClass + overriddenSymbols.addAll(fakeOverride.overriddenSymbols) + copyParameterDeclarationsFrom(fakeOverride) + annotations.addAll(fakeOverride.annotations) + fakeOverride.correspondingPropertySymbol?.owner?.let { fakeOverrideProperty -> + // NB: property is only generated for the sake of the type mapper. + // If both setter and getter are present, original property will be duplicated. + val newPropertyDescriptor = DescriptorsToIrRemapper.remapDeclaredProperty(fakeOverrideProperty.descriptor) + correspondingPropertySymbol = with(fakeOverrideProperty) { + IrPropertyImpl( + UNDEFINED_OFFSET, UNDEFINED_OFFSET, + IrDeclarationOrigin.DEFINED, IrPropertySymbolImpl(newPropertyDescriptor), + name, visibility, modality, isVar, isConst, isLateinit, isDelegated, + isExternal = false, + isExpect = isExpect + ).apply { + newPropertyDescriptor.bind(this) + parent = irClass + }.symbol + } + } + } + } + + + } + } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceDelegationLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceDelegationLowering.kt index 9bf3e946f68..a78aba84e2b 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceDelegationLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceDelegationLowering.kt @@ -5,15 +5,12 @@ package org.jetbrains.kotlin.backend.jvm.lower -import org.jetbrains.kotlin.backend.common.DescriptorsToIrRemapper import org.jetbrains.kotlin.backend.common.FileLoweringPass -import org.jetbrains.kotlin.backend.common.ir.copyParameterDeclarationsFrom import org.jetbrains.kotlin.backend.common.ir.isMethodOfAny import org.jetbrains.kotlin.backend.common.ir.passTypeArgumentsFrom import org.jetbrains.kotlin.backend.common.lower.createIrBuilder import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase import org.jetbrains.kotlin.backend.jvm.JvmBackendContext -import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface import org.jetbrains.kotlin.backend.jvm.ir.hasJvmDefault import org.jetbrains.kotlin.descriptors.Modality @@ -29,13 +26,8 @@ import org.jetbrains.kotlin.ir.declarations.IrClass import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin import org.jetbrains.kotlin.ir.declarations.IrFile import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction -import org.jetbrains.kotlin.ir.declarations.impl.IrFunctionImpl -import org.jetbrains.kotlin.ir.declarations.impl.IrPropertyImpl import org.jetbrains.kotlin.ir.expressions.IrCall import org.jetbrains.kotlin.ir.expressions.IrExpression -import org.jetbrains.kotlin.ir.symbols.IrSimpleFunctionSymbol -import org.jetbrains.kotlin.ir.symbols.impl.IrPropertySymbolImpl -import org.jetbrains.kotlin.ir.symbols.impl.IrSimpleFunctionSymbolImpl import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.ir.visitors.* import org.jetbrains.kotlin.utils.addToStdlib.safeAs @@ -47,12 +39,10 @@ internal val interfaceDelegationPhase = makeIrFilePhase( ) private class InterfaceDelegationLowering(val context: JvmBackendContext) : IrElementVisitorVoid, FileLoweringPass { - val replacementMap = mutableMapOf() override fun lower(irFile: IrFile) { irFile.acceptChildrenVoid(this) - // TODO: Replacer should be run on whole module, not on a single file. - irFile.acceptVoid(OverriddenSymbolsReplacer(replacementMap)) + irFile.acceptVoid(OverriddenSymbolsReplacer()) } override fun visitElement(element: IrElement) { @@ -69,81 +59,44 @@ private class InterfaceDelegationLowering(val context: JvmBackendContext) : IrEl private fun generateInterfaceMethods(irClass: IrClass) { val toRemove = mutableListOf() for (function in irClass.functions.toList()) { // Copy the list, because we are adding new declarations from the loop - if (function.origin !== IrDeclarationOrigin.FAKE_OVERRIDE) continue + function.getTargetForRedirection()?.let { implementation -> + toRemove.add(function) - // Only generate interface delegation for functions immediately inherited from an interface. - // (Otherwise, delegation will be present in the parent class) - if (function.overriddenSymbols.any { !it.owner.parentAsClass.isInterface && it.owner.modality != Modality.ABSTRACT }) { - continue + val delegation = generateDelegationToDefaultImpl(implementation, function) + irClass.declarations.add(delegation) } - - val implementation = function.resolveFakeOverride() ?: continue - - if (!implementation.hasInterfaceParent() - || Visibilities.isPrivate(implementation.visibility) - || implementation.isDefinitelyNotDefaultImplsMethod() - || implementation.isMethodOfAny() - || (!context.state.jvmDefaultMode.isCompatibility && implementation.hasJvmDefault()) - ) { - continue - } - - toRemove.add(function) - - val delegation = generateDelegationToDefaultImpl(irClass, implementation, function) - irClass.declarations.add(delegation) - replacementMap[function.symbol] = delegation.symbol } irClass.declarations.removeAll(toRemove) } + private fun IrSimpleFunction.getTargetForRedirection(): IrSimpleFunction? { + if (origin != IrDeclarationOrigin.FAKE_OVERRIDE) return null + parent.let { if (it is IrClass && it.isJvmInterface) return null } + + // Only generate interface delegation for functions immediately inherited from an interface. + // (Otherwise, delegation will be present in the parent class) + if (overriddenSymbols.any { !it.owner.parentAsClass.isInterface && it.owner.modality != Modality.ABSTRACT }) { + return null + } + + val implementation = resolveFakeOverride() ?: return null + if (!implementation.hasInterfaceParent() + || Visibilities.isPrivate(implementation.visibility) + || implementation.isDefinitelyNotDefaultImplsMethod() + || implementation.isMethodOfAny() + || (!context.state.jvmDefaultMode.isCompatibility && implementation.hasJvmDefault()) + ) { + return null + } + + return implementation + } + private fun generateDelegationToDefaultImpl( - irClass: IrClass, interfaceImplementation: IrSimpleFunction, classOverride: IrSimpleFunction ): IrSimpleFunction { - val inheritedProperty = classOverride.correspondingPropertySymbol?.owner - val descriptor = DescriptorsToIrRemapper.remapDeclaredSimpleFunction(classOverride.descriptor) - - val irFunction = - IrFunctionImpl( - UNDEFINED_OFFSET, - UNDEFINED_OFFSET, - JvmLoweredDeclarationOrigin.DEFAULT_IMPLS_BRIDGE, - IrSimpleFunctionSymbolImpl(descriptor), - classOverride.name, - Visibilities.PUBLIC, - classOverride.modality, - classOverride.returnType, - isInline = classOverride.isInline, - isExternal = false, - isTailrec = false, - isSuspend = classOverride.isSuspend, - isExpect = false - ).apply { - descriptor.bind(this) - parent = irClass - overriddenSymbols.addAll(classOverride.overriddenSymbols) - copyParameterDeclarationsFrom(classOverride) - annotations.addAll(classOverride.annotations) - - if (inheritedProperty != null) { - val propertyDescriptor = DescriptorsToIrRemapper.remapDeclaredProperty(inheritedProperty.descriptor) - IrPropertyImpl( - UNDEFINED_OFFSET, UNDEFINED_OFFSET, IrDeclarationOrigin.DEFINED, IrPropertySymbolImpl(propertyDescriptor), - inheritedProperty.name, Visibilities.PUBLIC, inheritedProperty.modality, inheritedProperty.isVar, - isConst = inheritedProperty.isConst, - isLateinit = inheritedProperty.isLateinit, - isDelegated = inheritedProperty.isDelegated, - isExpect = inheritedProperty.isExpect, - isExternal = false - ).apply { - propertyDescriptor.bind(this) - parent = irClass - correspondingPropertySymbol = symbol - } - } - } + val irFunction = context.declarationFactory.getDefaultImplsRedirection(classOverride) val defaultImplFun = context.declarationFactory.getDefaultImplsFunction(interfaceImplementation) context.createIrBuilder(irFunction.symbol, UNDEFINED_OFFSET, UNDEFINED_OFFSET).apply { @@ -163,15 +116,18 @@ private class InterfaceDelegationLowering(val context: JvmBackendContext) : IrEl return irFunction } - private class OverriddenSymbolsReplacer(val replacementMap: Map) : - IrElementVisitorVoid { + private inner class OverriddenSymbolsReplacer : IrElementVisitorVoid { override fun visitElement(element: IrElement) { element.acceptChildrenVoid(this) } override fun visitSimpleFunction(declaration: IrSimpleFunction) { - declaration.overriddenSymbols.replaceAll { symbol -> replacementMap[symbol] ?: symbol } + declaration.overriddenSymbols.replaceAll { symbol -> + if (symbol.owner.getTargetForRedirection() != null) + context.declarationFactory.getDefaultImplsRedirection(symbol.owner).symbol + else symbol + } super.visitSimpleFunction(declaration) } } @@ -236,7 +192,6 @@ private class InterfaceDefaultCallsLowering(val context: JvmBackendContext) : Ir } } - private fun IrSimpleFunction.isDefinitelyNotDefaultImplsMethod() = resolveFakeOverride()?.let { origin == IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB } == true || origin == IrDeclarationOrigin.FUNCTION_FOR_DEFAULT_PARAMETER || diff --git a/compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt b/compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt new file mode 100644 index 00000000000..af4180c9896 --- /dev/null +++ b/compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt @@ -0,0 +1,18 @@ +// FILE: 1.kt +class Test: Impl(), CProvider + +fun box() = "OK" + +// FILE: 2.kt +open class C +class D: C() + +interface CProvider { + fun getC(): C +} + +interface DProvider { + fun getC(): D = D() +} + +open class Impl: DProvider diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index a47c9e1b344..2f68232f53d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -1349,6 +1349,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/bridges/fakeOverrideFromInterfaceThroughIntermediateClass.kt"); } + @TestMetadata("fakeOverrideMultiFile.kt") + public void testFakeOverrideMultiFile() throws Exception { + runTest("compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt"); + } + @TestMetadata("fakeOverrideOfTraitImpl.kt") public void testFakeOverrideOfTraitImpl() throws Exception { runTest("compiler/testData/codegen/box/bridges/fakeOverrideOfTraitImpl.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 8ca4585acbf..e24fcb2d84a 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -1349,6 +1349,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/bridges/fakeOverrideFromInterfaceThroughIntermediateClass.kt"); } + @TestMetadata("fakeOverrideMultiFile.kt") + public void testFakeOverrideMultiFile() throws Exception { + runTest("compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt"); + } + @TestMetadata("fakeOverrideOfTraitImpl.kt") public void testFakeOverrideOfTraitImpl() throws Exception { runTest("compiler/testData/codegen/box/bridges/fakeOverrideOfTraitImpl.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index d531e60cabb..98d5130831b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -1329,6 +1329,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/bridges/fakeOverrideFromInterfaceThroughIntermediateClass.kt"); } + @TestMetadata("fakeOverrideMultiFile.kt") + public void testFakeOverrideMultiFile() throws Exception { + runTest("compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt"); + } + @TestMetadata("fakeOverrideOfTraitImpl.kt") public void testFakeOverrideOfTraitImpl() throws Exception { runTest("compiler/testData/codegen/box/bridges/fakeOverrideOfTraitImpl.kt"); 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 ca2ca89f035..851f9406bf3 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 @@ -1004,6 +1004,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/bridges/fakeOverrideFromInterfaceThroughIntermediateClass.kt"); } + @TestMetadata("fakeOverrideMultiFile.kt") + public void testFakeOverrideMultiFile() throws Exception { + runTest("compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt"); + } + @TestMetadata("fakeOverrideOfTraitImpl.kt") public void testFakeOverrideOfTraitImpl() throws Exception { runTest("compiler/testData/codegen/box/bridges/fakeOverrideOfTraitImpl.kt"); 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 e91a61cae49..9db812c71ec 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 @@ -1004,6 +1004,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/bridges/fakeOverrideFromInterfaceThroughIntermediateClass.kt"); } + @TestMetadata("fakeOverrideMultiFile.kt") + public void testFakeOverrideMultiFile() throws Exception { + runTest("compiler/testData/codegen/box/bridges/fakeOverrideMultiFile.kt"); + } + @TestMetadata("fakeOverrideOfTraitImpl.kt") public void testFakeOverrideOfTraitImpl() throws Exception { runTest("compiler/testData/codegen/box/bridges/fakeOverrideOfTraitImpl.kt");