witnesses for an async ObjC protocol requirement cannot be sync

While a Swift protocol can have its async requirements satisfied
by a sync witness, this is not the case for ObjC protocols because
it is not just a calling convention difference.

Because every async ObjC function requirement in the protocol also
has a sibling that is sync, a sync witness might be trying
to conform to the sync version that takes a completion handler.

Without this change, we were seeing an issue where we would
consider both the async and sync versions of an ObjC requirement,
and accidentially choose the async version when the witness was
sync, and then raise an error about the typechecker's mistake.

Instead of raising an error, this change removes the ability for
the typechecker to even consider the errornous conformance.

resolves rdar://73326234

Author: kavon

include/swift/AST/DiagnosticsSema.def

@@ -4165,8 +4165,8 @@ ERROR(actor_isolated_objc,none,
       "actor-isolated %0 %1 cannot be @objc",
        (DescriptiveDeclKind, DeclName))
 NOTE(protocol_witness_async_conflict,none,
-     "candidate is %select{not |}0'async', but protocol requirement is%select{| not}0",
-     (bool))
+     "candidate is %select{not |}0'async', but%select{| @objc}1 protocol requirement is%select{| not}0",
+     (bool, bool))
 ERROR(async_autoclosure_nonasync_function,none,
       "'async' autoclosure parameter in a non-'async' function", ())
 
