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.
2019-10-22 16:14:04 +03:00
parent c127cbaa6a
commit b63f9176a3
8 changed files with 127 additions and 86 deletions
@@ -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<IrSimpleFunction, IrSimpleFunction>()
    private val defaultImplsClasses = HashMap<IrClass, IrClass>()
+    private val defaultImplsRedirections = HashMap<IrSimpleFunction, IrSimpleFunction>()

    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
+                        }
+                    }
+                }
+            }
+
+
+        }
+
 }
@@ -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<IrSimpleFunctionSymbol, IrSimpleFunctionSymbol>()

    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<IrSimpleFunction>()
        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<IrSimpleFunctionSymbol, IrSimpleFunctionSymbol>) :
-        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 ||
@@ -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
@@ -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");
@@ -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");
@@ -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");
@@ -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");
@@ -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");