From 537188fce8967e88e122f0a0da5832ebf2b20e12 Mon Sep 17 00:00:00 2001 From: Svyatoslav Scherbina Date: Thu, 6 Feb 2020 16:37:09 +0300 Subject: [PATCH] Simplify calling primitive and nullable `@Throws` from Swift Prefer Swift code simplicity over following Obj-C conventions --- .../objcexport/ObjCExportCodeGenerator.kt | 62 +++++++------------ .../backend/konan/objcexport/MethodBridge.kt | 9 +-- .../objcexport/ObjCExportHeaderGenerator.kt | 33 +++++----- .../konan/objcexport/ObjCExportMapper.kt | 28 ++++++--- .../konan/objcexport/ObjCExportNamer.kt | 5 +- .../tests/framework/values/expectedLazy.h | 8 +-- .../tests/framework/values/values.swift | 6 +- 7 files changed, 67 insertions(+), 84 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 b6ec717e503..b898321880b 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 @@ -755,8 +755,6 @@ private fun ObjCExportCodeGenerator.generateObjCImp( } var errorOutPtr: LLVMValueRef? = null - var kotlinResultOutPtr: LLVMValueRef? = null - lateinit var kotlinResultOutBridge: TypeBridge val kotlinArgs = methodBridge.paramBridges.mapIndexedNotNull { index, paramBridge -> val parameter = param(index) @@ -774,13 +772,6 @@ private fun ObjCExportCodeGenerator.generateObjCImp( errorOutPtr = parameter null } - - is MethodBridgeValueParameter.KotlinResultOutParameter -> { - assert(kotlinResultOutPtr == null) - kotlinResultOutPtr = parameter - kotlinResultOutBridge = paramBridge.bridge - null - } } } @@ -803,12 +794,12 @@ private fun ObjCExportCodeGenerator.generateObjCImp( MethodBridge.ReturnValue.WithError.Success -> Int8(0).llvm // false - is MethodBridge.ReturnValue.WithError.RefOrNull -> { + is MethodBridge.ReturnValue.WithError.ZeroForError -> { if (returnType.successBridge == MethodBridge.ReturnValue.Instance.InitResult) { // Release init receiver, as required by convention. callFromBridge(objcRelease, listOf(param(0))) } - kNullInt8Ptr + Zero(returnType.objCType(context)).llvm } } @@ -818,13 +809,6 @@ private fun ObjCExportCodeGenerator.generateObjCImp( val targetResult = callKotlin(kotlinArgs, Lifetime.ARGUMENT, exceptionHandler) - kotlinResultOutPtr?.let { - ifThen(icmpNe(it, LLVMConstNull(it.type)!!)) { - val objCResult = kotlinToObjC(targetResult!!, kotlinResultOutBridge) - store(objCResult, it) - } - } - tailrec fun genReturnValueOnSuccess(returnBridge: MethodBridge.ReturnValue): LLVMValueRef? = when (returnBridge) { MethodBridge.ReturnValue.Void -> null MethodBridge.ReturnValue.HashCode -> { @@ -833,7 +817,7 @@ private fun ObjCExportCodeGenerator.generateObjCImp( } is MethodBridge.ReturnValue.Mapped -> kotlinToObjC(targetResult!!, returnBridge.bridge) MethodBridge.ReturnValue.WithError.Success -> Int8(1).llvm // true - is MethodBridge.ReturnValue.WithError.RefOrNull -> genReturnValueOnSuccess(returnBridge.successBridge) + is MethodBridge.ReturnValue.WithError.ZeroForError -> genReturnValueOnSuccess(returnBridge.successBridge) MethodBridge.ReturnValue.Instance.InitResult -> param(0) MethodBridge.ReturnValue.Instance.FactoryResult -> kotlinReferenceToObjC(targetResult!!) // provided by [callKotlin] } @@ -904,8 +888,6 @@ private fun ObjCExportCodeGenerator.generateKotlinToObjCBridge( val result = generateFunction(codegen, functionType, "kotlin2objc") { var errorOutPtr: LLVMValueRef? = null - var kotlinResultOutPtr: LLVMValueRef? = null - lateinit var kotlinResultOutBridge: TypeBridge val parameters = irFunction.allParameters.mapIndexed { index, parameterDescriptor -> parameterDescriptor to param(index) @@ -937,12 +919,9 @@ private fun ObjCExportCodeGenerator.generateKotlinToObjCBridge( error("Method is not instance and thus can't have bridge for overriding: $baseMethod") MethodBridgeValueParameter.ErrorOutParameter -> - alloca(int8TypePtr).also { errorOutPtr = it } - - is MethodBridgeValueParameter.KotlinResultOutParameter -> - alloca(bridge.bridge.objCType).also { - kotlinResultOutPtr = it - kotlinResultOutBridge = bridge.bridge + alloca(int8TypePtr).also { + store(kNullInt8Ptr, it) + errorOutPtr = it } } } @@ -981,17 +960,20 @@ private fun ObjCExportCodeGenerator.generateKotlinToObjCBridge( ifThen(icmpEq(targetResult, Int8(0).llvm)) { rethrow() } - - kotlinResultOutPtr?.let { - objCToKotlin(load(it), kotlinResultOutBridge, lifetime) - } + null } - is MethodBridge.ReturnValue.WithError.RefOrNull -> { - ifThen(icmpEq(targetResult, kNullInt8Ptr)) { - rethrow() + is MethodBridge.ReturnValue.WithError.ZeroForError -> { + if (returnBridge.successMayBeZero) { + val error = load(errorOutPtr!!) + ifThen(icmpNe(error, kNullInt8Ptr)) { + rethrow() + } + } else { + ifThen(icmpEq(targetResult, kNullInt8Ptr)) { + rethrow() + } } - assert(kotlinResultOutPtr == null) genKotlinBaseMethodResult(lifetime, returnBridge.successBridge) } @@ -1446,7 +1428,6 @@ private val MethodBridgeParameter.objCType: LLVMTypeRef get() = when (this) { is MethodBridgeReceiver -> ReferenceBridge.objCType MethodBridgeSelector -> int8TypePtr MethodBridgeValueParameter.ErrorOutParameter -> pointerType(ReferenceBridge.objCType) - is MethodBridgeValueParameter.KotlinResultOutParameter -> pointerType(this.bridge.objCType) } private fun MethodBridge.ReturnValue.objCType(context: Context): LLVMTypeRef { @@ -1457,8 +1438,8 @@ private fun MethodBridge.ReturnValue.objCType(context: Context): LLVMTypeRef { MethodBridge.ReturnValue.WithError.Success -> ObjCValueType.BOOL.llvmType MethodBridge.ReturnValue.Instance.InitResult, - MethodBridge.ReturnValue.Instance.FactoryResult, - is MethodBridge.ReturnValue.WithError.RefOrNull -> ReferenceBridge.objCType + MethodBridge.ReturnValue.Instance.FactoryResult -> ReferenceBridge.objCType + is MethodBridge.ReturnValue.WithError.ZeroForError -> this.successBridge.objCType(context) } } @@ -1503,8 +1484,8 @@ private fun MethodBridge.ReturnValue.getObjCEncoding(targetFamily: Family): Stri MethodBridge.ReturnValue.WithError.Success -> ObjCValueType.BOOL.encoding MethodBridge.ReturnValue.Instance.InitResult, - MethodBridge.ReturnValue.Instance.FactoryResult, - is MethodBridge.ReturnValue.WithError.RefOrNull -> ReferenceBridge.objCEncoding + MethodBridge.ReturnValue.Instance.FactoryResult -> ReferenceBridge.objCEncoding + is MethodBridge.ReturnValue.WithError.ZeroForError -> this.successBridge.getObjCEncoding(targetFamily) } private val MethodBridgeParameter.objCEncoding: String get() = when (this) { @@ -1512,7 +1493,6 @@ private val MethodBridgeParameter.objCEncoding: String get() = when (this) { is MethodBridgeReceiver -> ReferenceBridge.objCEncoding MethodBridgeSelector -> ":" MethodBridgeValueParameter.ErrorOutParameter -> "^${ReferenceBridge.objCEncoding}" - is MethodBridgeValueParameter.KotlinResultOutParameter -> "^${this.bridge.objCEncoding}" } private val TypeBridge.objCEncoding: String get() = when (this) { diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/MethodBridge.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/MethodBridge.kt index cd1364e2ec1..4bf3f513a3b 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/MethodBridge.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/MethodBridge.kt @@ -35,7 +35,6 @@ internal object MethodBridgeSelector : MethodBridgeParameter() internal sealed class MethodBridgeValueParameter : MethodBridgeParameter() { data class Mapped(val bridge: TypeBridge) : MethodBridgeValueParameter() object ErrorOutParameter : MethodBridgeValueParameter() - data class KotlinResultOutParameter(val bridge: TypeBridge) : MethodBridgeValueParameter() } internal data class MethodBridge( @@ -55,7 +54,7 @@ internal data class MethodBridge( sealed class WithError : ReturnValue() { object Success : WithError() - data class RefOrNull(val successBridge: ReturnValue) : WithError() + data class ZeroForError(val successBridge: ReturnValue, val successMayBeZero: Boolean) : WithError() } } @@ -87,8 +86,7 @@ internal fun MethodBridge.valueParametersAssociated( when (it) { is MethodBridgeValueParameter.Mapped -> it to kotlinParameters.next() - is MethodBridgeValueParameter.ErrorOutParameter, - is MethodBridgeValueParameter.KotlinResultOutParameter -> it to null + is MethodBridgeValueParameter.ErrorOutParameter -> it to null } }.also { assert(!kotlinParameters.hasNext()) } } @@ -103,8 +101,7 @@ internal fun MethodBridge.parametersAssociated( is MethodBridgeValueParameter.Mapped, MethodBridgeReceiver.Instance -> it to kotlinParameters.next() - MethodBridgeReceiver.Static, MethodBridgeSelector, MethodBridgeValueParameter.ErrorOutParameter, - is MethodBridgeValueParameter.KotlinResultOutParameter -> + MethodBridgeReceiver.Static, MethodBridgeSelector, MethodBridgeValueParameter.ErrorOutParameter -> it to null MethodBridgeReceiver.Factory -> { 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 4e1e14d6dde..acef339a6d8 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 @@ -15,7 +15,6 @@ import org.jetbrains.kotlin.builtins.getReturnTypeFromFunctionType import org.jetbrains.kotlin.builtins.getValueParameterTypesFromFunctionType import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.constants.ArrayValue import org.jetbrains.kotlin.resolve.constants.KClassValue import org.jetbrains.kotlin.resolve.deprecation.Deprecation @@ -582,7 +581,6 @@ internal class ObjCExportTranslatorImpl( } } MethodBridgeValueParameter.ErrorOutParameter -> "error" - is MethodBridgeValueParameter.KotlinResultOutParameter -> "result" } val uniqueName = unifyName(candidateName, usedNames) @@ -592,15 +590,6 @@ internal class ObjCExportTranslatorImpl( is MethodBridgeValueParameter.Mapped -> mapType(p!!.type, bridge.bridge, objCExportScope) MethodBridgeValueParameter.ErrorOutParameter -> ObjCPointerType(ObjCNullableReferenceType(ObjCClassType("NSError")), nullable = true) - - is MethodBridgeValueParameter.KotlinResultOutParameter -> { - val resultType = mapType(method.returnType!!, bridge.bridge, objCExportScope) - // Note: non-nullable reference or pointer type is unusable here - // when passing reference to local variable from Swift because it - // would require a non-null initializer then. - val pointeeType = resultType.makeNullableIfReferenceOrPointer() - ObjCPointerType(pointeeType, nullable = true) - } } parameters += ObjCParameter(uniqueName, p, type) @@ -621,6 +610,14 @@ internal class ObjCExportTranslatorImpl( val attributes = mutableListOf() attributes += swiftNameAttribute(swiftName) + if (baseMethodBridge.returnBridge is MethodBridge.ReturnValue.WithError.ZeroForError + && baseMethodBridge.returnBridge.successMayBeZero) { + + // Method may return zero on success, but + // standard Objective-C convention doesn't suppose this happening. + // Add non-standard convention hint for Swift: + attributes += "swift_error(nonnull_error)" // Means "failure <=> (error != nil)". + } if (method is ConstructorDescriptor && !method.constructedClass.isArray) { // TODO: check methodBridge instead. attributes += "objc_designated_initializer" @@ -659,11 +656,17 @@ internal class ObjCExportTranslatorImpl( MethodBridge.ReturnValue.HashCode -> ObjCPrimitiveType.NSUInteger is MethodBridge.ReturnValue.Mapped -> mapType(method.returnType!!, returnBridge.bridge, objCExportScope) MethodBridge.ReturnValue.WithError.Success -> ObjCPrimitiveType.BOOL - is MethodBridge.ReturnValue.WithError.RefOrNull -> { - val successReturnType = mapReturnType(returnBridge.successBridge, method, objCExportScope) as? ObjCNonNullReferenceType - ?: error("Function is expected to have non-null return type: $method") + is MethodBridge.ReturnValue.WithError.ZeroForError -> { + val successReturnType = mapReturnType(returnBridge.successBridge, method, objCExportScope) - ObjCNullableReferenceType(successReturnType) + if (!returnBridge.successMayBeZero) { + check(successReturnType is ObjCNonNullReferenceType + || (successReturnType is ObjCPointerType && !successReturnType.nullable)) { + "Unexpected return type: $successReturnType in $method" + } + } + + successReturnType.makeNullableIfReferenceOrPointer() } MethodBridge.ReturnValue.Instance.InitResult, 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 2167356c26d..2e9a5a02e21 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 @@ -237,7 +237,6 @@ private fun ObjCExportMapper.bridgeParameter(parameter: ParameterDescriptor): Me private fun ObjCExportMapper.bridgeReturnType( descriptor: FunctionDescriptor, - valueParameters: MutableList, convertExceptionsToErrors: Boolean ): MethodBridge.ReturnValue { val returnType = descriptor.returnType!! @@ -247,7 +246,11 @@ private fun ObjCExportMapper.bridgeReturnType( } else { MethodBridge.ReturnValue.Instance.InitResult }.let { - if (convertExceptionsToErrors) MethodBridge.ReturnValue.WithError.RefOrNull(it) else it + if (convertExceptionsToErrors) { + MethodBridge.ReturnValue.WithError.ZeroForError(it, successMayBeZero = false) + } else { + it + } } descriptor.containingDeclaration == descriptor.builtIns.any && descriptor.name.asString() == "hashCode" -> { @@ -268,20 +271,25 @@ private fun ObjCExportMapper.bridgeReturnType( else -> { val returnTypeBridge = bridgeType(returnType) + val successReturnValueBridge = MethodBridge.ReturnValue.Mapped(returnTypeBridge) if (convertExceptionsToErrors) { - if (returnTypeBridge is ReferenceBridge && !TypeUtils.isNullableType(returnType)) { - MethodBridge.ReturnValue.WithError.RefOrNull(MethodBridge.ReturnValue.Mapped(returnTypeBridge)) - } else { - valueParameters += MethodBridgeValueParameter.KotlinResultOutParameter(returnTypeBridge) - MethodBridge.ReturnValue.WithError.Success - } + val canReturnZero = !returnTypeBridge.isReferenceOrPointer() || TypeUtils.isNullableType(returnType) + MethodBridge.ReturnValue.WithError.ZeroForError( + successReturnValueBridge, + successMayBeZero = canReturnZero + ) } else { - MethodBridge.ReturnValue.Mapped(returnTypeBridge) + successReturnValueBridge } } } } +private fun TypeBridge.isReferenceOrPointer(): Boolean = when (this) { + ReferenceBridge, is BlockPointerBridge -> true + is ValueTypeBridge -> this.objCValueType == ObjCValueType.POINTER +} + private fun ObjCExportMapper.bridgeMethodImpl(descriptor: FunctionDescriptor): MethodBridge { assert(isBaseMethod(descriptor)) @@ -306,7 +314,7 @@ private fun ObjCExportMapper.bridgeMethodImpl(descriptor: FunctionDescriptor): M valueParameters += bridgeParameter(it) } - val returnBridge = bridgeReturnType(descriptor, valueParameters, convertExceptionsToErrors) + val returnBridge = bridgeReturnType(descriptor, convertExceptionsToErrors) if (convertExceptionsToErrors) { valueParameters += MethodBridgeValueParameter.ErrorOutParameter } 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 eccc91ea420..b254d1898ff 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 @@ -453,13 +453,11 @@ internal class ObjCExportNamerImpl( else -> it!!.name.asString().toIdentifier() } MethodBridgeValueParameter.ErrorOutParameter -> "error" - is MethodBridgeValueParameter.KotlinResultOutParameter -> "result" } if (index == 0) { append(when { - bridge is MethodBridgeValueParameter.ErrorOutParameter || - bridge is MethodBridgeValueParameter.KotlinResultOutParameter -> "AndReturn" + bridge is MethodBridgeValueParameter.ErrorOutParameter -> "AndReturn" method is ConstructorDescriptor -> "With" else -> "" @@ -504,7 +502,6 @@ internal class ObjCExportNamerImpl( else -> it!!.name.asString().toIdentifier() } MethodBridgeValueParameter.ErrorOutParameter -> continue@parameters - is MethodBridgeValueParameter.KotlinResultOutParameter -> "result" } append(label) diff --git a/backend.native/tests/framework/values/expectedLazy.h b/backend.native/tests/framework/values/expectedLazy.h index 2fe38a8b6e3..fa66fdd4053 100644 --- a/backend.native/tests/framework/values/expectedLazy.h +++ b/backend.native/tests/framework/values/expectedLazy.h @@ -270,9 +270,9 @@ __attribute__((swift_name("BridgeBase"))) - (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer)); + (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead"))); - (id _Nullable)foo1AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo1()"))); -- (BOOL)foo2AndReturnResult:(int32_t * _Nullable)result error:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo2(result:)"))); +- (int32_t)foo2AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo2()"))) __attribute__((swift_error(nonnull_error))); - (BOOL)foo3AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo3()"))); -- (BOOL)foo4AndReturnResult:(ValuesKotlinNothing * _Nullable * _Nullable)result error:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo4(result:)"))); +- (ValuesKotlinNothing * _Nullable)foo4AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo4()"))) __attribute__((swift_error(nonnull_error))); @end; __attribute__((objc_subclassing_restricted)) @@ -281,9 +281,9 @@ __attribute__((swift_name("Bridge"))) - (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer)); + (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead"))); - (ValuesKotlinNothing * _Nullable)foo1AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo1()"))); -- (BOOL)foo2AndReturnResult:(int32_t * _Nullable)result error:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo2(result:)"))); +- (int32_t)foo2AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo2()"))) __attribute__((swift_error(nonnull_error))); - (BOOL)foo3AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo3()"))); -- (BOOL)foo4AndReturnResult:(ValuesKotlinNothing * _Nullable * _Nullable)result error:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo4(result:)"))); +- (ValuesKotlinNothing *)foo4AndReturnError:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("foo4()"))) __attribute__((swift_error(nonnull_error))); @end; __attribute__((objc_subclassing_restricted)) diff --git a/backend.native/tests/framework/values/values.swift b/backend.native/tests/framework/values/values.swift index 969450649c4..77f27754ebc 100644 --- a/backend.native/tests/framework/values/values.swift +++ b/backend.native/tests/framework/values/values.swift @@ -291,8 +291,7 @@ func testExceptions() throws { try assertTrue(error.kotlinException is MyException) } do { - var result: Int32 = 0 - try bridge.foo2(result: &result) + try bridge.foo2() } catch let error as NSError { try assertTrue(error.kotlinException is MyException) } @@ -302,8 +301,7 @@ func testExceptions() throws { try assertTrue(error.kotlinException is MyException) } do { - var result: KotlinNothing? = nil - try bridge.foo4(result: &result) + try bridge.foo4() } catch let error as NSError { try assertTrue(error.kotlinException is MyException) }