@@ -4214,9 +4214,6 @@ ERROR(objc_ambiguous_async_convention,none,
 NOTE(objc_ambiguous_async_convention_candidate,none,
      "%0 provides async here", (DeclName))
 
-ERROR(satisfy_async_objc,none,
-      "satisfying an asychronous @objc %select{method|initializer}0 with "
-      "a synchronous %select{method|initializer}0 is not supported", (bool))
 ERROR(async_objc_dynamic_self,none,
       "asynchronous method returning 'Self' cannot be '@objc'", ())
 

lib/Sema/TypeCheckAttr.cpp

@@ -1162,8 +1162,11 @@ void AttributeChecker::visitObjCAttr(ObjCAttr *attr) {
         if (CD->isObjCZeroParameterWithLongSelector())
           numParameters = 0;  // Something like "init(foo: ())"
 
-      // A throwing method has an error parameter.
-      if (func->hasThrows())
+      // An async method, even if it is also 'throws', has
+      // one additional completion handler parameter in ObjC.
+      if (func->hasAsync())
+        ++numParameters;
+      else if (func->hasThrows()) // A throwing method has an error parameter.
         ++numParameters;
 
       unsigned numArgumentNames = objcName->getNumArgs();

lib/Sema/TypeCheckDeclObjC.cpp

@@ -1795,11 +1795,8 @@ void markAsObjC(ValueDecl *D, ObjCReason reason,
 
     // Attach the foreign async convention.
     if (inheritedAsyncConvention) {
-      if (!method->hasAsync())
-        method->diagnose(diag::satisfy_async_objc,
-                         isa<ConstructorDecl>(method));
-      else
-        method->setForeignAsyncConvention(*inheritedAsyncConvention);
+      assert(method->hasAsync() && "async objc req offered for sync witness?");
+      method->setForeignAsyncConvention(*inheritedAsyncConvention);
 
     } else if (method->hasAsync()) {
       assert(asyncConvention && "Missing async convention");

lib/Sema/TypeCheckProtocol.cpp

@@ -731,6 +731,12 @@ swift::matchWitness(
       return RequirementMatch(witness, MatchKind::AsyncConflict);
     }
 
+    // If witness is sync, the requirement cannot be @objc and 'async'
+    if (!witnessFnType->getExtInfo().isAsync() &&
+          (req->isObjC() && reqFnType->getExtInfo().isAsync())) {
+      return RequirementMatch(witness, MatchKind::AsyncConflict);
+    }
+
     // If the witness is 'throws', the requirement must be.
     if (witnessFnType->getExtInfo().isThrowing() &&
         !reqFnType->getExtInfo().isThrowing()) {
@@ -2381,7 +2387,8 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
 
   case MatchKind::AsyncConflict:
     diags.diagnose(match.Witness, diag::protocol_witness_async_conflict,
-                   cast<AbstractFunctionDecl>(match.Witness)->hasAsync());
+                   cast<AbstractFunctionDecl>(match.Witness)->hasAsync(),
+                   req->isObjC());
     break;
 
   case MatchKind::ThrowsConflict:

test/ClangImporter/objc_async_conformance.swift

@@ -11,3 +11,57 @@ class C1: ConcurrentProtocol {
 
   func askUser(toJumpThroughHoop hoop: String) async -> String { "hello" }
 }
+
+// try to conform to an objc protocol that has both a sync and async requirement
+// that has the same name, and both requirements are optional.
+class C2 : NSObject, OptionalObserver {}
+extension C2 {
+  func hello(_ session: NSObject) -> Bool { true }
+}
+
+// a version of C2 that requires both sync and async methods (differing only by
+// completion handler) in ObjC, is not possible to conform to with 'async' in
+// a Swift protocol
+class C3 : NSObject, RequiredObserver {}
+extension C3 {
+  func hello() -> Bool { true } // expected-note {{'hello()' previously declared here}}
+  func hello() async -> Bool { true } // expected-error {{invalid redeclaration of 'hello()'}}
+}
+
+// the only way to conform to 'RequiredObserver' in Swift is to not use 'async'
+class C4 : NSObject, RequiredObserver {}
+extension C4 {
+  func hello() -> Bool { true }
+  func hello(_ completion : @escaping (Bool) -> Void) -> Void { completion(true) }
+}
+
+///////
+// selector conflicts
+
+// attempting to satisfy the ObjC async requirement in two ways simultaenously
+// is problematic due to a clash in selector names on this ObjC-compatible type
+class SelectorConflict : NSObject, RequiredObserverOnlyCompletion {
+  func hello() async -> Bool { true } // expected-note {{method 'hello()' declared here}}
+
+  // expected-error@+1 {{method 'hello' with Objective-C selector 'hello:' conflicts with method 'hello()' with the same Objective-C selector}}
+  func hello(_ completion : @escaping (Bool) -> Void) -> Void { completion(true) }
+}
+
+// making either one of the two methods nonobjc fixes it:
+class SelectorOK1 : NSObject, RequiredObserverOnlyCompletion {
+  @nonobjc func hello() async -> Bool { true }
+  func hello(_ completion : @escaping (Bool) -> Void) -> Void { completion(true) }
+}
+
+class SelectorOK2 : NSObject, RequiredObserverOnlyCompletion {
+  func hello() async -> Bool { true }
+  @nonobjc func hello(_ completion : @escaping (Bool) -> Void) -> Void { completion(true) }
+}
+
+// additional coverage for situation like C4, where the method names don't
+// clash on the ObjC side, but they do on Swift side, BUT their ObjC selectors
+// differ, so it's OK.
+class Rock : NSObject, Rollable {
+  func roll(completionHandler: @escaping () -> Void) { completionHandler() }
+  func roll() { roll(completionHandler: {}) }
+}

test/Concurrency/async_conformance.swift

@@ -0,0 +1,41 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
+// REQUIRES: objc_interop
+// REQUIRES: concurrency
+
+import Foundation
+
+// async objc requirement, sync witness
+@objc protocol Tracker {
+    func track(event: String) async // expected-note {{protocol requires function 'track(event:)' with type '(String) async -> ()'; do you want to add a stub?}}
+}
+class Dog: NSObject, Tracker { // expected-error {{type 'Dog' does not conform to protocol 'Tracker'}}
+    func track(event: String) {} // expected-note {{candidate is not 'async', but @objc protocol requirement is}}
+}
+
+// sync objc requirement, async witness
+@objc protocol Runner {
+    func run(event: String) // expected-note {{protocol requires function 'run(event:)' with type '(String) -> ()'; do you want to add a stub?}}
+}
+class Athlete: NSObject, Runner { // expected-error {{type 'Athlete' does not conform to protocol 'Runner'}}
+    func run(event: String) async {} // expected-note {{candidate is 'async', but @objc protocol requirement is not}}
+}
+
+
+// async swift protocol, sync witness
+protocol Snacker {
+    func snack(food: String) async
+}
+
+class Foodie: Snacker {
+    func snack(food: String) {}
+}
+
+
+// sync swift protocol, async witness
+protocol Backer {
+    func back(stonk: String) // expected-note {{protocol requires function 'back(stonk:)' with type '(String) -> ()'; do you want to add a stub?}}
+}
+
+class Investor: Backer {  // expected-error {{type 'Investor' does not conform to protocol 'Backer'}}
+    func back(stonk: String) async {}  // expected-note {{candidate is 'async', but protocol requirement is not}}
+}

test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h

@@ -77,6 +77,24 @@ typedef void (^CompletionHandler)(NSString * _Nullable, NSString * _Nullable_res
 -(void)missingAtAttributeMethod __attribute__((__swift_attr__("asyncHandler")));
 @end
 
+@protocol OptionalObserver <NSObject>
+@optional
+- (void)hello:(NSObject *)session completion:(void (^)(BOOL answer))completion;
+- (BOOL)hello:(NSObject *)session;
+@end
+
+@protocol RequiredObserverOnlyCompletion <NSObject>
+- (void)hello:(void (^)(BOOL answer))completion;
+@end
+
+@protocol RequiredObserver <RequiredObserverOnlyCompletion>
+- (BOOL)hello;
+@end
+
+@protocol Rollable <NSObject>
+- (void)rollWithCompletionHandler: (void (^)(void))completionHandler;
+@end
+
 #define MAGIC_NUMBER 42
 
 #pragma clang assume_nonnull end