Match @objcImpl member types

Check the types of @objcImpl candidates against their requirements and diagnose *most* mismatches.

Unlike typical type matching rules, @objcImpl allows an implementation’s parameter *and* result types to be an IUO when the requirement isn’t. This runs against the normal covariance rules in the case of the result type. It’s meant to allow an implementation to handle nils passed to it and return nils even if the declaration formally claims nils are not permitted; this is occasionally necessary to reimplement Objective-C APIs without breaking ABI compatibility.

Fixes rdar://102063730.

Author: beccadax

include/swift/AST/DiagnosticsSema.def

@@ -1633,6 +1633,11 @@ ERROR(objc_implementation_must_be_settable,none,
       "header",
       (DescriptiveDeclKind, ValueDecl *, DescriptiveDeclKind))
 
+ERROR(objc_implementation_type_mismatch,none,
+      "%0 %1 of type %2 does not match type %3 declared by the "
+      "header",
+      (DescriptiveDeclKind, ValueDecl *, Type, Type))
+
 ERROR(objc_implementation_wrong_objc_name,none,
       "selector %0 for %1 %2 not found in header; did you mean %3?",
       (ObjCSelector, DescriptiveDeclKind, ValueDecl *, ObjCSelector))

lib/Sema/TypeCheckDeclObjC.cpp

