Merge pull request #59098 from DougGregor/se-0338-5.7

[SE-0338] Implement Sendable checking for SE-0338

Author: DougGregor

CHANGELOG.md

@@ -5,6 +5,39 @@ _**Note:** This is in reverse chronological order, so newer entries are added to
 
 ## Swift 5.7
 
+* [SE-0338][]:
+
+  Non-isolated async functions now always execute on the global concurrent pool,
+  so calling a non-isolated async function from actor-isolated code will leave
+  the actor. For example:
+
+  ```swift
+  class C { }
+
+  func f(_: C) async { /* always executes on the global concurrent pool */ }
+
+  actor A {
+    func g(c: C) async {
+      /* always executes on the actor */
+      print("on the actor")
+
+      await f(c)
+    }
+  }
+  ```
+
+  Prior to this change, the call from `f` to `g` might have started execution of
+  `g` on the actor, which could lead to actors being busy longer than strictly
+  necessary. Now, the non-isolated async function will always hop to the global
+  cooperative pool, not run on the actor. This can result in a behavior change
+  for programs that assumed that a non-isolated async function called from a
+  `@MainActor` context will be executed on the main actor, although such
+  programs were already technically incorrect.
+
+  Additionally, when leaving an actor to execution on the global cooperative
+  pool, `Sendable` checking will be performed, so the compiler will emit a
+  diagnostic in the call to `f` if `c` is not of `Sendable` type.
+
 * [SE-0353][]:
 
   Protocols with primary associated types can now be used in existential types,
@@ -9316,6 +9349,7 @@ Swift 1.0
 [SE-0335]: <https://github.com/apple/swift-evolution/blob/main/proposals/0335-existential-any.md>
 [SE-0336]: <https://github.com/apple/swift-evolution/blob/main/proposals/0336-distributed-actor-isolation.md>
 [SE-0337]: <https://github.com/apple/swift-evolution/blob/main/proposals/0337-support-incremental-migration-to-concurrency-checking.md>
+[SE-0338]: <https://github.com/apple/swift-evolution/blob/main/proposals/0338-clarify-execution-non-actor-async.md>
 [SE-0340]: <https://github.com/apple/swift-evolution/blob/main/proposals/0340-swift-noasync.md>
 [SE-0341]: <https://github.com/apple/swift-evolution/blob/main/proposals/0341-opaque-parameters.md>
 [SE-0343]: <https://github.com/apple/swift-evolution/blob/main/proposals/0343-top-level-concurrency.md>

include/swift/AST/ActorIsolation.h

@@ -131,6 +131,8 @@ class ActorIsolation {
     return getKind() == GlobalActor || getKind() == GlobalActorUnsafe;
   }
 
+  bool isDistributedActor() const;
+
   Type getGlobalActor() const {
     assert(isGlobalActor());
     return globalActor;

include/swift/AST/DiagnosticsSema.def

@@ -4470,14 +4470,11 @@ NOTE(note_add_nonisolated_to_decl,none,
      (DeclName, DescriptiveDeclKind))
 NOTE(note_add_async_and_throws_to_decl,none,
      "mark the protocol requirement %0 '%select{|async|throws|async throws}1' "
-     "in order witness it with 'distributed' function declared in distributed actor %2",
-     (DeclName, unsigned, DeclName))
+     "to allow actor-isolated conformances",
+     (DeclName, unsigned))
 NOTE(note_add_distributed_to_decl,none,
-     "add 'distributed' to %0 to make this %1 witness the protocol requirement",
+     "add 'distributed' to %0 to make this %1 satisfy the protocol requirement",
      (DeclName, DescriptiveDeclKind))
-NOTE(note_distributed_requirement_defined_here,none,
-     "distributed instance method requirement %0 declared here",
-     (DeclName))
 NOTE(note_add_globalactor_to_function,none,
      "add '@%0' to make %1 %2 part of global actor %3", 
      (StringRef, DescriptiveDeclKind, DeclName, Type))
