From 20d3e12e7cdc60af02d7e7c4620f4b577b1141c8 Mon Sep 17 00:00:00 2001 From: Igor Chevdar Date: Wed, 10 Oct 2018 14:30:46 +0300 Subject: [PATCH] Fix for https://github.com/JetBrains/kotlin-native/issues/2179. Erased local objects types, as otherwise we would have to copy them correctly, and this is hard because they could leak out of the inline function quite high (see the test). --- .../lower/DeepCopyIrTreeWithDescriptors.kt | 126 +----------------- .../backend/konan/optimizations/DFGBuilder.kt | 2 +- .../backend/konan/optimizations/DataFlowIR.kt | 11 +- backend.native/tests/build.gradle | 5 + .../inline/localObjectReturnedFromWhen.kt | 21 +++ 5 files changed, 36 insertions(+), 129 deletions(-) create mode 100644 backend.native/tests/codegen/inline/localObjectReturnedFromWhen.kt diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DeepCopyIrTreeWithDescriptors.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DeepCopyIrTreeWithDescriptors.kt index 25279b31858..fce13e0759e 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DeepCopyIrTreeWithDescriptors.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DeepCopyIrTreeWithDescriptors.kt @@ -14,7 +14,6 @@ import org.jetbrains.kotlin.backend.konan.descriptors.initialize import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.impl.* import org.jetbrains.kotlin.ir.IrElement -import org.jetbrains.kotlin.ir.builders.Scope import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.declarations.impl.* import org.jetbrains.kotlin.ir.descriptors.IrTemporaryVariableDescriptorImpl @@ -22,14 +21,11 @@ import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.expressions.impl.* import org.jetbrains.kotlin.ir.symbols.impl.createClassSymbolOrNull import org.jetbrains.kotlin.ir.symbols.impl.createFunctionSymbol -import org.jetbrains.kotlin.ir.symbols.impl.createValueSymbol import org.jetbrains.kotlin.ir.types.IrSimpleType import org.jetbrains.kotlin.ir.types.IrType import org.jetbrains.kotlin.ir.types.makeNullable import org.jetbrains.kotlin.ir.types.toKotlinType import org.jetbrains.kotlin.ir.util.* -import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid -import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.ir.visitors.acceptVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid import org.jetbrains.kotlin.name.Name @@ -43,8 +39,8 @@ import org.jetbrains.kotlin.types.TypeUtils import org.jetbrains.kotlin.types.Variance internal class DeepCopyIrTreeWithDescriptors(val targetDescriptor: FunctionDescriptor, - val parentDescriptor: DeclarationDescriptor, - val context: Context) { + val parentDescriptor: DeclarationDescriptor, + val context: Context) { private val descriptorSubstituteMap: MutableMap = mutableMapOf() private var typeSubstitutor: TypeSubstitutor? = null @@ -776,96 +772,13 @@ internal class DeepCopyIrTreeWithDescriptors(val targetDescriptor: FunctionDescr class SubstitutedDescriptor(val inlinedFunction: FunctionDescriptor, val descriptor: DeclarationDescriptor) -/** - * Searches recursively for value that is corresponds to [key], - * then to typeMapper(this[key]) and so on until typeMapper or Map.get returns null. - * Returns null if there is no corresponding value for given [key] inside the map - * Throws IllegalStateException if recursion led to cycle. - */ -private fun Map.deepSearch(key: K, typeMapper: (V) -> K?): V? { - var result: V? = null - // Prevent cycles by remembering visited keys. - val visitedKeys = mutableSetOf() - var keyToVisit = key - do { - if (keyToVisit in visitedKeys) { - throw IllegalStateException("Detected cycle inside $this") - } else { - visitedKeys += keyToVisit - } - val value = this[keyToVisit] - if (value != null) { - result = value - keyToVisit = typeMapper(value) ?: return result - } - } while (value != null) - return result -} - -/** - * Maps old variables to new ones that have fixed type. - */ -private fun createVariableSubstitutionMap(element: IrElement, - globalSubstituteMap: Map) - : Map { - - val variableSubstituteMap = mutableMapOf() - - element.acceptChildrenVoid(object: IrElementVisitorVoidWithContext() { - override fun visitElement(element: IrElement) { - element.acceptChildrenVoid(this) - } - - override fun visitVariable(declaration: IrVariable) { - declaration.acceptChildrenVoid(this) - - val oldDescriptor = declaration.descriptor - val oldClassDescriptor = oldDescriptor.type.constructor.declarationDescriptor as? ClassDescriptor - - // This recursive search inside the map requires some explanation. - // Take a look at the following code: - // - // inline fun exec(f: () -> Unit) = f() - // - // inline fun test2() { - // val sr = object {} // <- problematic place - // } - // - // fun box() { - // exec { - // test2() - // } - // } - // - // Here test2 will be inlined twice, so descriptor of the object will be changed twice (ex. from A to B and from B to C). - // If we use ordinary access to the map (globalSubstituteMap[A]) then we get wrong descriptor (B). - // So we have to search recursively inside the map to find correct descriptor. - val typeMapper: (SubstitutedDescriptor) -> ClassDescriptor? = { it.descriptor as? ClassDescriptor } - val substitutedDescriptor = oldClassDescriptor?.let { globalSubstituteMap.deepSearch(it, typeMapper) } - if (substitutedDescriptor != null && allScopes.all { it.scope.scopeOwner != substitutedDescriptor.inlinedFunction }) { - val newDescriptor = IrTemporaryVariableDescriptorImpl( - containingDeclaration = oldDescriptor.containingDeclaration, - name = oldDescriptor.name, - outType = (substitutedDescriptor.descriptor as ClassDescriptor).defaultType, - isMutable = oldDescriptor.isVar) - variableSubstituteMap[oldDescriptor] = newDescriptor - } - } - }) - - return variableSubstituteMap -} - internal class DescriptorSubstitutorForExternalScope( val globalSubstituteMap: Map, val context: Context ) : IrElementTransformerVoidWithContext() { - private val variableSubstituteMap = mutableMapOf() - fun run(element: IrElement) { - variableSubstituteMap += createVariableSubstitutionMap(element, globalSubstituteMap) element.transformChildrenVoid(this) } @@ -883,41 +796,6 @@ internal class DescriptorSubstitutorForExternalScope( } } - //---------------------------------------------------------------------// - - override fun visitVariable(declaration: IrVariable): IrDeclaration { - declaration.transformChildrenVoid(this) - - val oldDescriptor = declaration.descriptor - val newDescriptor = variableSubstituteMap[oldDescriptor] ?: return declaration - - return IrVariableImpl( - startOffset = declaration.startOffset, - endOffset = declaration.endOffset, - origin = declaration.origin, - descriptor = newDescriptor, - type = context.ir.translateErased(newDescriptor.type), - initializer = declaration.initializer - ) - } - - //-------------------------------------------------------------------------// - - override fun visitGetValue(expression: IrGetValue): IrExpression { - expression.transformChildrenVoid(this) - - val oldDescriptor = expression.descriptor - val newDescriptor = variableSubstituteMap[oldDescriptor] ?: return expression - - return IrGetValueImpl( - startOffset = expression.startOffset, - endOffset = expression.endOffset, - type = context.ir.translateErased(newDescriptor.type), - origin = expression.origin, - symbol = createValueSymbol(newDescriptor) - ) - } - //-------------------------------------------------------------------------// private fun copyIrCallImpl(oldExpression: IrCallImpl, substitutedDescriptor: SubstitutedDescriptor): IrCallImpl { diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DFGBuilder.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DFGBuilder.kt index 00f9a533bc7..7c1ba9dffbf 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DFGBuilder.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DFGBuilder.kt @@ -573,7 +573,7 @@ internal class ModuleDFGBuilder(val context: Context, val irModule: IrModuleFrag DataFlowIR.Node.NewObject( symbolTable.mapFunction(callee), arguments, - symbolTable.mapClassReferenceType(callee.constructedClass), + symbolTable.mapClassReferenceType(callee.constructedClass, false), value ) } else { diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DataFlowIR.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DataFlowIR.kt index f4c7810779f..2fb16fa8701 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DataFlowIR.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DataFlowIR.kt @@ -488,11 +488,14 @@ internal object DataFlowIR { private fun ClassDescriptor.isFinal() = modality == Modality.FINAL && kind != ClassKind.ENUM_CLASS - fun mapClassReferenceType(descriptor: ClassDescriptor): Type { + fun mapClassReferenceType(descriptor: ClassDescriptor, eraseLocalObjects: Boolean = true): Type { // Do not try to devirtualize ObjC classes. if (descriptor.module.name == Name.special("") || descriptor.isObjCClass()) return Type.Virtual + if (eraseLocalObjects && descriptor.isAnonymousObject && descriptor.isLocal) + return mapClassReferenceType(descriptor.getSuperClassNotAny() ?: context.irBuiltIns.anyClass.owner) + val isFinal = descriptor.isFinal() val isAbstract = descriptor.isAbstract() val name = descriptor.fqNameSafe.asString() @@ -531,7 +534,7 @@ internal object DataFlowIR { return erasure.singleOrNull { !it.isInterface } ?: context.ir.symbols.any.owner } - fun mapPrimitiveBinaryType(primitiveBinaryType: PrimitiveBinaryType): Type = + private fun mapPrimitiveBinaryType(primitiveBinaryType: PrimitiveBinaryType): Type = primitiveMap.getOrPut(primitiveBinaryType) { Type.Public( primitiveBinaryType.ordinal.toLong(), @@ -544,11 +547,11 @@ internal object DataFlowIR { ) } - fun mapType(type: IrType): Type { + fun mapType(type: IrType, eraseLocalObjects: Boolean = true): Type { val binaryType = type.computeBinaryType() return when (binaryType) { is BinaryType.Primitive -> mapPrimitiveBinaryType(binaryType.type) - is BinaryType.Reference -> mapClassReferenceType(choosePrimary(binaryType.types.toList())) + is BinaryType.Reference -> mapClassReferenceType(choosePrimary(binaryType.types.toList()), eraseLocalObjects) } } diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 5dee8a3701d..2395729e749 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -2571,6 +2571,11 @@ task inline_twiceInlinedObject(type: RunKonanTest) { source = "codegen/inline/twiceInlinedObject.kt" } +task inline_localObjectReturnedFromWhen(type: RunKonanTest) { + goldValue = "Ok\n" + source = "codegen/inline/localObjectReturnedFromWhen.kt" +} + task classDeclarationInsideInline(type: RunKonanTest) { source = "codegen/inline/classDeclarationInsideInline.kt" goldValue = "test1: 1.0\n1\ntest2\n" diff --git a/backend.native/tests/codegen/inline/localObjectReturnedFromWhen.kt b/backend.native/tests/codegen/inline/localObjectReturnedFromWhen.kt new file mode 100644 index 00000000000..ec7266de504 --- /dev/null +++ b/backend.native/tests/codegen/inline/localObjectReturnedFromWhen.kt @@ -0,0 +1,21 @@ +/* + * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the LICENSE file. + */ + +package codegen.inline.localObjectReturnedFromWhen + +import kotlin.test.* + +fun foo() { + 123?.let { + object : () -> Unit { + override fun invoke() = Unit + } + } +} + +@Test fun runTest() { + foo() + println("Ok") +} \ No newline at end of file