Merge pull request #35662 from kavon/objc-async-conformance-bugs

[concurrency] fixes for conformance to async ObjC protocol requirements

Author: kavon

include/swift/AST/DiagnosticsSema.def

@@ -4169,8 +4169,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", ())
 
@@ -4218,9 +4218,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

@@ -1169,8 +1169,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