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

During actor isolation inference, we would unconditionally choose the
isolation of the overridden decl (when, say, there is no attribute on the decl).
The overridden decl is identified with `getOverriddenDecl`.

This works mostly fine, but initializers have some unusual overridden decls
returned by that method. For example, in the following code:

```swift
@objc class PictureFrame: NSObject {
  init(size: Int) { }
}

@mainactor
class FooFrame: PictureFrame {
  init() {
    super.init(size: 0)
  }
}
```

that method claims that `FooFrame.init()` overrides `PictureFrame.init()`, when
it really does not! So, if we were to unconditionally take the isolation from
`PictureFrame.init()` (and thus `NSObject.init()`), then we'd infer that
`FooFrame.init()` has unspecified isolation, despite the nominal it resides in
being marked as `MainActor`. This is in essence the problem in SR-15694, where
placing the isolation directly on the initializer fixes this issue.

If `FooFrame.init()` really does override, then why can it be `MainActor`? Well,
we have a rule in one part of the type-checker saying that if an ObjC-imported
decl has unspecified isolation, then overriding it with isolation is permitted.
But the other part of the type-checker dealing with the isolation inference was
not permitting that.

So, this patch unifies how actor-isolation inference is conducted to reuse that
same logic. In addition, the inference now effectively says that, if the decl
has no inferred isolation, or the inferred isolation is invalid according to the
overriding rules, then we use the isolation of the overridden decl. This
preserves the old behavior of the inference, while also fixing this issue with
ObjC declarations by expanding where isolation is allowed.

For example, as a consequence of this change, in the following code:

```swift
@mainactor
class FooFrame: NotIsolatedPictureFrame {
  override func rotate() {
    mainActorFn()
  }
}
```

if `NotIsolatedPictureFrame` is a plain-old not-isolated class imported from
Objective-C, then `rotate` will now have `MainActor` isolation, instead of
having unspecified isolation like it was inferred to have previously. This
helps make things consistent, because `rotate` is allowed to be `@MainActor` if
it were explicitly marked as such.

resolves rdar://87217618 / SR-15694

Author: kavon

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;