From 14fb86d4879d3992f5011274d3809934cd1be073 Mon Sep 17 00:00:00 2001 From: SvyatoslavScherbina Date: Wed, 24 Apr 2019 15:01:47 +0300 Subject: [PATCH] Refactor and improve Objective-C override checks (#2907) --- .../kotlin/backend/konan/ObjCInterop.kt | 47 ++++++++++--------- backend.native/tests/interop/objc/smoke.h | 9 ++++ backend.native/tests/interop/objc/smoke.kt | 8 ++++ backend.native/tests/interop/objc/smoke.m | 25 +++++++++- 4 files changed, 67 insertions(+), 22 deletions(-) diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ObjCInterop.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ObjCInterop.kt index 172a5732f06..bb1f7eeb421 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ObjCInterop.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ObjCInterop.kt @@ -147,32 +147,37 @@ class ObjCOverridabilityCondition : ExternalOverridabilityCondition { subDescriptor: CallableDescriptor, subClassDescriptor: ClassDescriptor? ): ExternalOverridabilityCondition.Result { - - if (superDescriptor.name != subDescriptor.name) { - return ExternalOverridabilityCondition.Result.UNKNOWN - } - - val superClass = superDescriptor.containingDeclaration as? ClassDescriptor - val subClass = subDescriptor.containingDeclaration as? ClassDescriptor - - if (superClass == null || !superClass.isObjCClass() || subClass == null) { - return ExternalOverridabilityCondition.Result.UNKNOWN - } - - return if (areSelectorsEqual(superDescriptor, subDescriptor)) { - // Also check the method signatures if the subclass is user-defined: - if (subClass.isExternalObjCClass()) { - ExternalOverridabilityCondition.Result.OVERRIDABLE - } else { - ExternalOverridabilityCondition.Result.UNKNOWN + if (superDescriptor.name == subDescriptor.name) { // Slow path: + if (superDescriptor is FunctionDescriptor && subDescriptor is FunctionDescriptor) { + superDescriptor.getExternalObjCMethodInfo()?.let { superInfo -> + val subInfo = subDescriptor.getExternalObjCMethodInfo() + if (subInfo != null) { + // Overriding Objective-C method by Objective-C method in interop stubs. + // Don't even check method signatures: + return if (superInfo.selector == subInfo.selector) { + ExternalOverridabilityCondition.Result.OVERRIDABLE + } else { + ExternalOverridabilityCondition.Result.INCOMPATIBLE + } + } else { + // Overriding Objective-C method by Kotlin method. + if (!parameterNamesMatch(superDescriptor, subDescriptor)) { + return ExternalOverridabilityCondition.Result.INCOMPATIBLE + } + } + } + } else if (superDescriptor.isExternalObjCClassProperty() && subDescriptor.isExternalObjCClassProperty()) { + return ExternalOverridabilityCondition.Result.OVERRIDABLE } - } else { - ExternalOverridabilityCondition.Result.INCOMPATIBLE } + return ExternalOverridabilityCondition.Result.UNKNOWN } - private fun areSelectorsEqual(first: CallableDescriptor, second: CallableDescriptor): Boolean { + private fun CallableDescriptor.isExternalObjCClassProperty() = this is PropertyDescriptor && + (this.containingDeclaration as? ClassDescriptor)?.isExternalObjCClass() == true + + private fun parameterNamesMatch(first: FunctionDescriptor, second: FunctionDescriptor): Boolean { // The original Objective-C method selector is represented as // function name and parameter names (except first). diff --git a/backend.native/tests/interop/objc/smoke.h b/backend.native/tests/interop/objc/smoke.h index 31c0e4d20c1..6ef3c8a45d2 100644 --- a/backend.native/tests/interop/objc/smoke.h +++ b/backend.native/tests/interop/objc/smoke.h @@ -150,12 +150,21 @@ NSObject* createNSObject() { -(instancetype _Nonnull)initWithFormat:(NSString*)format, ...; +(instancetype _Nonnull)testVarargsWithFormat:(NSString*)format, ...; @property NSString* formatted; + ++(NSString* _Nonnull)stringWithFormat:(NSString*)format, ...; ++(NSObject* _Nonnull)stringWithFormat:(NSString*)format args:(void*)args; @end; @interface TestVarargs (TestVarargsExtension) -(instancetype _Nonnull)initWithFormat:(NSString*)format, ...; @end; +@interface TestVarargsSubclass : TestVarargs +// Test clashes: +-(instancetype _Nonnull)initWithFormat:(NSString*)format args:(void*)args; ++(NSString* _Nonnull)stringWithFormat:(NSString*)format args:(void*)args; +@end; + @interface TestOverrideInit : NSObject -(instancetype)initWithValue:(int)value NS_DESIGNATED_INITIALIZER; +(instancetype)createWithValue:(int)value; diff --git a/backend.native/tests/interop/objc/smoke.kt b/backend.native/tests/interop/objc/smoke.kt index cb852f7d0e3..a069e1da462 100644 --- a/backend.native/tests/interop/objc/smoke.kt +++ b/backend.native/tests/interop/objc/smoke.kt @@ -299,6 +299,14 @@ fun testVarargs() { format = "%d %d %d" ).formatted ) + + assertEquals( + "7", + TestVarargsSubclass.stringWithFormat( + "%d", + 7 + ) + ) } fun testOverrideInit() { diff --git a/backend.native/tests/interop/objc/smoke.m b/backend.native/tests/interop/objc/smoke.m index c557bc34837..4f5479ee6f2 100644 --- a/backend.native/tests/interop/objc/smoke.m +++ b/backend.native/tests/interop/objc/smoke.m @@ -134,7 +134,7 @@ static CustomRetainMethodsImpl* retainedCustomRetainMethodsImpl; } +(instancetype _Nonnull)testVarargsWithFormat:(NSString*)format, ... { - TestVarargs* result = [[TestVarargs alloc] init]; + TestVarargs* result = [[self alloc] init]; va_list args; va_start(args, format); @@ -144,6 +144,29 @@ static CustomRetainMethodsImpl* retainedCustomRetainMethodsImpl; return result; } ++(NSString* _Nonnull)stringWithFormat:(NSString*)format, ... { + va_list args; + va_start(args, format); + NSString* result = [[NSString alloc] initWithFormat:format arguments:args]; + va_end(args); + + return result; +} + ++(NSObject* _Nonnull)stringWithFormat:(NSString*)format args:(void*)args { + abort(); +} + +@end; + +@implementation TestVarargsSubclass +-(instancetype _Nonnull)initWithFormat:(NSString*)format args:(void*)args { + abort(); +} + ++(NSString* _Nonnull)stringWithFormat:(NSString*)format args:(void*)args { + abort(); +} @end; @implementation TestOverrideInit