Merge pull request #76458 from xedin/rdar-131347583

[Concurrency] Allow global actor mismatches while overriding `@precon…

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -3329,6 +3329,9 @@ ERROR(override_multiple_decls_base,none,
 ERROR(override_sendability_mismatch,none,
       "declaration %0 has a type with different sendability from any potential "
       "overrides", (DeclName))
+ERROR(override_global_actor_isolation_mismatch,none,
+      "declaration %0 has a type with different global actor isolation from any potential "
+      "overrides", (DeclName))
 ERROR(override_multiple_decls_arg_mismatch,none,
       "declaration %0 has different argument labels from any potential "
       "overrides", (DeclName))

include/swift/AST/Types.h

@@ -362,6 +362,8 @@ enum class TypeMatchFlags {
   IgnoreFunctionSendability = 1 << 6,
   /// Ignore `any Sendable` and compositions with Sendable protocol.
   IgnoreSendability = 1 << 7,
+  /// Ignore global actor isolation attributes on functions when matching types.
+  IgnoreFunctionGlobalActorIsolation = 1 << 8,
 };
 using TypeMatchOptions = OptionSet<TypeMatchFlags>;
 

lib/AST/Type.cpp

@@ -3186,6 +3186,13 @@ static bool matchesFunctionType(CanAnyFunctionType fn1, CanAnyFunctionType fn2,
     ext2 = ext2.withSendable(false);
   }
 
+  if (matchMode.contains(TypeMatchFlags::IgnoreFunctionGlobalActorIsolation)) {
+    if (ext1.getGlobalActor())
+      ext1 = ext1.withoutIsolation();
+    if (ext2.getGlobalActor())
+      ext2 = ext2.withoutIsolation();
+  }
+
   // If specified, allow an escaping function parameter to override a
   // non-escaping function parameter when the parameter is optional.
   // Note that this is checking 'ext2' rather than 'ext1' because parameters

lib/Sema/TypeCheckDeclOverride.cpp

@@ -522,7 +522,10 @@ static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
 namespace {
   enum class OverrideCheckingAttempt {
     PerfectMatch,
+    // Ignores only @Sendable and `any Sendable` annotations
     MismatchedSendability,
+    // Ignores both sendability and global actor isolation annotations.
+    MismatchedConcurrency,
     MismatchedOptional,
     MismatchedTypes,
     BaseName,
@@ -555,16 +558,36 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
   case OverrideCheckingAttempt::MismatchedSendability: {
     SendableCheckContext fromContext(decl->getDeclContext(),
                                      SendableCheck::Explicit);
-    auto baseDeclClass =
-        decl->getOverriddenDecl()->getDeclContext()->getSelfClassDecl();
 
-    diagnoseSendabilityErrorBasedOn(baseDeclClass, fromContext,
-                                    [&](DiagnosticBehavior limit) {
-      diags.diagnose(decl, diag::override_sendability_mismatch, decl->getName())
-          .limitBehaviorUntilSwiftVersion(limit, 6)
+    for (const auto &match : matches) {
+      auto baseDeclClass = match.Decl->getDeclContext()->getSelfClassDecl();
+
+      diagnoseSendabilityErrorBasedOn(
+          baseDeclClass, fromContext, [&](DiagnosticBehavior limit) {
+            diags
+                .diagnose(decl, diag::override_sendability_mismatch,
+                          decl->getName())
+                .limitBehaviorUntilSwiftVersion(limit, 6)
+                .limitBehaviorIf(
+                    fromContext.preconcurrencyBehavior(baseDeclClass));
+            return false;
+          });
+    }
+    break;
+  }
+  case OverrideCheckingAttempt::MismatchedConcurrency: {
+    SendableCheckContext fromContext(decl->getDeclContext(),
+                                     SendableCheck::Explicit);
+
+    for (const auto &match : matches) {
+      auto baseDeclClass = match.Decl->getDeclContext()->getSelfClassDecl();
+
+      diags
+          .diagnose(decl, diag::override_global_actor_isolation_mismatch,
+                    decl->getName())
+          .limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning, 6)
           .limitBehaviorIf(fromContext.preconcurrencyBehavior(baseDeclClass));
-      return false;
-    });
+    }
     break;
   }
   case OverrideCheckingAttempt::BaseName:
@@ -591,7 +614,7 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
 
   for (auto match : matches) {
     auto matchDecl = match.Decl;
-    if (attempt == OverrideCheckingAttempt::PerfectMatch) {
+    if (attempt <= OverrideCheckingAttempt::MismatchedConcurrency) {
       diags.diagnose(matchDecl, diag::overridden_here);
       continue;
     }
@@ -896,6 +919,7 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
   switch (attempt) {
   case OverrideCheckingAttempt::PerfectMatch:
   case OverrideCheckingAttempt::MismatchedSendability:
+  case OverrideCheckingAttempt::MismatchedConcurrency:
   case OverrideCheckingAttempt::MismatchedOptional:
   case OverrideCheckingAttempt::MismatchedTypes:
     name = decl->getName();
@@ -993,6 +1017,11 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
     if (attempt == OverrideCheckingAttempt::MismatchedSendability)
       matchMode |= TypeMatchFlags::IgnoreFunctionSendability;
 
+    if (attempt == OverrideCheckingAttempt::MismatchedConcurrency) {
+      matchMode |= TypeMatchFlags::IgnoreFunctionSendability;
+      matchMode |= TypeMatchFlags::IgnoreFunctionGlobalActorIsolation;
+    }
+
     auto declFnTy = getDeclComparisonType()->getAs<AnyFunctionType>();
     auto parentDeclTy = getMemberTypeForComparison(parentDecl, decl);
     auto parentDeclFnTy = parentDeclTy->getAs<AnyFunctionType>();
@@ -1199,29 +1228,13 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
   Type declTy = getDeclComparisonType();
   Type owningTy = dc->getDeclaredInterfaceType();
   auto isClassContext = classDecl != nullptr;
-  bool allowsSendabilityMismatches =
+  bool allowsConcurrencyMismatches =
       attempt == OverrideCheckingAttempt::MismatchedSendability ||
+      attempt == OverrideCheckingAttempt::MismatchedConcurrency ||
       (attempt == OverrideCheckingAttempt::PerfectMatch &&
        baseDecl->preconcurrency());
   bool mismatchedOnSendability = false;
-
-  auto diagnoseSendabilityMismatch = [&]() {
-    SendableCheckContext fromContext(decl->getDeclContext(),
-                                     SendableCheck::Explicit);
-    auto baseDeclClass = baseDecl->getDeclContext()->getSelfClassDecl();
-
-    diagnoseSendabilityErrorBasedOn(
-        baseDeclClass, fromContext, [&](DiagnosticBehavior limit) {
-          diags
-              .diagnose(decl, diag::override_sendability_mismatch,
-                        decl->getName())
-              .limitBehaviorUntilSwiftVersion(limit, 6)
-              .limitBehaviorIf(
-                  fromContext.preconcurrencyBehavior(baseDeclClass));
-          diags.diagnose(baseDecl, diag::overridden_here);
-          return false;
-        });
-  };
+  bool mismatchedOnIsolation = false;
 
   if (declIUOAttr == matchDeclIUOAttr && declTy->isEqual(baseTy)) {
     // Nothing to do.
@@ -1289,15 +1302,21 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
     TypeMatchOptions options;
     options |= TypeMatchFlags::AllowOverride;
     if (!propertyTy->matches(parentPropertyCanTy, options)) {
-      if (allowsSendabilityMismatches) {
+      if (allowsConcurrencyMismatches) {
         options |= TypeMatchFlags::IgnoreSendability;
         options |= TypeMatchFlags::IgnoreFunctionSendability;
 
         mismatchedOnSendability =
             propertyTy->matches(parentPropertyCanTy, options);
+
+        if (!mismatchedOnSendability) {
+          options |= TypeMatchFlags::IgnoreFunctionGlobalActorIsolation;
+          mismatchedOnIsolation =
+              propertyTy->matches(parentPropertyCanTy, options);
+        }
       }
 
-      if (!mismatchedOnSendability) {
+      if (!mismatchedOnSendability && !mismatchedOnIsolation) {
         diags.diagnose(property, diag::override_property_type_mismatch,
                        property->getName(), propertyTy, parentPropertyTy);
         noteFixableMismatchedTypes(decl, baseDecl);
@@ -1316,28 +1335,36 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
 
     // The overridden property must not be mutable.
     if (cast<AbstractStorageDecl>(baseDecl)->supportsMutation() &&
-        !(optionalVsIUO || mismatchedOnSendability)) {
+        !(optionalVsIUO || mismatchedOnSendability || mismatchedOnIsolation)) {
       diags.diagnose(property, diag::override_mutable_covariant_property,
-                  property->getName(), parentPropertyTy, propertyTy);
+                     property->getName(), parentPropertyTy, propertyTy);
       diags.diagnose(baseDecl, diag::property_override_here);
       return true;
     }
-
-    if (mismatchedOnSendability && !emittedMatchError) {
-      diagnoseSendabilityMismatch();
-      return checkSingleOverride(decl, baseDecl);
-    }
   }
 
   if (emittedMatchError)
     return true;
 
-  if (attempt == OverrideCheckingAttempt::MismatchedSendability) {
-    diagnoseSendabilityMismatch();
+  if (mismatchedOnSendability || mismatchedOnIsolation) {
+    OverrideMatch match{baseDecl, /*isExact=*/false};
+
+    if (mismatchedOnSendability) {
+      diagnoseGeneralOverrideFailure(
+          decl, match, OverrideCheckingAttempt::MismatchedSendability);
+    }
+
+    if (mismatchedOnIsolation) {
+      diagnoseGeneralOverrideFailure(
+          decl, match, OverrideCheckingAttempt::MismatchedConcurrency);
+    }
+
+    return checkSingleOverride(decl, baseDecl);
   }
+
   // Catch-all to make sure we don't silently accept something we shouldn't.
-  else if (attempt != OverrideCheckingAttempt::PerfectMatch) {
-    OverrideMatch match{decl, /*isExact=*/false};
+  if (attempt != OverrideCheckingAttempt::PerfectMatch) {
+    OverrideMatch match{baseDecl, /*isExact=*/false};
     diagnoseGeneralOverrideFailure(decl, match, attempt);
   }
 
@@ -1400,6 +1427,7 @@ checkPotentialOverrides(ValueDecl *decl,
     case OverrideCheckingAttempt::PerfectMatch:
       break;
     case OverrideCheckingAttempt::MismatchedSendability:
+    case OverrideCheckingAttempt::MismatchedConcurrency:
       // Don't keep looking if the user didn't indicate it's an override.
       if (!decl->getAttrs().hasAttribute<OverrideAttr>())
         return false;

test/Concurrency/predates_concurrency.swift

@@ -271,6 +271,19 @@ do {
       nil
     }
 
+    @preconcurrency
+    open var test5: (@MainActor () -> Void)? { // expected-note {{overridden declaration is here}}
+      nil
+    }
+
+    open var test6: (@MainActor () -> Void)? { // expected-note {{attempt to override property here}}
+      nil
+    }
+
+    @preconcurrency
+    func test7(_: (@MainActor () -> Void)? = nil) { // expected-note {{overridden declaration is here}}
+    }
+
     init() {
       self.test1 = nil
       self.test2 = [:]
@@ -301,6 +314,20 @@ do {
       // expected-warning@-1 {{declaration 'test4' has a type with different sendability from any potential overrides; this is an error in the Swift 6 language mode}}
       nil
     }
+
+    override var test5: (() -> Void)? {
+      // expected-warning@-1 {{declaration 'test5' has a type with different global actor isolation from any potential overrides; this is an error in the Swift 6 language mode}}
+      nil
+    }
+
+    override var test6: (() -> Void)? {
+      // expected-error@-1 {{property 'test6' with type '(() -> Void)?' cannot override a property with type '(@MainActor () -> Void)?'}}
+      nil
+    }
+
+    override func test7(_: (() -> Void)?) {
+      // expected-warning@-1 {{declaration 'test7' has a type with different global actor isolation from any potential overrides; this is an error in the Swift 6 language mode}}
+    }
   }
 }
 

test/Concurrency/predates_concurrency_swift6.swift

@@ -130,6 +130,15 @@ do {
       nil
     }
 
+    @preconcurrency
+    open var test5: (@MainActor () -> Void)? { // expected-note {{overridden declaration is here}}
+      nil
+    }
+
+    @preconcurrency
+    func test6(_: (@MainActor () -> Void)? = nil) { // expected-note {{overridden declaration is here}}
+    }
+
     init() {
       self.test1 = nil
       self.test2 = [:]
@@ -160,5 +169,14 @@ do {
       // expected-error@-1 {{declaration 'test4' has a type with different sendability from any potential overrides}}
       nil
     }
+
+    override var test5: (() -> Void)? {
+      // expected-error@-1 {{declaration 'test5' has a type with different global actor isolation from any potential overrides}}
+      nil
+    }
+
+    override func test6(_: (() -> Void)?) {
+      // expected-error@-1 {{declaration 'test6' has a type with different global actor isolation from any potential overrides}}
+    }
   }
 }

test/Concurrency/sendable_objc_attr_in_type_context_swift5.swift

@@ -44,6 +44,7 @@ typedef void (^CompletionHandler)(void (^ SWIFT_SENDABLE)(void)) SWIFT_SENDABLE;
 -(void) withSendableCustom: (void (^)(MyValue *_Nullable SWIFT_SENDABLE)) handler;
 -(void) withNonSendable:(NSString *)operation completionHandler:(void (^ _Nullable NONSENDABLE)(NSString *_Nullable, NSError * _Nullable)) handler;
 -(void) withAliasCompletionHandler:(CompletionHandler)handler;
+-(void) withMainActorId: (void (MAIN_ACTOR ^)(id)) handler;
 @end
 
 // Placement of SWIFT_SENDABLE matters here
@@ -67,6 +68,17 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
 
 //--- main.swift
 
+do {
+  class SubTestNoActor : Test {
+    @objc override func withMainActorId(_: @escaping (Any) -> Void) {}
+    // expected-warning@-1 {{declaration 'withMainActorId' has a type with different global actor isolation from any potential overrides; this is an error in the Swift 6 language mode}}
+  }
+
+  class SubTestWithActor : Test {
+    @objc override func withMainActorId(_: @MainActor @escaping (Any) -> Void) {} // Ok
+  }
+}
+
 func test_sendable_attr_in_type_context(test: Test) {
   let fn: (String?, (any Error)?) -> Void = { _,_ in }
 

test/Concurrency/sendable_objc_attr_in_type_context_swift5_strict.swift

@@ -45,6 +45,7 @@ typedef void (^CompletionHandler)(void (^ SWIFT_SENDABLE)(void)) SWIFT_SENDABLE;
 -(void) withSendableCustom: (void (^)(MyValue *_Nullable SWIFT_SENDABLE)) handler;
 -(void) withNonSendable:(NSString *)operation completionHandler:(void (^ _Nullable NONSENDABLE)(NSString *_Nullable, NSError * _Nullable)) handler;
 -(void) withAliasCompletionHandler:(CompletionHandler)handler;
+-(void) withMainActorId: (void (MAIN_ACTOR ^)(id)) handler;
 @end
 
 // Placement of SWIFT_SENDABLE matters here
@@ -68,6 +69,17 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
 
 //--- main.swift
 
+do {
+  class SubTestNoActor : Test {
+    @objc override func withMainActorId(_: @escaping (Any) -> Void) {}
+    // expected-warning@-1 {{declaration 'withMainActorId' has a type with different global actor isolation from any potential overrides; this is an error in the Swift 6 language mode}}
+  }
+
+  class SubTestWithActor : Test {
+    @objc override func withMainActorId(_: @MainActor @escaping (Any) -> Void) {} // Ok
+  }
+}
+
 func test_sendable_attr_in_type_context(test: Test) {
   let fn: (String?, (any Error)?) -> Void = { _,_ in }
 

test/Concurrency/sendable_objc_attr_in_type_context_swift6.swift

@@ -44,6 +44,7 @@ typedef void (^CompletionHandler)(void (^ SWIFT_SENDABLE)(void)) SWIFT_SENDABLE;
 -(void) withSendableCustom: (void (^)(MyValue *_Nullable SWIFT_SENDABLE)) handler;
 -(void) withNonSendable:(NSString *)operation completionHandler:(void (^ _Nullable NONSENDABLE)(NSString *_Nullable, NSError * _Nullable)) handler;
 -(void) withAliasCompletionHandler:(CompletionHandler)handler;
+-(void) withMainActorId: (void (MAIN_ACTOR ^)(id)) handler;
 @end
 
 // Placement of SWIFT_SENDABLE matters here
@@ -67,6 +68,17 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
 
 //--- main.swift
 
+do {
+  class SubTestNoActor : Test {
+    @objc override func withMainActorId(_: @escaping (Any) -> Void) {}
+    // expected-error@-1 {{declaration 'withMainActorId' has a type with different global actor isolation from any potential overrides}}
+  }
+
+  class SubTestWithActor : Test {
+    @objc override func withMainActorId(_: @MainActor @escaping (Any) -> Void) {} // Ok
+  }
+}
+
 func test_sendable_attr_in_type_context(test: Test) {
   let fn: (String?, (any Error)?) -> Void = { _,_ in }