Sendable checking for overrides.

When an override means crossing an actor boundary, check Sendability of
parameters and results.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4658,8 +4658,8 @@ ERROR(isolated_parameter_not_actor,none,
 WARNING(non_sendable_param_type,none,
         "non-sendable type %0 %select{passed in call to %4 %2 %3|"
         "passed in implicitly asynchronous call to %4 %2 %3|"
-        "in parameter of %4 %2 %3 satisfying non-isolated protocol "
-        "requirement|"
+        "in parameter of %4 %2 %3 satisfying protocol requirement|"
+        "in parameter of %4 overriding %2 %3|"
         "in parameter of %4 '@objc' %2 %3}1 cannot cross actor boundary",
         (Type, unsigned, DescriptiveDeclKind, DeclName, ActorIsolation))
 WARNING(non_sendable_call_param_type,none,
@@ -4669,7 +4669,8 @@ WARNING(non_sendable_call_param_type,none,
 WARNING(non_sendable_result_type,none,
         "non-sendable type %0 returned by %select{call to %4 %2 %3|"
         "implicitly asynchronous call to %4 %2 %3|"
-        "%4 %2 %3 satisfying non-isolated protocol requirement|"
+        "%4 %2 %3 satisfying protocol requirement|"
+        "%4 overriding %2 %3|"
         "%4 '@objc' %2 %3}1 cannot cross actor boundary",
         (Type, unsigned, DescriptiveDeclKind, DeclName, ActorIsolation))
 WARNING(non_sendable_call_result_type,none,
@@ -4680,7 +4681,8 @@ WARNING(non_sendable_property_type,none,
         "non-sendable type %0 in %select{"
         "%select{asynchronous access to %5 %1 %2|"
         "implicitly asynchronous access to %5 %1 %2|"
-        "conformance of %5 %1 %2 to non-isolated protocol requirement|"
+        "conformance of %5 %1 %2 to protocol requirement|"
+        "%5 overriding %1 %2|"
         "%5 '@objc' %1 %2}4|captured local %1 %2}3 cannot "
         "cross %select{actor|task}3 boundary",
         (Type, DescriptiveDeclKind, DeclName, bool, unsigned, ActorIsolation))

lib/Sema/TypeCheckConcurrency.cpp

@@ -3539,19 +3539,25 @@ static ActorIsolation getOverriddenIsolationFor(ValueDecl *value) {
   return isolation.subst(subs);
 }
 
+static ConcreteDeclRef getDeclRefInContext(ValueDecl *value) {
+  auto declContext = value->getInnermostDeclContext();
+  if (auto genericEnv = declContext->getGenericEnvironmentOfContext()) {
+    return ConcreteDeclRef(
+        value, genericEnv->getForwardingSubstitutionMap());
+  }
+
+  return ConcreteDeclRef(value);
+}
+
 /// 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 isolation the isolation of the overriding declaration.
 static OverrideIsolationResult validOverrideIsolation(
     ValueDecl *value, ActorIsolation isolation,
     ValueDecl *overridden, ActorIsolation overriddenIsolation) {
-  ConcreteDeclRef valueRef(value);
+  ConcreteDeclRef valueRef = getDeclRefInContext(value);
   auto declContext = value->getInnermostDeclContext();
-  if (auto genericEnv = declContext->getGenericEnvironmentOfContext()) {
-    valueRef = ConcreteDeclRef(
-        value, genericEnv->getForwardingSubstitutionMap());
-  }
 
   auto refResult = ActorReferenceResult::forReference(
       valueRef, SourceLoc(), declContext, None, None,
@@ -3570,9 +3576,7 @@ static OverrideIsolationResult validOverrideIsolation(
     if (isAsyncDecl(overridden) ||
         isAccessibleAcrossActors(
             overridden, refResult.isolation, declContext)) {
-      // FIXME: Perform Sendable checking here because we're entering an
-      // actor.
-      return OverrideIsolationResult::Allowed;
+      return OverrideIsolationResult::Sendable;
     }
 
     // If the overridden declaration is from Objective-C with no actor
@@ -3948,7 +3952,9 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
     return;
 
   case OverrideIsolationResult::Sendable:
-    // FIXME: Do the Sendable check.
+    diagnoseNonSendableTypesInReference(
+        getDeclRefInContext(value), value->getInnermostDeclContext(),
+        value->getLoc(), SendableCheckReason::Override);
     return;
 
   case OverrideIsolationResult::Disallowed:

lib/Sema/TypeCheckConcurrency.h

@@ -83,6 +83,9 @@ enum class SendableCheckReason {
   /// actor isolation.
   Conformance,
 
+  /// An override of a function.
+  Override,
+
   /// The declaration is being exposed to Objective-C.
   ObjC,
 };

test/Concurrency/concurrent_value_checking.swift

@@ -223,7 +223,7 @@ protocol AsyncProto {
 }
 
 extension A1: AsyncProto {
-  func asyncMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of actor-isolated instance method 'asyncMethod' satisfying non-isolated protocol requirement cannot cross actor boundary}}
+  func asyncMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of actor-isolated instance method 'asyncMethod' satisfying protocol requirement cannot cross actor boundary}}
 }
 
 protocol MainActorProto {
@@ -232,7 +232,7 @@ protocol MainActorProto {
 
 class SomeClass: MainActorProto {
   @SomeGlobalActor
-  func asyncMainMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of global actor 'SomeGlobalActor'-isolated instance method 'asyncMainMethod' satisfying non-isolated protocol requirement cannot cross actor boundary}}
+  func asyncMainMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of global actor 'SomeGlobalActor'-isolated instance method 'asyncMainMethod' satisfying protocol requirement cannot cross actor boundary}}
 }
 
 // ----------------------------------------------------------------------

test/Concurrency/sendable_conformance_checking.swift

@@ -40,9 +40,9 @@ protocol AsyncProtocolWithNotSendable {
 // actor's domain.
 @available(SwiftStdlib 5.1, *)
 actor A3: AsyncProtocolWithNotSendable {
-  func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
+  func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
 
-  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
+  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
     get async {
       NotSendable()
     }
@@ -53,9 +53,9 @@ actor A3: AsyncProtocolWithNotSendable {
 // actor's domain.
 @available(SwiftStdlib 5.1, *)
 actor A4: AsyncProtocolWithNotSendable {
-  func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
+  func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
 
-  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
+  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
     get {
       NotSendable()
     }
@@ -98,9 +98,9 @@ protocol AsyncThrowingProtocolWithNotSendable {
 // actor's domain.
 @available(SwiftStdlib 5.1, *)
 actor A7: AsyncThrowingProtocolWithNotSendable {
-  func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
+  func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
 
-  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
+  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
     get async {
       NotSendable()
     }
@@ -111,9 +111,9 @@ actor A7: AsyncThrowingProtocolWithNotSendable {
 // actor's domain.
 @available(SwiftStdlib 5.1, *)
 actor A8: AsyncThrowingProtocolWithNotSendable {
-  func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
+  func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
 
-  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
+  var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
     get {
       NotSendable()
     }

test/Concurrency/sendable_override_checking.swift

@@ -0,0 +1,28 @@
+// RUN: %target-typecheck-verify-swift
+// REQUIRES: concurrency
+
+@available(SwiftStdlib 5.1, *)
+class NotSendable { // expected-note 2{{class 'NotSendable' does not conform to the 'Sendable' protocol}}
+}
+
+@available(SwiftStdlib 5.1, *)
+@available(*, unavailable)
+extension NotSendable: Sendable { }
+
+@available(SwiftStdlib 5.1, *)
+class Super {
+  func f(_: NotSendable) async { }
+  @MainActor func g1(_: NotSendable) { }
+  @MainActor func g2(_: NotSendable) async { }
+}
+
+@available(SwiftStdlib 5.1, *)
+class Sub: Super {
+  @MainActor override func f(_: NotSendable) async { }
+  // expected-warning@-1{{non-sendable type 'NotSendable' in parameter of main actor-isolated overriding instance method 'f' cannot cross actor boundary}}
+
+  nonisolated override func g1(_: NotSendable) { } // okay, synchronous
+
+  nonisolated override func g2(_: NotSendable) async { }
+  // expected-warning@-1{{non-sendable type 'NotSendable' in parameter of nonisolated overriding instance method 'g2' cannot cross actor boundary}}
+}

test/Distributed/distributed_protocol_isolation.swift

@@ -128,7 +128,7 @@ protocol Server {
 }
 actor MyServer : Server {
   // expected-note@+1{{consider making generic parameter 'Message' conform to the 'Sendable' protocol}} {{29-29=, Sendable}}
-  func send<Message: Codable>(message: Message) throws -> String { "" }  // expected-warning{{non-sendable type 'Message' in parameter of actor-isolated instance method 'send(message:)' satisfying non-isolated protocol requirement cannot cross actor boundary}}
+  func send<Message: Codable>(message: Message) throws -> String { "" }  // expected-warning{{non-sendable type 'Message' in parameter of actor-isolated instance method 'send(message:)' satisfying protocol requirement cannot cross actor boundary}}
 }
 
 protocol AsyncThrowsAll {