@@ -3020,7 +3020,7 @@ class ObjCImplementationChecker {
     WrongStaticness,
     WrongCategory,
     WrongDeclKind,
-//    WrongType,
+    WrongType,
     WrongWritability,
 
     Match,
@@ -3213,6 +3213,71 @@ class ObjCImplementationChecker {
     return lhs == rhs;
   }
 
+  static bool matchParamTypes(Type reqTy, Type implTy, ValueDecl *implDecl) {
+    TypeMatchOptions matchOpts = {};
+
+    // Try a plain type match.
+    if (implTy->matchesParameter(reqTy, matchOpts))
+      return true;
+
+    // If the implementation type is IUO, try unwrapping it.
+    if (auto unwrappedImplTy = implTy->getOptionalObjectType())
+      return implDecl->isImplicitlyUnwrappedOptional()
+                && unwrappedImplTy->matchesParameter(reqTy, matchOpts);
+
+    return false;
+  }
+
+  static bool matchTypes(Type reqTy, Type implTy, ValueDecl *implDecl) {
+    TypeMatchOptions matchOpts = {};
+
+    // Try a plain type match.
+    if (reqTy->matches(implTy, matchOpts))
+      return true;
+
+    // If the implementation type is optional, try unwrapping it.
+    if (auto unwrappedImplTy = implTy->getOptionalObjectType())
+      return implDecl->isImplicitlyUnwrappedOptional()
+                  && reqTy->matches(unwrappedImplTy, matchOpts);
+
+    // Apply these rules to the result type and parameters if it's a function
+    // type.
+    if (auto funcReqTy = reqTy->getAs<AnyFunctionType>())
+      if (auto funcImplTy = implTy->getAs<AnyFunctionType>())
+        return funcReqTy->matchesFunctionType(funcImplTy, matchOpts,
+                                              [=]() -> bool {
+          auto reqParams = funcReqTy->getParams();
+          auto implParams = funcImplTy->getParams();
+          if (reqParams.size() != implParams.size())
+            return false;
+
+          auto implParamList =
+              cast<AbstractFunctionDecl>(implDecl)->getParameters();
+
+          for (auto i : indices(reqParams)) {
+            const auto &reqParam = reqParams[i];
+            const auto &implParam = implParams[i];
+            ParamDecl *implParamDecl = implParamList->get(i);
+
+            if (!matchParamTypes(reqParam.getOldType(), implParam.getOldType(),
+                                 implParamDecl))
+              return false;
+          }
+
+          return matchTypes(funcReqTy->getResult(), funcImplTy->getResult(),
+                            implDecl);
+        });
+
+    return false;
+  }
+
+  static Type getMemberType(ValueDecl *decl) {
+    if (isa<AbstractFunctionDecl>(decl))
+      // Strip off the uncurried `self` parameter.
+      return decl->getInterfaceType()->getAs<AnyFunctionType>()->getResult();
+    return decl->getInterfaceType();
+  }
+
   MatchOutcome matchesImpl(ValueDecl *req, ValueDecl *cand,
                            ObjCSelector explicitObjCName) const {
     bool hasObjCNameMatch =
@@ -3246,7 +3311,8 @@ class ObjCImplementationChecker {
     if (cand->getKind() != req->getKind())
       return MatchOutcome::WrongDeclKind;
 
-    // FIXME: Diagnose type mismatches (with allowance for extra optionality)
+    if (!matchTypes(getMemberType(req), getMemberType(cand), cand))
+      return MatchOutcome::WrongType;
 
     if (auto reqVar = dyn_cast<AbstractStorageDecl>(req))
       if (reqVar->isSettable(nullptr) &&
@@ -3330,6 +3396,12 @@ class ObjCImplementationChecker {
                cand->getDescriptiveKind(), cand, req->getDescriptiveKind());
       return;
 
+    case MatchOutcome::WrongType:
+      diagnose(cand, diag::objc_implementation_type_mismatch,
+               cand->getDescriptiveKind(), cand,
+               getMemberType(cand), getMemberType(req));
+      return;
+
     case MatchOutcome::WrongWritability:
       diagnose(cand, diag::objc_implementation_must_be_settable,
                cand->getDescriptiveKind(), cand, req->getDescriptiveKind());

test/IRGen/objc_implementation.swift

@@ -22,7 +22,7 @@
 // CHECK: [[selector_data_implProperty:@[^, ]+]] = private global [13 x i8] c"implProperty\00", section "__TEXT,__objc_methname,cstring_literals", align 1
 // CHECK: [[selector_data_setImplProperty_:@[^, ]+]] = private global [17 x i8] c"setImplProperty:\00", section "__TEXT,__objc_methname,cstring_literals", align 1
 // CHECK: [[selector_data__cxx_destruct:@[^, ]+]] = private global [14 x i8] c".cxx_destruct\00", section "__TEXT,__objc_methname,cstring_literals", align 1
-// CHECK: [[_INSTANCE_METHODS_ImplClass:@[^, ]+]] = internal constant { i32, i32, [7 x { i8*, i8*, i8* }] } { i32 24, i32 7, [7 x { i8*, i8*, i8* }] [{ i8*, i8*, i8* } { i8* getelementptr inbounds ([5 x i8], [5 x i8]* @"\01L_selector_data(init)", i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.@16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationEABycfcTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([13 x i8], [13 x i8]* [[selector_data_implProperty]], i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.i16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE12implPropertys5Int32VvgTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([17 x i8], [17 x i8]* [[selector_data_setImplProperty_]], i64 0, i64 0), i8* getelementptr inbounds ([11 x i8], [11 x i8]* @".str.10.v20@0:8i16", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE12implPropertys5Int32VvsTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([12 x i8], [12 x i8]* [[selector_data_mainMethod_]], i64 0, i64 0), i8* getelementptr inbounds ([11 x i8], [11 x i8]* @".str.10.v20@0:8i16", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE10mainMethodyys5Int32VFTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([14 x i8], [14 x i8]* @"\01L_selector_data(copyWithZone:)", i64 0, i64 0), i8* getelementptr inbounds ([12 x i8], [12 x i8]* @".str.11.@24@0:8^v16", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE4copy4withyp10ObjectiveC6NSZoneV_tFTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([8 x i8], [8 x i8]* @"\01L_selector_data(dealloc)", i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.v16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationEfDTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([14 x i8], [14 x i8]* [[selector_data__cxx_destruct]], i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.v16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassCfETo{{(\.ptrauth)?}}" to i8*) }] }, section "__DATA, __objc_data", align 8
+// CHECK: [[_INSTANCE_METHODS_ImplClass:@[^, ]+]] = internal constant { i32, i32, [7 x { i8*, i8*, i8* }] } { i32 24, i32 7, [7 x { i8*, i8*, i8* }] [{ i8*, i8*, i8* } { i8* getelementptr inbounds ([5 x i8], [5 x i8]* @"\01L_selector_data(init)", i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.@16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationEABycfcTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([13 x i8], [13 x i8]* [[selector_data_implProperty]], i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.i16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE12implPropertys5Int32VvgTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([17 x i8], [17 x i8]* [[selector_data_setImplProperty_]], i64 0, i64 0), i8* getelementptr inbounds ([11 x i8], [11 x i8]* @".str.10.v20@0:8i16", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE12implPropertys5Int32VvsTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([12 x i8], [12 x i8]* [[selector_data_mainMethod_]], i64 0, i64 0), i8* getelementptr inbounds ([11 x i8], [11 x i8]* @".str.10.v20@0:8i16", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE10mainMethodyys5Int32VFTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([14 x i8], [14 x i8]* @"\01L_selector_data(copyWithZone:)", i64 0, i64 0), i8* getelementptr inbounds ([12 x i8], [12 x i8]* @".str.11.@24@0:8^v16", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationE4copy4withypSg10ObjectiveC6NSZoneVSg_tFTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([8 x i8], [8 x i8]* @"\01L_selector_data(dealloc)", i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.v16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassC19objc_implementationEfDTo{{(\.ptrauth)?}}" to i8*) }, { i8*, i8*, i8* } { i8* getelementptr inbounds ([14 x i8], [14 x i8]* [[selector_data__cxx_destruct]], i64 0, i64 0), i8* getelementptr inbounds ([8 x i8], [8 x i8]* @".str.7.v16@0:8", i64 0, i64 0), i8* bitcast ({{.*}}* @"$sSo9ImplClassCfETo{{(\.ptrauth)?}}" to i8*) }] }, section "__DATA, __objc_data", align 8
 // CHECK: [[_IVARS_ImplClass:@[^, ]+]] = internal constant { i32, i32, [2 x { i64*, i8*, i8*, i32, i32 }] } { i32 32, i32 2, [2 x { i64*, i8*, i8*, i32, i32 }] [{ i64*, i8*, i8*, i32, i32 } { i64* @"$sSo9ImplClassC19objc_implementationE12implPropertys5Int32VvpWvd", i8* getelementptr inbounds ([13 x i8], [13 x i8]* @.str.12.implProperty, i64 0, i64 0), i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str.0., i64 0, i64 0), i32 2, i32 4 }, { i64*, i8*, i8*, i32, i32 } { i64* @"$sSo9ImplClassC19objc_implementationE13implProperty2So8NSObjectCSgvpWvd", i8* getelementptr inbounds ([14 x i8], [14 x i8]* @.str.13.implProperty2, i64 0, i64 0), i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str.0., i64 0, i64 0), i32 3, i32 8 }] }, section "__DATA, __objc_const", align 8
 // CHECK: [[_PROPERTIES_ImplClass:@[^, ]+]] = internal constant { i32, i32, [1 x { i8*, i8* }] } { i32 16, i32 1, [1 x { i8*, i8* }] [{ i8*, i8* } { i8* getelementptr inbounds ([13 x i8], [13 x i8]* @.str.12.implProperty, i64 0, i64 0), i8* getelementptr inbounds ([19 x i8], [19 x i8]* @".str.18.Ti,N,VimplProperty", i64 0, i64 0) }] }, section "__DATA, __objc_const", align 8
 // FIXME: Should just be @_PROTOCOLS_ImplClass, but an IRGen bug causes the protocol list to be emitted twice.
@@ -51,7 +51,7 @@
   @objc func mainMethod(_: Int32) { print(implProperty) }
 
   @objc(copyWithZone:)
-  func copy(with zone: NSZone) -> Any {
+  func copy(with zone: NSZone?) -> Any? {
     let copy = ImplClass()
     copy.implProperty = implProperty
     copy.implProperty2 = implProperty2
@@ -191,10 +191,10 @@ public func fn(impl: ImplClass, swiftSub: SwiftSubclass) {
 // CHECK-LABEL: define internal void @"$sSo9ImplClassC19objc_implementationE10mainMethodyys5Int32VFTo"
 
 // Swift calling convention -[ImplClass copyWithZone:]
-// CHECK-LABEL: define hidden swiftcc void @"$sSo9ImplClassC19objc_implementationE4copy4withyp10ObjectiveC6NSZoneV_tF"
+// CHECK-LABEL: define hidden swiftcc void @"$sSo9ImplClassC19objc_implementationE4copy4withypSg10ObjectiveC6NSZoneVSg_tF"
 
 // ObjC calling convention -[ImplClass copyWithZone:]
-// CHECK-LABEL: define internal i8* @"$sSo9ImplClassC19objc_implementationE4copy4withyp10ObjectiveC6NSZoneV_tFTo"
+// CHECK-LABEL: define internal i8* @"$sSo9ImplClassC19objc_implementationE4copy4withypSg10ObjectiveC6NSZoneVSg_tFTo"
 
 // Swift calling convention -[ImplClass dealloc]
 // CHECK-LABEL: define linkonce_odr hidden swiftcc void @"$sSo9ImplClassC19objc_implementationEfD"

test/decl/ext/Inputs/objc_implementation.h

@@ -19,6 +19,7 @@
 - (void)methodFromHeader3:(int)param;
 - (void)methodFromHeader4:(int)param;
 - (int)methodFromHeader5;
+- (void)methodFromHeader6:(int)param;
 
 @property int propertyFromHeader1;
 @property int propertyFromHeader2;
@@ -30,6 +31,7 @@
 @property int propertyFromHeader8;
 @property int propertyFromHeader9;
 @property int propertyFromHeader10;
+@property int propertyFromHeader11;
 
 @property (readonly) int readonlyPropertyFromHeader1;
 @property (readonly) int readonlyPropertyFromHeader2;
@@ -40,6 +42,7 @@
 
 + (void)classMethod1:(int)param;
 + (void)classMethod2:(int)param;
++ (void)classMethod3:(int)param;
 
 - (void)instanceMethod1:(int)param;
 - (void)instanceMethod2:(int)param;
@@ -108,6 +111,28 @@
 
 @end
 
+@interface ObjCClass (TypeMatchOptionality)
+
+- (nullable id)nullableResultAndArg:(nullable id)arg;
+- (nonnull id)nonnullResultAndArg:(nonnull id)arg;
+- (null_unspecified id)nullUnspecifiedResultAndArg:(null_unspecified id)arg;
+
+- (nonnull id)nonnullResult1;
+- (nonnull id)nonnullResult2;
+- (nonnull id)nonnullResult3;
+
+- (void)nonnullArgument1:(nonnull id)arg;
+- (void)nonnullArgument2:(nonnull id)arg;
+- (void)nonnullArgument3:(nonnull id)arg;
+
+- (nullable id)nullableResult;
+- (void)nullableArgument:(nullable id)arg;
+
+- (int)nonPointerResult;
+- (void)nonPointerArgument:(int)arg;
+
+@end
+
 @interface ObjCSubclass : ObjCClass
 
 - (void)subclassMethodFromHeader1:(int)param;

test/decl/ext/objc_implementation.swift

@@ -41,6 +41,10 @@
   var methodFromHeader5: CInt
   // expected-error@-1 {{property 'methodFromHeader5' does not match the instance method declared by the header}}
 
+  func method(fromHeader6: Double) {
+    // expected-error@-1 {{instance method 'method(fromHeader6:)' of type '(Double) -> ()' does not match type '(Int32) -> Void' declared by the header}}
+  }
+
   var propertyFromHeader1: CInt
   // OK, provides an implementation with a stored property
 
@@ -75,6 +79,9 @@
     return 1
   }
 
+  var propertyFromHeader11: Float
+  // expected-error@-1 {{property 'propertyFromHeader11' of type 'Float' does not match type 'Int32' declared by the header}}
+
   var readonlyPropertyFromHeader1: CInt
   // OK, provides an implementation with a stored property that's nonpublicly settable
 
@@ -146,6 +153,10 @@
     // expected-warning@-1 {{instance method 'classMethod2' does not match class method declared in header; this will become an error before '@_objcImplementation' is stabilized}} {{3-3=class }}
   }
 
+  class func classMethod3(_: Float) {
+    // expected-error@-1 {{class method 'classMethod3' of type '(Float) -> ()' does not match type '(Int32) -> Void' declared by the header}}
+  }
+
   func instanceMethod1(_: CInt) {
     // OK
   }
@@ -308,6 +319,26 @@
   func optionalMethod1() {}
 }
 
+@_objcImplementation(TypeMatchOptionality) extension ObjCClass {
+  func nullableResultAndArg(_: Any?) -> Any? { fatalError() } // no-error
+  func nonnullResultAndArg(_: Any) -> Any { fatalError() } // no-error
+  func nullUnspecifiedResultAndArg(_: Any!) -> Any! { fatalError() } // no-error
+
+  func nonnullResult1() -> Any { fatalError() } // no-error
+  func nonnullResult2() -> Any? { fatalError() } // expected-error {{instance method 'nonnullResult2()' of type '() -> Any?' does not match type '() -> Any' declared by the header}}
+  func nonnullResult3() -> Any! { fatalError() } // no-error (special case)
+
+  func nonnullArgument1(_: Any) {} // no-error
+  func nonnullArgument2(_: Any?) {} // expected-error {{instance method 'nonnullArgument2' of type '(Any?) -> ()' does not match type '(Any) -> Void' declared by the header}}
+  func nonnullArgument3(_: Any!) {} // no-error (special case)
+
+  func nullableResult() -> Any { fatalError() } // expected-error {{instance method 'nullableResult()' of type '() -> Any' does not match type '() -> Any?' declared by the header}}
+  func nullableArgument(_: Any) {} // expected-error {{instance method 'nullableArgument' of type '(Any) -> ()' does not match type '(Any?) -> Void' declared by the header}}
+
+  func nonPointerResult() -> CInt! { fatalError() } // expected-error{{method cannot be implicitly @objc because its result type cannot be represented in Objective-C}}
+  func nonPointerArgument(_: CInt!) {} // expected-error {{method cannot be implicitly @objc because the type of the parameter cannot be represented in Objective-C}}
+}
+
 @_objcImplementation extension ObjCClass {}
 // expected-error@-1 {{duplicate implementation of Objective-C class 'ObjCClass'}}
 

test/decl/ext/objc_implementation_conflicts.swift

@@ -28,6 +28,14 @@ import objc_implementation_private
     // OK
   }
 
+  func methodFromHeader5() -> CInt {
+    return 1 // OK
+  }
+
+  func method(fromHeader6: CInt) {
+    // OK
+  }
+
   @objc fileprivate func methodNot(fromHeader1: CInt) {
     // OK, declares a new @objc dynamic method.
   }
@@ -54,22 +62,26 @@ import objc_implementation_private
     set {}
   }
 
-  @objc let propertyFromHeader5: CInt
-  // FIXME: bad, needs to be settable
+  @objc var propertyFromHeader5: CInt
 
   @objc var propertyFromHeader6: CInt {
-    // FIXME: bad, needs a setter
     get { return 1 }
+    set {}
   }
 
   @objc var propertyFromHeader7: CInt {
     get { return 1 }
+    set {}
   }
 
   var propertyFromHeader8: CInt
 
   @objc var propertyFromHeader9: CInt
 
+  @objc var propertyFromHeader10: CInt
+
+  @objc var propertyFromHeader11: CInt
+
   @objc var readonlyPropertyFromHeader1: CInt
   // OK, provides an implementation with a stored property that's nonpublicly settable
 
@@ -136,6 +148,7 @@ import objc_implementation_private
 
   class func classMethod1(_: CInt) {}
   class func classMethod2(_: CInt) {}
+  class func classMethod3(_: CInt) {}
 
   func instanceMethod1(_: CInt) {}
   func instanceMethod2(_: CInt) {}