From 61fc3e99b12394b523f0951b66a6ac94a58538bd Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Tue, 24 Jan 2023 21:14:56 +0100 Subject: [PATCH] [K/N] Fix generics in FunctionReferenceLowering for more edge cases There were still some cases where the generated function reference class would contain references to generic parameters that were not in scope. One example of that was `Array<*>::get`. Before this fix the generated function reference class would still contain a reference to the type parameter declared in the `Array` class. Also, the logic of generifying the generated function reference class was incorrect. For example, for function `fun foo()` if we had a reference `::foo` (where `T` comes from the outer scope), the class would still contain two generic parameter, despite that only one generic parameter from the outer scope was referenced. This is also fixed here. --- .../konan/lower/FunctionReferenceLowering.kt | 55 +++++--- .../backend.native/tests/build.gradle | 5 + .../filecheck/generic_function_references.kt | 124 ++++++++++++++++++ 3 files changed, 163 insertions(+), 21 deletions(-) create mode 100644 kotlin-native/backend.native/tests/filecheck/generic_function_references.kt diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/FunctionReferenceLowering.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/FunctionReferenceLowering.kt index f1a2dfe9d1b..9702bd6db15 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/FunctionReferenceLowering.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/FunctionReferenceLowering.kt @@ -211,7 +211,7 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt /** * The first element of a pair is a type parameter of [referencedFunction], the second element is its argument in - * [functionReference], to be passed upon instantiation of the function reference class. + * [functionReference]. */ private val allTypeParametersAndArguments: List> = referencedFunction.typeParameters.map { typeParam -> @@ -224,16 +224,11 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt private val allTypeParametersAndArgumentsMap: Map = allTypeParametersAndArguments.toMap() /** - * The type arguments of [functionReference] that are not concrete types, + * The distinct type arguments of [functionReference] that are not concrete types, * but are themselves type parameters coming from an enclosing scope. - * - * The first element of a pair is a type parameter of [referencedFunction], the second element is its argument in - * [functionReference]. - * - * [functionReferenceClass] is only generic over these type parameters. */ - private val typeParametersAndArgumentsFromEnclosingScope: List> = - allTypeParametersAndArguments.filter { it.second.isTypeParameter() } + private val typeParametersFromEnclosingScope: List = allTypeParametersAndArguments + .mapNotNull { (_, typeArgument) -> (typeArgument.classifierOrNull as? IrTypeParameterSymbol)?.owner }.distinct() private val functionReferenceClass = irFactory.buildClass { startOffset = this@FunctionReferenceBuilder.startOffset @@ -245,7 +240,7 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt parent = this@FunctionReferenceBuilder.parent // The function reference class only needs to be generic over type parameters coming from an enclosing scope. - copyTypeParameters(typeParametersAndArgumentsFromEnclosingScope.map { it.first.owner }) + copyTypeParameters(typeParametersFromEnclosingScope) createParameterDeclarations() // copy the generated name for IrClass, partially solves KT-47194 @@ -253,20 +248,38 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt } /** - * Remaps type parameters of [referencedFunction] to type parameters of [functionReferenceClass]. - * - * Note: this is not a 1-to-1 mapping. Some type parameters of [referencedFunction] may be bound to a concrete type, in this case - * there will be no corresponding type parameter in [functionReferenceClass]. + * Remaps [typeParametersFromEnclosingScope] to type parameters of [functionReferenceClass]. */ private val typeParameterRemapper = IrTypeParameterRemapper( - typeParametersAndArgumentsFromEnclosingScope.map { it.first.owner }.zip(functionReferenceClass.typeParameters).toMap() + typeParametersFromEnclosingScope.zip(functionReferenceClass.typeParameters).toMap() ) - private val functionParameterTypes = unboundFunctionParameters.map { typeParameterRemapper.remapType(it.type) } - private val functionReturnType = typeParameterRemapper.remapType(referencedFunction.returnType) + private val functionParameterAndReturnTypes = (functionReference.type as IrSimpleType).arguments.map { + when (it) { + is IrTypeProjection -> typeParameterRemapper.remapType(it.type) + is IrStarProjection -> context.irBuiltIns.anyNType + } + } + + private val functionParameterTypes = functionParameterAndReturnTypes.dropLast(1) + private val functionReturnType = functionParameterAndReturnTypes.last() private val functionReferenceThis = functionReferenceClass.thisReceiver!! + /** + * Replaces [typeParameter] of [referencedFunction] with the corresponding type parameter of [functionReferenceClass] + * if such correspondence takes place. Otherwise, just returns [typeParameter] as [IrType]. + */ + private fun substituteTypeParameterOfReferencedFunction(typeParameter: IrTypeParameter): IrType { + if (typeParameter.parent != referencedFunction) { + compilationException( + "The type parameter ${typeParameter.render()} is not defined in the referenced function ${referencedFunction.render()}", + functionReference + ) + } + return typeParameterRemapper.remapType(allTypeParametersAndArguments[typeParameter.index].second) + } + /** * Substitutes a bound value parameter's [type] with a new type according to the following rules: * @@ -278,7 +291,8 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt * - Otherwise, just return [type] itself. */ private fun substituteBoundValueParameterType(type: IrType): IrType = - typeParameterRemapper.remapType(type).substitute(allTypeParametersAndArgumentsMap) + ((type.classifierOrNull as? IrTypeParameterSymbol)?.owner?.let(this::substituteTypeParameterOfReferencedFunction) ?: type) + .substitute(allTypeParametersAndArgumentsMap) private val argumentToPropertiesMap = boundFunctionParameters.associateWith { functionReferenceClass.addField { @@ -321,7 +335,6 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt transformedSuperMethod = samSuperClass.functions.single { it.owner.modality == Modality.ABSTRACT }.owner } else { val numberOfParameters = unboundFunctionParameters.size - val functionParameterAndReturnTypes = functionParameterTypes + functionReturnType if (isKSuspendFunction) { val suspendFunctionClass = symbols.kSuspendFunctionN(numberOfParameters).owner superTypes += suspendFunctionClass.typeWith(functionParameterAndReturnTypes) @@ -434,7 +447,7 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt val clazz = buildClass() val constructor = buildConstructor() val arguments = functionReference.getArgumentsWithIr() - val typeArguments = typeParametersAndArgumentsFromEnclosingScope.map { it.second } + val typeArguments = typeParametersFromEnclosingScope.map { it.defaultType } val expression = if (arguments.isEmpty()) { irBuilder.irConstantObject(clazz, emptyMap(), typeArguments) } else { @@ -538,7 +551,7 @@ internal class FunctionReferenceLowering(val generationState: NativeGenerationSt assert(unboundIndex == valueParameters.size) { "Not all arguments of are used" } referencedFunction.typeParameters.forEach { typeParam -> - putTypeArgument(typeParam.index, substituteBoundValueParameterType(typeParam.defaultType)) + putTypeArgument(typeParam.index, substituteTypeParameterOfReferencedFunction(typeParam)) } } ) diff --git a/kotlin-native/backend.native/tests/build.gradle b/kotlin-native/backend.native/tests/build.gradle index df580f4ca09..5c26cddd7f1 100644 --- a/kotlin-native/backend.native/tests/build.gradle +++ b/kotlin-native/backend.native/tests/build.gradle @@ -6597,6 +6597,11 @@ fileCheckTest("filecheck_bce") { annotatedSource = project.file('filecheck/bce.kt') } +fileCheckTest("filecheck_generic_function_references") { + enabled = project.globalTestArgs.contains('-opt') + annotatedSource = project.file('filecheck/generic_function_references.kt') +} + fileCheckTest("filecheck_adopted_function_reference") { annotatedSource = project.file('filecheck/adopted_function_reference.kt') } diff --git a/kotlin-native/backend.native/tests/filecheck/generic_function_references.kt b/kotlin-native/backend.native/tests/filecheck/generic_function_references.kt new file mode 100644 index 00000000000..54dbc58cc0c --- /dev/null +++ b/kotlin-native/backend.native/tests/filecheck/generic_function_references.kt @@ -0,0 +1,124 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +import kotlin.reflect.KFunction2 + +fun stringify(collection: StringifyTP, size: (StringifyTP) -> Int, get: StringifyTP.(Int) -> Any?): String { + var res = "[" + for (i in 0 until size(collection)) { + if (i > 0) res += ", " + res += collection.get(i).toString() + } + res += "]" + return res +} + +interface I + +// CHECK-LABEL: define %struct.ObjHeader* @"kfun:#stringifyArray(kotlin.Array<0:0>){0\C2\A7}kotlin.String" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader** {{%[0-9]+}}) +fun stringifyArray(array: Array) = + // CHECK: call %struct.ObjHeader* @"kfun:#stringify(0:0;kotlin.Function1<0:0,kotlin.Int>;kotlin.Function2<0:0,kotlin.Int,kotlin.Any?>){0\C2\A7}kotlin.String" + stringify( + array, + { it.size }, // $stringifyArray$lambda$0$FUNCTION_REFERENCE$0 + Array<*>::get // $get$FUNCTION_REFERENCE$1 + ) + +// CHECK-LABEL: define %struct.ObjHeader* @"kfun:#stringifyIntArray(kotlin.Array){}kotlin.String" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader** {{%[0-9]+}}) +fun stringifyIntArray(array: Array) = + // CHECK: call %struct.ObjHeader* @"kfun:#stringify(0:0;kotlin.Function1<0:0,kotlin.Int>;kotlin.Function2<0:0,kotlin.Int,kotlin.Any?>){0\C2\A7}kotlin.String" + stringify( + array, + { it.size }, // $stringifyIntArray$lambda$1$FUNCTION_REFERENCE$2 + Array::get // $get$FUNCTION_REFERENCE$3 + ) + +class N(val v: Int) : I { + override fun toString() = v.toString() +} + +@Suppress("UNUSED_PARAMETER") +fun foo(p1: BazTP0, p2: BazTP1) {} + +fun bar() { + val ref: KFunction2 = ::foo // $foo$FUNCTION_REFERENCE$4 + println(ref) +} + +fun main() { + println(stringifyArray(arrayOf(N(2), N(14)))) + println(stringifyIntArray(arrayOf(1, 2, 3))) + + bar() + bar() + + val ref: KFunction2 = ::foo // $foo$FUNCTION_REFERENCE$5 + println(ref) +} + +// CHECK-LABEL: define internal i32 @"kfun:$stringifyArray$lambda$0$FUNCTION_REFERENCE$0.invoke#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal void @"kfun:$stringifyArray$lambda$0$FUNCTION_REFERENCE$0.#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal %struct.ObjHeader* @"kfun:$stringifyArray$lambda$0$FUNCTION_REFERENCE$0.$invoke(kotlin.Array<1:0>){}kotlin.Int#internal" +// CHECK-SAME: (%struct.ObjHeader* [[this:%[0-9]+]], %struct.ObjHeader* [[array:%[0-9]+]], %struct.ObjHeader** {{%[0-9]+}}) +// CHECK: call i32 @"kfun:$stringifyArray$lambda$0$FUNCTION_REFERENCE$0.invoke#internal"(%struct.ObjHeader* [[this]], %struct.ObjHeader* [[array]]) + +// CHECK-LABEL: define internal %struct.ObjHeader* @"kfun:$get$FUNCTION_REFERENCE$1.invoke#internal" +// CHECK-SAME: (%struct.ObjHeader* [[this:%[0-9]+]], %struct.ObjHeader* [[array:%[0-9]+]], i32 [[index:%[0-9]+]], %struct.ObjHeader** [[ret:%[0-9]+]]) +// CHECK: call %struct.ObjHeader* @Kotlin_Array_get(%struct.ObjHeader* [[array]], i32 [[index]], %struct.ObjHeader** [[ret]]) + +// CHECK-LABEL: define internal void @"kfun:$get$FUNCTION_REFERENCE$1.#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal %struct.ObjHeader* @"kfun:$get$FUNCTION_REFERENCE$1.$invoke(kotlin.Array<*>;kotlin.Int){}kotlin.Any?#internal" +// CHECK-SAME: (%struct.ObjHeader* [[this:%[0-9]+]], %struct.ObjHeader* [[array:%[0-9]+]], %struct.ObjHeader* [[boxedIndex:%[0-9]+]], %struct.ObjHeader** [[ret:%[0-9]+]]) +// CHECK: call %struct.ObjHeader* @"kfun:$get$FUNCTION_REFERENCE$1.invoke#internal"(%struct.ObjHeader* [[this]], %struct.ObjHeader* [[array]], i32 {{%[0-9]+}}, %struct.ObjHeader** [[ret]]) + +// CHECK-LABEL: define internal i32 @"kfun:$stringifyIntArray$lambda$1$FUNCTION_REFERENCE$2.invoke#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal void @"kfun:$stringifyIntArray$lambda$1$FUNCTION_REFERENCE$2.#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal %struct.ObjHeader* @"kfun:$stringifyIntArray$lambda$1$FUNCTION_REFERENCE$2.$invoke(kotlin.Array){}kotlin.Int#internal" +// CHECK-SAME: (%struct.ObjHeader* [[this:%[0-9]+]], %struct.ObjHeader* [[array:%[0-9]+]], %struct.ObjHeader** {{%[0-9]+}}) +// CHECK: call i32 @"kfun:$stringifyIntArray$lambda$1$FUNCTION_REFERENCE$2.invoke#internal"(%struct.ObjHeader* [[this]], %struct.ObjHeader* [[array]]) + +// CHECK-LABEL: define internal i32 @"kfun:$get$FUNCTION_REFERENCE$3.invoke#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader* {{%[0-9]+}}, i32 {{%[0-9]+}}) + +// CHECK-LABEL: define internal void @"kfun:$get$FUNCTION_REFERENCE$3.#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal %struct.ObjHeader* @"kfun:$get$FUNCTION_REFERENCE$3.$invoke(kotlin.Array;kotlin.Int){}kotlin.Int#internal" +// CHECK-SAME: (%struct.ObjHeader* [[this:%[0-9]+]], %struct.ObjHeader* [[array:%[0-9]+]], %struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader** {{%[0-9]+}}) +// CHECK: call i32 @"kfun:$get$FUNCTION_REFERENCE$3.invoke#internal"(%struct.ObjHeader* [[this]], %struct.ObjHeader* [[array]], i32 {{%[0-9]+}}) + +// CHECK-LABEL: define internal void @"kfun:$foo$FUNCTION_REFERENCE$4.invoke#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader* [[p1:%[0-9]+]], %struct.ObjHeader* [[p2:%[0-9]+]]) +// CHECK: call void @"kfun:#foo(0:0;0:1){0\C2\A7;1\C2\A7}"(%struct.ObjHeader* [[p1]], %struct.ObjHeader* [[p2]]) + +// CHECK-LABEL: define internal void @"kfun:$foo$FUNCTION_REFERENCE$4.#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal %struct.ObjHeader* @"kfun:$foo$FUNCTION_REFERENCE$4.$invoke(1:0;1:0){}#internal" +// CHECK-SAME: (%struct.ObjHeader* [[this:%[0-9]+]], %struct.ObjHeader* [[p1:%[0-9]+]], %struct.ObjHeader* [[p2:%[0-9]+]], %struct.ObjHeader** {{%[0-9]+}}) +// CHECK: call void @"kfun:$foo$FUNCTION_REFERENCE$4.invoke#internal"(%struct.ObjHeader* [[this]], %struct.ObjHeader* [[p1]], %struct.ObjHeader* [[p2]]) + +// CHECK-LABEL: define internal void @"kfun:$foo$FUNCTION_REFERENCE$5.invoke#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 {{%[0-9]+}}) +// CHECK: call void @"kfun:#foo(0:0;0:1){0\C2\A7;1\C2\A7}"(%struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal void @"kfun:$foo$FUNCTION_REFERENCE$5.#internal" +// CHECK-SAME: (%struct.ObjHeader* {{%[0-9]+}}) + +// CHECK-LABEL: define internal %struct.ObjHeader* @"kfun:$foo$FUNCTION_REFERENCE$5.$invoke(kotlin.Int;kotlin.Int){}#internal" +// CHECK-SAME: (%struct.ObjHeader* [[this:%[0-9]+]], %struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader* {{%[0-9]+}}, %struct.ObjHeader** {{%[0-9]+}}) +// CHECK: call void @"kfun:$foo$FUNCTION_REFERENCE$5.invoke#internal"(%struct.ObjHeader* [[this]], i32 {{%[0-9]+}}, i32 {{%[0-9]+}})