[SR-15694] make isolation inference + override more consistent

lib/Sema/TypeCheckConcurrency.cpp

@@ -3593,6 +3593,36 @@ static bool checkClassGlobalActorIsolation(
   return true;
 }
 
+/// Generally speaking, the isolation of the decl that overrides
+/// must match the overridden decl. But there are a number of exceptions,
+/// e.g., the decl that overrides can be nonisolated.
+/// \param newIso the isolation of the overriding declaration.
+static bool validOverrideIsolation(ActorIsolation newIso,
+                                   ValueDecl *overriddenDecl,
+                                   ActorIsolation overriddenIso) {
+  // If the isolation matches, we're done.
+  if (newIso == overriddenIso)
+    return true;
+
+  // If the overriding declaration is non-isolated, it's okay.
+  if (newIso.isIndependent() || newIso.isUnspecified())
+    return true;
+
+  // If both are actor-instance isolated, we're done. This wasn't caught by
+  // the equality case above because the nominal type describing the actor
+  // will differ when we're overriding.
+  if (newIso.getKind() == overriddenIso.getKind() &&
+      newIso.getKind() == ActorIsolation::ActorInstance)
+    return true;
+
+  // If the overridden declaration is from Objective-C with no actor annotation,
+  // allow it.
+  if (overriddenDecl->hasClangNode() && !overriddenIso)
+    return true;
+
+  return false;
+}
+
 ActorIsolation ActorIsolationRequest::evaluate(
     Evaluator &evaluator, ValueDecl *value) const {
   // If this declaration has actor-isolated "self", it's isolated to that
@@ -3650,9 +3680,36 @@ ActorIsolation ActorIsolationRequest::evaluate(
         if (!ctor->hasAsync())
           defaultIsolation = ActorIsolation::forIndependent();
 
+  // Look for and remember the overridden declaration's isolation.
+  Optional<ActorIsolation> overriddenIso;
+  ValueDecl *overriddenValue = nullptr;
+  if ( (overriddenValue = value->getOverriddenDecl()) ) {
+    auto iso = getActorIsolation(overriddenValue);
+    SubstitutionMap subs;
+
+    if (Type selfType = value->getDeclContext()->getSelfInterfaceType()) {
+      subs = selfType->getMemberSubstitutionMap(
+          value->getModuleContext(), overriddenValue);
+    }
+    iso = iso.subst(subs);
+
+    // use the overridden decl's iso as the default isolation for this decl.
+    defaultIsolation = iso;
+    overriddenIso = iso;
+  }
+
   // Function used when returning an inferred isolation.
   auto inferredIsolation = [&](
       ActorIsolation inferred, bool onlyGlobal = false) {
+    // check if the inferred isolation is valid in the context of
+    // its overridden isolation.
+    if (overriddenValue) {
+      // if the inferred isolation is not valid, then carry-over the overridden
+      // declaration's isolation as this decl's inferred isolation.
+      if (!validOverrideIsolation(inferred, overriddenValue, *overriddenIso))
+        inferred = *overriddenIso;
+    }
+
     // Add an implicit attribute to capture the actor isolation that was
     // inferred, so that (e.g.) it will be printed and serialized.
     ASTContext &ctx = value->getASTContext();
@@ -3739,20 +3796,6 @@ ActorIsolation ActorIsolationRequest::evaluate(
     }
   }
 
-  // If the declaration overrides another declaration, it must have the same
-  // actor isolation.
-  if (auto overriddenValue = value->getOverriddenDecl()) {
-    auto isolation = getActorIsolation(overriddenValue);
-    SubstitutionMap subs;
-
-    if (Type selfType = value->getDeclContext()->getSelfInterfaceType()) {
-      subs = selfType->getMemberSubstitutionMap(
-          value->getModuleContext(), overriddenValue);
-    }
-
-    return inferredIsolation(isolation.subst(subs));
-  }
-
   // If this is an accessor, use the actor isolation of its storage
   // declaration.
   if (auto accessor = dyn_cast<AccessorDecl>(value)) {
@@ -3947,30 +3990,7 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
     overriddenIsolation = overriddenIsolation.subst(subs);
   }
 
-  // If the isolation matches, we're done.
-  if (isolation == overriddenIsolation)
-    return;
-
-  // FIXME: do we need this?
-  if (overriddenIsolation == ActorIsolation::Unspecified &&
-      isolation == ActorIsolation::Independent) {
-    return;
-  }
-
-  // If the overriding declaration is non-isolated, it's okay.
-  if (isolation.isIndependent() || isolation.isUnspecified())
-    return;
-
-  // If both are actor-instance isolated, we're done. This wasn't caught by
-  // the equality case above because the nominal type describing the actor
-  // will differ when we're overriding.
-  if (isolation.getKind() == overriddenIsolation.getKind() &&
-      isolation.getKind() == ActorIsolation::ActorInstance)
-    return;
-
-  // If the overridden declaration is from Objective-C with no actor annotation,
-  // allow it.
-  if (overridden->hasClangNode() && !overriddenIsolation)
+  if (validOverrideIsolation(isolation, overridden, overriddenIsolation))
     return;
 
   // Isolation mismatch. Diagnose it.

test/ClangImporter/objc_async.swift

@@ -230,4 +230,69 @@ func testMirrored(instance: ClassWithAsync) async {
   func g() async { }
 }
 
+
+@MainActor func mainActorFn() {}
+@SomeGlobalActor func sgActorFn() {}
+
+// Check inferred isolation for overridden decls from ObjC.
+// Note that even if the override is not present, it
+// can have an affect. -- rdar://87217618 / SR-15694
+@MainActor
+class FooFrame: PictureFrame {
+  init() {
+    super.init(size: 0)
+  }
+
+  override init(size n: Int) {
+    super.init(size: n)
+  }
+
+  override func rotate() {
+    mainActorFn()
+  }
+}
+
+class BarFrame: PictureFrame {
+  init() {
+    super.init(size: 0)
+  }
+
+  override init(size n: Int) {
+    super.init(size: n)
+  }
+
+  override func rotate() {
+    mainActorFn()
+  }
+}
+
+@SomeGlobalActor
+class BazFrame: NotIsolatedPictureFrame {
+  init() {
+    super.init(size: 0)
+  }
+
+  override init(size n: Int) {
+    super.init(size: n)
+  }
+
+  override func rotate() {
+    sgActorFn()
+  }
+}
+
+@SomeGlobalActor
+class BazFrameIso: PictureFrame { // expected-error {{global actor 'SomeGlobalActor'-isolated class 'BazFrameIso' has different actor isolation from main actor-isolated superclass 'PictureFrame'}}
+}
+
+func check() async {
+  _ = await BarFrame()
+  _ = await FooFrame()
+  _ = await BazFrame()
+
+  _ = await BarFrame(size: 0)
+  _ = await FooFrame(size: 0)
+  _ = await BazFrame(size: 0)
+}
+
 } // SwiftStdlib 5.5

test/Concurrency/actor_isolation.swift

@@ -403,11 +403,11 @@ struct GenericGlobalActor<T> {
 }
 
 @available(SwiftStdlib 5.1, *)
-@SomeGlobalActor func onions() {} // expected-note{{calls to global function 'onions()' from outside of its actor context are implicitly asynchronous}}
+@SomeGlobalActor func onions_sga() {} // expected-note 2{{calls to global function 'onions_sga()' from outside of its actor context are implicitly asynchronous}}
 
 @available(SwiftStdlib 5.1, *)
-@MainActor func beets() { onions() } // expected-error{{call to global actor 'SomeGlobalActor'-isolated global function 'onions()' in a synchronous main actor-isolated context}}
-// expected-note@-1{{calls to global function 'beets()' from outside of its actor context are implicitly asynchronous}}
+@MainActor func beets_ma() { onions_sga() } // expected-error{{call to global actor 'SomeGlobalActor'-isolated global function 'onions_sga()' in a synchronous main actor-isolated context}}
+// expected-note@-1 4{{calls to global function 'beets_ma()' from outside of its actor context are implicitly asynchronous}}
 
 @available(SwiftStdlib 5.1, *)
 actor Crystal {
@@ -942,7 +942,7 @@ class SomeClassWithInits {
   deinit {
     print(mutableState) // Okay, we're actor-isolated
     print(SomeClassWithInits.shared) // expected-error{{static property 'shared' isolated to global actor 'MainActor' can not be referenced from this synchronous context}}
-    beets() //expected-error{{call to main actor-isolated global function 'beets()' in a synchronous nonisolated context}}
+    beets_ma() //expected-error{{call to main actor-isolated global function 'beets_ma()' in a synchronous nonisolated context}}
   }
 
   func isolated() { }
@@ -1356,3 +1356,68 @@ actor DunkTracker {
     }
   }
 }
+
+@MainActor
+class MA {
+  func method() {}
+}
+
+@SomeGlobalActor class SGA: MA {} // expected-error {{global actor 'SomeGlobalActor'-isolated class 'SGA' has different actor isolation from main actor-isolated superclass 'MA'}}
+
+protocol SGA_Proto {
+  @SomeGlobalActor func method() // expected-note {{'method()' declared here}}
+}
+
+// try to override a MA method with inferred isolation from a protocol requirement
+class SGA_MA: MA, SGA_Proto {
+  // expected-error@+2 {{call to global actor 'SomeGlobalActor'-isolated global function 'onions_sga()' in a synchronous main actor-isolated context}}
+  // expected-warning@+1 {{instance method 'method()' isolated to global actor 'MainActor' can not satisfy corresponding requirement from protocol 'SGA_Proto' isolated to global actor 'SomeGlobalActor'}}
+  override func method() { onions_sga() }
+}
+
+class None_MA: MA {
+  override func method() { beets_ma() }
+}
+
+class None {
+  func method() {} // expected-note{{overridden declaration is here}}
+}
+
+// try to add inferred isolation while overriding
+@MainActor
+class MA_None1: None {
+  // FIXME: bad note, since the problem is a mismatch in overridden vs inferred isolation; this wont help.
+  // expected-note@+1 {{add '@MainActor' to make instance method 'method()' part of global actor 'MainActor'}}
+  override func method() {
+    beets_ma() // expected-error {{call to main actor-isolated global function 'beets_ma()' in a synchronous nonisolated context}}
+  }
+}
+
+class MA_None2: None {
+  @MainActor
+  override func method() { // expected-error {{main actor-isolated instance method 'method()' has different actor isolation from nonisolated overridden declaration}}
+    beets_ma()
+  }
+}
+
+class MADirect {
+  @MainActor func method1() {}
+  @MainActor func method2() {}
+}
+
+class None_MADirect: MADirect {
+  // default-isolation vs overridden-MainActor = mainactor
+  override func method1() { beets_ma() }
+
+  // directly-nonisolated vs overridden mainactor = nonisolated
+  nonisolated override func method2() { beets_ma() } // expected-error {{call to main actor-isolated global function 'beets_ma()' in a synchronous nonisolated context}}
+}
+
+@SomeGlobalActor
+class SGA_MADirect: MADirect {
+  // inferred-SomeGlobalActor vs overridden-MainActor = mainactor
+  override func method1() { beets_ma() }
+
+  // directly-nonisolated vs overridden-MainActor = nonisolated
+  nonisolated override func method2() { beets_ma() } // expected-error {{call to main actor-isolated global function 'beets_ma()' in a synchronous nonisolated context}}
+}

test/Incremental/Verifier/single-file-private/AnyObject.swift

@@ -80,7 +80,9 @@ import Foundation
 // expected-member {{ObjectiveC.NSObject.Hasher}}
 // expected-member {{ObjectiveC.NSObjectProtocol.hash}}
 
+// expected-member {{Swift.Hashable.init}}
 // expected-member {{Swift.Hashable.deinit}}
+// expected-member {{Swift.Equatable.init}}
 // expected-member {{Swift.Equatable.deinit}}
 
 // expected-member {{Swift.Hashable.==}}

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

@@ -4,6 +4,7 @@
 #pragma clang assume_nonnull begin
 
 #define MAIN_ACTOR __attribute__((__swift_attr__("@MainActor")))
+#define UI_ACTOR __attribute__((swift_attr("@UIActor")))
 
 #ifdef __SWIFT_ATTR_SUPPORTS_SENDABLE_DECLS
   #define SENDABLE __attribute__((__swift_attr__("@Sendable")))
@@ -247,6 +248,17 @@ typedef NS_ERROR_ENUM(unsigned, NonSendableErrorCode, NonSendableErrorDomain) {
 } NONSENDABLE;
 // expected-warning@-3 {{cannot make error code type 'NonSendableErrorCode' non-sendable because Swift errors are always sendable}}
 
+UI_ACTOR
+@interface PictureFrame : NSObject
+- (instancetype)initWithSize:(NSInteger)frame NS_DESIGNATED_INITIALIZER;
+- (void)rotate;
+@end
+
+@interface NotIsolatedPictureFrame : NSObject
+- (instancetype)initWithSize:(NSInteger)frame NS_DESIGNATED_INITIALIZER;
+- (void)rotate;
+@end
+
 typedef NSString *SendableStringEnum NS_STRING_ENUM;
 typedef NSString *NonSendableStringEnum NS_STRING_ENUM NONSENDABLE;