@@ -4558,13 +4555,16 @@ ERROR(override_implicit_unowned_executor,none,
       "cannot override an actor's 'unownedExecutor' property that wasn't "
       "explicitly defined", ())
 ERROR(actor_isolated_non_self_reference,none,
-        "actor-isolated %0 %1 can not be "
+        "%5 %0 %1 can not be "
         "%select{referenced|mutated|used 'inout'}2 "
-        "%select{on a non-isolated actor instance|"
+        "%select{from outside the actor|on a different actor instance|"
+        "on a non-isolated actor instance|"
         "from a Sendable function|from a Sendable closure|"
         "from an 'async let' initializer|from global actor %4|"
-        "from the main actor|from a non-isolated context|from a non-isolated autoclosure}3",
-        (DescriptiveDeclKind, DeclName, unsigned, unsigned, Type))
+        "from the main actor|from a non-isolated context|"
+        "from a non-isolated autoclosure}3",
+        (DescriptiveDeclKind, DeclName, unsigned, unsigned, Type,
+         ActorIsolation))
 ERROR(distributed_actor_isolated_non_self_reference,none,
       "distributed actor-isolated %0 %1 can not be accessed from a "
       "non-isolated context",
@@ -4579,18 +4579,6 @@ ERROR(actor_isolated_inout_state,none,
 ERROR(actor_isolated_mutating_func,none,
       "cannot call mutating async function %0 on actor-isolated %1 %2",
       (DeclName, DescriptiveDeclKind, DeclName))
-ERROR(global_actor_from_instance_actor_context,none,
-      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
-      " from actor %3 %select{|in a synchronous context}5",
-      (DescriptiveDeclKind, DeclName, Type, DeclName, unsigned, bool))
-ERROR(global_actor_from_other_global_actor_context,none,
-      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
-      " from different global actor %3 %select{|in a synchronous context}5",
-      (DescriptiveDeclKind, DeclName, Type, Type, unsigned, bool))
-ERROR(global_actor_from_nonactor_context,none,
-      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
-      " from %select{this|a non-isolated}3%select{| synchronous}5 context",
-      (DescriptiveDeclKind, DeclName, Type, bool, unsigned, bool))
 ERROR(actor_isolated_call,none,
       "call to %0 function in a synchronous %1 context",
       (ActorIsolation, ActorIsolation))
@@ -4684,19 +4672,9 @@ WARNING(shared_mutable_state_access,none,
         "reference to %0 %1 is not concurrency-safe because it involves "
         "shared mutable state", (DescriptiveDeclKind, DeclName))
 ERROR(actor_isolated_witness,none,
-     "actor-isolated %0 %1 cannot be used to satisfy a protocol requirement",
-     (DescriptiveDeclKind, DeclName))
-ERROR(distributed_actor_isolated_witness,none,
-     "distributed actor-isolated %0 %1 cannot be used to satisfy a protocol requirement",
-     (DescriptiveDeclKind, DeclName))
-ERROR(global_actor_isolated_witness,none,
-      "%0 %1 isolated to global actor %2 can not satisfy corresponding "
-      "requirement from protocol %3",
-      (DescriptiveDeclKind, DeclName, Type, Identifier))
-ERROR(global_actor_isolated_requirement_witness_conflict,none,
-      "%0 %1 isolated to global actor %2 can not satisfy corresponding "
-      "requirement from protocol %3 isolated to global actor %4",
-      (DescriptiveDeclKind, DeclName, Type, Identifier, Type))
+     "%select{|distributed }0%1 %2 %3 cannot be used to satisfy %4 protocol "
+     "requirement",
+     (bool, ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
 ERROR(actor_cannot_conform_to_global_actor_protocol,none,
       "actor %0 cannot conform to global actor isolated protocol %1",
       (Type, Type))
@@ -4708,9 +4686,10 @@ ERROR(isolated_parameter_not_actor,none,
 
 WARNING(non_sendable_param_type,none,
         "non-sendable type %0 %select{passed in call to %4 %2 %3|"
+        "exiting %4 context in call to non-isolated %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,
@@ -4719,8 +4698,10 @@ WARNING(non_sendable_call_param_type,none,
         (Type, bool, ActorIsolation))
 WARNING(non_sendable_result_type,none,
         "non-sendable type %0 returned by %select{call to %4 %2 %3|"
+        "call from %4 context to non-isolated %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,
@@ -4730,8 +4711,10 @@ WARNING(non_sendable_call_result_type,none,
 WARNING(non_sendable_property_type,none,
         "non-sendable type %0 in %select{"
         "%select{asynchronous access to %5 %1 %2|"
+        "asynchronous access from %5 context to non-isolated %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/AST/Decl.cpp

@@ -9375,3 +9375,7 @@ void swift::simple_display(llvm::raw_ostream &out, AnyFunctionRef fn) {
   else
     out << "closure";
 }
+
+bool ActorIsolation::isDistributedActor() const {
+  return getKind() == ActorInstance && getActor()->isDistributedActor();
+}

lib/Sema/CSApply.cpp

@@ -7852,18 +7852,6 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
   return ctorCall;
 }
 
-/// Determine whether this closure should be treated as Sendable.
-static bool isSendableClosure(ConstraintSystem &cs,
-                              const AbstractClosureExpr *closure) {
-  if (auto fnType = cs.getType(const_cast<AbstractClosureExpr *>(closure))
-                        ->getAs<FunctionType>()) {
-    if (fnType->isSendable())
-      return true;
-  }
-
-  return false;
-}
-
 bool ExprRewriter::isDistributedThunk(ConcreteDeclRef ref, Expr *context) {
   auto *FD = dyn_cast_or_null<AbstractFunctionDecl>(ref.getDecl());
   if (!(FD && FD->isInstanceMember() && FD->isDistributed()))
@@ -7887,52 +7875,60 @@ bool ExprRewriter::isDistributedThunk(ConcreteDeclRef ref, Expr *context) {
 
   // If this is a method reference on an potentially isolated
   // actor then it cannot be a remote thunk.
-  if (isPotentiallyIsolatedActor(actor, [&](ParamDecl *P) {
-        return P->isIsolated() ||
-               llvm::is_contained(solution.isolatedParams, P);
-      }))
-    return false;
-
-  bool isInAsyncLetInitializer = target && target->isAsyncLetInitializer();
-
-  auto isActorInitOrDeInitContext = [&](const DeclContext *dc) {
-    return ::isActorInitOrDeInitContext(
-        dc, [&](const AbstractClosureExpr *closure) {
-          return isSendableClosure(cs, closure);
-        });
-  };
-
-  switch (ActorIsolationRestriction::forDeclaration(ref, dc)) {
-  case ActorIsolationRestriction::CrossActorSelf: {
-    // Not a thunk if it's used in actor init or de-init.
-    if (!isInAsyncLetInitializer && isActorInitOrDeInitContext(dc))
-      return false;
+  bool isPotentiallyIsolated = isPotentiallyIsolatedActor(
+      actor,
+    [&](ParamDecl *P) {
+    return P->isIsolated() ||
+           llvm::is_contained(solution.isolatedParams, P);
+  });
+
+  // Adjust the declaration context to the innermost context that is neither
+  // a local function nor a closure, so that the actor reference is checked
+  auto referenceDC = dc;
+  while (true) {
+    switch (referenceDC->getContextKind()) {
+    case DeclContextKind::AbstractClosureExpr:
+    case DeclContextKind::Initializer:
+    case DeclContextKind::SerializedLocal:
+      referenceDC = referenceDC->getParent();
+      continue;
 
-    // Here we know that the method could be used across actors
-    // and the actor it's used on is non-isolated, which means
-    // that it could be a thunk, so we have to be conservative
-    // about it.
-    return true;
-  }
+      case DeclContextKind::AbstractFunctionDecl:
+      case DeclContextKind::GenericTypeDecl:
+      case DeclContextKind::SubscriptDecl:
+        if (auto value = dyn_cast<ValueDecl>(referenceDC->getAsDecl())) {
+          if (value->isLocalCapture()) {
+            referenceDC = referenceDC->getParent();
+            continue;
+          }
+        }
+        break;
 
-  case ActorIsolationRestriction::ActorSelf: {
-    // An instance member of an actor can be referenced from an actor's
-    // designated initializer or deinitializer.
-    if (actor->isActorSelf() && !isInAsyncLetInitializer) {
-      if (auto *fn = isActorInitOrDeInitContext(dc)) {
-        if (!(isa<ConstructorDecl>(fn) &&
-              cast<ConstructorDecl>(fn)->isConvenienceInit()))
-          return false;
-      }
+      case DeclContextKind::EnumElementDecl:
+      case DeclContextKind::ExtensionDecl:
+      case DeclContextKind::FileUnit:
+      case DeclContextKind::Module:
+      case DeclContextKind::TopLevelCodeDecl:
+        break;
     }
 
-    // Call on a non-isolated actor in async context requires
-    // implicit thunk.
-    return isInAsyncLetInitializer || cs.isAsynchronousContext(dc);
+    break;
   }
 
-  default:
+  // Create a simple actor reference, assuming that we might be in a
+  // non-isolated context but knowing whether it's potentially isolated.
+  // We only care about the "distributed" flag.
+  ReferencedActor actorRef = ReferencedActor(
+      actor, isPotentiallyIsolated, ReferencedActor::NonIsolatedContext);
+  auto refResult = ActorReferenceResult::forReference(
+      ref, context->getLoc(), referenceDC, None, actorRef);
+  switch (refResult) {
+  case ActorReferenceResult::ExitsActorToNonisolated:
+  case ActorReferenceResult::SameConcurrencyDomain:
     return false;
+
+  case ActorReferenceResult::EntersActor:
+    return refResult.options.contains(ActorReferenceResult::Flags::Distributed);
   }
 }