From 61dc49b4b7b7fe7eedbdb540f9a0bef30a5bb090 Mon Sep 17 00:00:00 2001 From: SvyatoslavScherbina Date: Mon, 18 Dec 2017 12:29:09 +0300 Subject: [PATCH] Fix more `-produce framework` bugs (#1152) * Support array constructors as factory methods when producing framework #KT-21802 Fixed * Improve name mangling for produced framework * Improve multi-inheritance handling when producing framework #KT-21817 Fixed #KT-21836 Fixed --- .../objcexport/ObjCExportCodeGenerator.kt | 118 +++++++++++++++--- .../objcexport/ObjCExportHeaderGenerator.kt | 17 ++- .../konan/objcexport/ObjCExportMapper.kt | 23 +++- .../konan/objcexport/ObjCExportNamer.kt | 19 ++- runtime/src/main/cpp/ObjCExport.mm | 2 +- 5 files changed, 139 insertions(+), 40 deletions(-) diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt index 92b10b727a7..a27fff35814 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.backend.konan.llvm.objcexport import llvm.* +import org.jetbrains.kotlin.backend.common.atMostOne import org.jetbrains.kotlin.backend.common.descriptors.allParameters import org.jetbrains.kotlin.backend.konan.* import org.jetbrains.kotlin.backend.konan.descriptors.* @@ -222,6 +223,8 @@ internal class ObjCExportCodeGenerator( private val impType = pointerType(functionType(int8TypePtr, true, int8TypePtr, int8TypePtr)) + internal val directMethodAdapters = mutableMapOf() + inner class ObjCToKotlinMethodAdapter( selector: String, encoding: String, @@ -482,6 +485,41 @@ private fun ObjCExportCodeGenerator.generateObjCImp( return result } +private fun ObjCExportCodeGenerator.generateObjCImpForArrayConstructor( + target: ConstructorDescriptor, + methodBridge: MethodBridge +): LLVMValueRef { + // TODO: adapt exceptions. + + val result = LLVMAddFunction(context.llvmModule, "", objCFunctionType(methodBridge))!! + + generateFunction(codegen, result) { + callFromBridge(context.llvm.initRuntimeIfNeeded, emptyList()) + + val kotlinValueArgs = methodBridge.paramBridges + .drop(1) // Drop class method receiver. + .mapIndexed { index, typeBridge -> + objCToKotlin(param(index + 2), typeBridge, Lifetime.ARGUMENT) + } + + val arrayInstance = callFromBridge( + context.llvm.allocArrayFunction, + listOf(target.constructedClass.llvmTypeInfoPtr, kotlinValueArgs.first()), + resultLifetime = Lifetime.ARGUMENT + ) + callFromBridge( + target.llvmFunction, + listOf(arrayInstance) + kotlinValueArgs + ) + + ret(kotlinToObjC(arrayInstance, ReferenceBridge)) + } + + LLVMSetLinkage(result, LLVMLinkage.LLVMPrivateLinkage) + + return result +} + // TODO: cache bridges. private fun ObjCExportCodeGenerator.generateKotlinToObjCBridge( descriptor: FunctionDescriptor, @@ -585,19 +623,35 @@ private fun ObjCExportCodeGenerator.createMethodVirtualAdapter( private fun ObjCExportCodeGenerator.createMethodAdapter( implementation: FunctionDescriptor?, baseMethod: FunctionDescriptor -): ObjCExportCodeGenerator.ObjCToKotlinMethodAdapter { - val selectorName = namer.getSelector(baseMethod) - val methodBridge = mapper.bridgeMethod(baseMethod) - val objCEncoding = getEncoding(methodBridge) - val objCToKotlin = constPointer(generateObjCImp(implementation, methodBridge)) +) = createMethodAdapter(DirectAdapterRequest(implementation, baseMethod)) - return ObjCToKotlinMethodAdapter(selectorName, objCEncoding, objCToKotlin) +private fun ObjCExportCodeGenerator.createMethodAdapter( + request: DirectAdapterRequest +): ObjCExportCodeGenerator.ObjCToKotlinMethodAdapter = this.directMethodAdapters.getOrPut(request) { + + val selectorName = namer.getSelector(request.base) + val methodBridge = mapper.bridgeMethod(request.base) + val objCEncoding = getEncoding(methodBridge) + val objCToKotlin = constPointer(generateObjCImp(request.implementation, methodBridge)) + + ObjCToKotlinMethodAdapter(selectorName, objCEncoding, objCToKotlin) } private fun ObjCExportCodeGenerator.createConstructorAdapter( descriptor: ConstructorDescriptor ): ObjCExportCodeGenerator.ObjCToKotlinMethodAdapter = createMethodAdapter(descriptor, descriptor) +private fun ObjCExportCodeGenerator.createArrayConstructorAdapter( + descriptor: ConstructorDescriptor +): ObjCExportCodeGenerator.ObjCToKotlinMethodAdapter { + val selectorName = namer.getSelector(descriptor) + val methodBridge = mapper.bridgeMethod(descriptor) + val objCEncoding = getEncoding(methodBridge) + val objCToKotlin = constPointer(generateObjCImpForArrayConstructor(descriptor, methodBridge)) + + return ObjCToKotlinMethodAdapter(selectorName, objCEncoding, objCToKotlin) +} + private fun ObjCExportCodeGenerator.vtableIndex(descriptor: FunctionDescriptor): Int? { assert(descriptor.isOverridable) val classDescriptor = descriptor.containingDeclaration as ClassDescriptor @@ -634,10 +688,17 @@ private fun ObjCExportCodeGenerator.createTypeAdapter( descriptor: ClassDescriptor ): ObjCExportCodeGenerator.ObjCTypeAdapter { val adapters = mutableListOf() + val classAdapters = mutableListOf() if (descriptor != context.builtIns.any) { descriptor.constructors.forEach { - if (mapper.shouldBeExposed(it)) adapters += createConstructorAdapter(it) + if (mapper.shouldBeExposed(it)) { + if (it.constructedClass.isArray) { + classAdapters += createArrayConstructorAdapter(it) + } else { + adapters += createConstructorAdapter(it) + } + } } } @@ -645,15 +706,8 @@ private fun ObjCExportCodeGenerator.createTypeAdapter( val exposedMethods = descriptor.contributedMethods.filter { mapper.shouldBeExposed(it) } + categoryMethods - exposedMethods.filter { it.kind.isReal }.forEach { method -> - mapper.getBaseMethods(method).mapTo(adapters) { base -> - val implementation = if (method.modality == Modality.ABSTRACT) { - null - } else { - OverriddenFunctionDescriptor(method, base).getImplementation(context) - } - createMethodAdapter(implementation, base) - } + exposedMethods.forEach { + adapters += createDirectAdapters(it) } val reverseAdapters = mutableListOf() @@ -692,7 +746,7 @@ private fun ObjCExportCodeGenerator.createTypeAdapter( } else { // Mark it as non-overridable: - baseMethods.forEach { base -> + baseMethods.distinctBy { namer.getSelector(it) }.forEach { base -> reverseAdapters += KotlinToObjCMethodAdapter( namer.getSelector(base), -1, @@ -732,8 +786,6 @@ private fun ObjCExportCodeGenerator.createTypeAdapter( emptyList() } - val classAdapters = mutableListOf() - if (descriptor.isUnit()) { classAdapters += createUnitInstanceAdapter() } @@ -752,6 +804,34 @@ private fun ObjCExportCodeGenerator.createTypeAdapter( ) } +internal data class DirectAdapterRequest(val implementation: FunctionDescriptor?, val base: FunctionDescriptor) + +private fun ObjCExportCodeGenerator.createDirectAdapters( + method: FunctionDescriptor +): List { + + fun FunctionDescriptor.getAllRequiredDirectAdapters() = mapper.getBaseMethods(this).map { base -> + val implementation = if (this.modality == Modality.ABSTRACT) { + null + } else { + OverriddenFunctionDescriptor(this, base).getImplementation(context) + } + DirectAdapterRequest(implementation, base) + } + + val superClassMethod = method.overriddenDescriptors + .atMostOne { !(it.containingDeclaration as ClassDescriptor).isInterface } + + val inheritedAdapters = superClassMethod?.getAllRequiredDirectAdapters().orEmpty() + val requiredAdapters = method.getAllRequiredDirectAdapters() + + return (requiredAdapters - inheritedAdapters) + .distinctBy { namer.getSelector(it.base) } + .map { + createMethodAdapter(it) + } +} + private fun ObjCExportCodeGenerator.createUnitInstanceAdapter(): ObjCExportCodeGenerator.ObjCToKotlinMethodAdapter { val selector = "unit" val methodBridge = MethodBridge(ReferenceBridge, listOf(ReferenceBridge, ReferenceBridge)) diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportHeaderGenerator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportHeaderGenerator.kt index edb6ef869ff..173353bd42e 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportHeaderGenerator.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportHeaderGenerator.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.backend.konan.objcexport import org.jetbrains.kotlin.backend.konan.Context import org.jetbrains.kotlin.backend.konan.KonanCompilationException import org.jetbrains.kotlin.backend.konan.descriptors.getPackageFragments +import org.jetbrains.kotlin.backend.konan.descriptors.isArray import org.jetbrains.kotlin.backend.konan.descriptors.isInterface import org.jetbrains.kotlin.backend.konan.descriptors.isUnit import org.jetbrains.kotlin.backend.konan.reportCompilationError @@ -210,12 +211,18 @@ internal class ObjCExportHeaderGenerator(val context: Context) { val presentConstructors = mutableSetOf() descriptor.constructors.filter { mapper.shouldBeExposed(it) }.forEach { - presentConstructors += getSelector(it) + if (!descriptor.isArray) presentConstructors += getSelector(it) +"${getSignature(it, it)};" +"" } + if (descriptor.isArray) { + +"+(instancetype)alloc __attribute__((unavailable));" + +"+(instancetype)allocWithZone:(struct _NSZone *)zone __attribute__((unavailable));" + +"" + } + // Hide "unimplemented" super constructors: superClass?.constructors?.filter { mapper.shouldBeExposed(it) }?.forEach { if (getSelector(it) !in presentConstructors) { @@ -320,8 +327,7 @@ internal class ObjCExportHeaderGenerator(val context: Context) { val propertySetter = property.setter if (propertySetter != null && mapper.shouldBeExposed(propertySetter)) { - val baseSetter = mapper.getBaseMethods(propertySetter).single() - val setterSelector = getSelector(baseSetter) + val setterSelector = mapper.getBaseMethods(propertySetter).map { namer.getSelector(it) }.distinct().single() if (setterSelector != "set" + name.capitalize() + ":") { attributes += "setter=$setterSelector" } @@ -342,7 +348,8 @@ internal class ObjCExportHeaderGenerator(val context: Context) { val selectorParts = getSelector(baseMethod).split(':') - if (methodBridge.isKotlinTopLevel) { + val isArrayConstructor = method is ConstructorDescriptor && method.constructedClass.isArray + if (methodBridge.isKotlinTopLevel || isArrayConstructor) { append("+") } else { append("-") @@ -392,7 +399,7 @@ internal class ObjCExportHeaderGenerator(val context: Context) { append(" NS_SWIFT_NAME($swiftName)") - if (method is ConstructorDescriptor) { + if (method is ConstructorDescriptor && !isArrayConstructor) { append(" NS_DESIGNATED_INITIALIZER") } } diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportMapper.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportMapper.kt index 783e93167af..5819374c8ab 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportMapper.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportMapper.kt @@ -16,10 +16,11 @@ package org.jetbrains.kotlin.backend.konan.objcexport -import org.jetbrains.kotlin.backend.common.descriptors.allParameters +import org.jetbrains.kotlin.backend.common.descriptors.explicitParameters import org.jetbrains.kotlin.backend.common.descriptors.isSuspend import org.jetbrains.kotlin.backend.konan.ValueType import org.jetbrains.kotlin.backend.konan.correspondingValueType +import org.jetbrains.kotlin.backend.konan.descriptors.isArray import org.jetbrains.kotlin.backend.konan.isObjCObjectType import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns @@ -67,7 +68,9 @@ internal fun ObjCExportMapper.getBaseMethods(descriptor: FunctionDescriptor): Li if (isBaseMethod(descriptor)) { listOf(descriptor) } else { - descriptor.overriddenDescriptors.filter { shouldBeExposed(it) }.flatMap { getBaseMethods(it.original)} + descriptor.overriddenDescriptors.filter { shouldBeExposed(it) } + .flatMap { getBaseMethods(it.original)} + .distinct() } internal fun ObjCExportMapper.isBaseProperty(descriptor: PropertyDescriptor) = @@ -77,7 +80,9 @@ internal fun ObjCExportMapper.getBaseProperties(descriptor: PropertyDescriptor): if (isBaseProperty(descriptor)) { listOf(descriptor) } else { - descriptor.overriddenDescriptors.flatMap { getBaseProperties(it.original) } + descriptor.overriddenDescriptors + .flatMap { getBaseProperties(it.original) } + .distinct() } internal tailrec fun KotlinType.getErasedTypeClass(): ClassDescriptor = @@ -151,12 +156,22 @@ internal fun ObjCExportMapper.bridgeMethod(descriptor: FunctionDescriptor): Meth val paramBridges = mutableListOf() + if (descriptor is ConstructorDescriptor) { + if (descriptor.constructedClass.isArray) { + // Generated as class factory method. + paramBridges += ReferenceBridge // Receiver of class method. + } else { + // Generated as Objective-C instance init method. + paramBridges += ReferenceBridge // Receiver of init method. + } + } + val isTopLevel = isTopLevel(descriptor) if (isTopLevel) { paramBridges += ReferenceBridge } - descriptor.allParameters.mapTo(paramBridges) { bridgeType(it.type) } + descriptor.explicitParameters.mapTo(paramBridges) { bridgeType(it.type) } return MethodBridge(returnBridge, paramBridges, isKotlinTopLevel = isTopLevel) } diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportNamer.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportNamer.kt index 7d1baf44e64..99b174dd5bc 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportNamer.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportNamer.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.backend.konan.objcexport import org.jetbrains.kotlin.backend.konan.Context +import org.jetbrains.kotlin.backend.konan.descriptors.isArray import org.jetbrains.kotlin.backend.konan.descriptors.isInterface import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.incremental.components.NoLookupLocation @@ -94,7 +95,7 @@ internal class ObjCExportNamer(val context: Context, val mapper: ObjCExportMappe val parameters = mapper.objCValueParameters(method) StringBuilder().apply { - append(method.mangledName) + append(method.getMangledName(forSwift = false)) parameters.forEachIndexed { index, it -> val name = when { @@ -132,7 +133,7 @@ internal class ObjCExportNamer(val context: Context, val mapper: ObjCExportMappe val parameters = mapper.objCValueParameters(method) StringBuilder().apply { - append(method.mangledName) + append(method.getMangledName(forSwift = true)) append("(") parameters.forEach { @@ -150,13 +151,9 @@ internal class ObjCExportNamer(val context: Context, val mapper: ObjCExportMappe append(")") }.mangledSequence { - if (parameters.isNotEmpty()) { - // "foo(label:)" -> "foo(label_:)" - insert(lastIndex - 1, '_') - } else { - // "foo()" -> "foo_()" - insert(lastIndex - 2, '_') - } + // "foo(label:)" -> "foo(label_:)" + // "foo()" -> "foo_()" + insert(lastIndex - 1, '_') } } @@ -196,9 +193,9 @@ internal class ObjCExportNamer(val context: Context, val mapper: ObjCExportMappe methodSwiftNames.forceAssign(equals, "isEqual(:)") } - private val FunctionDescriptor.mangledName: String get() { + private fun FunctionDescriptor.getMangledName(forSwift: Boolean): String { if (this is ConstructorDescriptor) { - return "init" + return if (this.constructedClass.isArray && !forSwift) "array" else "init" } val candidate = when (this) { diff --git a/runtime/src/main/cpp/ObjCExport.mm b/runtime/src/main/cpp/ObjCExport.mm index 75a8f6f1fb0..c72b2f4dce3 100644 --- a/runtime/src/main/cpp/ObjCExport.mm +++ b/runtime/src/main/cpp/ObjCExport.mm @@ -152,7 +152,7 @@ static void initializeClass(Class clazz); if (typeInfo->instanceSize_ < 0) { [NSException raise:NSGenericException - format:@"Allocating %s is not supported yet", + format:@"%s must be allocated and initialized with a factory method", class_getName(object_getClass(self))]; }