[Executors] Remote distributed actors get "crash on enqueue" default executor (#64969)

Author: ktoso

include/swift/AST/Decl.h

@@ -4535,7 +4535,6 @@ class ClassDecl final : public NominalTypeDecl {
 
   /// Fetch this class's unownedExecutor property, if it has one.
   const VarDecl *getUnownedExecutorProperty() const;
-  const VarDecl *getLocalUnownedExecutorProperty() const;
 
   /// Is this the NSObject class type?
   bool isNSObject() const;

include/swift/AST/KnownIdentifiers.def

@@ -231,8 +231,6 @@ IDENTIFIER(className)
 IDENTIFIER(_defaultActorInitialize)
 IDENTIFIER(_defaultActorDestroy)
 IDENTIFIER(unownedExecutor)
-IDENTIFIER(localUnownedExecutor)
-IDENTIFIER(_unwrapLocalUnownedExecutor)
 
 IDENTIFIER_(ErrorType)
 IDENTIFIER(Code)

include/swift/AST/KnownSDKDecls.def

@@ -21,7 +21,7 @@
 
 KNOWN_SDK_FUNC_DECL(Distributed, IsRemoteDistributedActor, "__isRemoteActor")
 KNOWN_SDK_FUNC_DECL(Distributed, IsLocalDistributedActor, "__isLocalActor")
-KNOWN_SDK_FUNC_DECL(Distributed, GetUnwrapLocalDistributedActorUnownedExecutor, "_getUnwrapLocalDistributedActorUnownedExecutor")
+KNOWN_SDK_FUNC_DECL(Distributed, BuildDefaultDistributedRemoteActorUnownedExecutor, "buildDefaultDistributedRemoteActorExecutor")
 
 #undef KNOWN_SDK_FUNC_DECL
 

lib/AST/Decl.cpp

@@ -9648,29 +9648,6 @@ const VarDecl *ClassDecl::getUnownedExecutorProperty() const {
   return nullptr;
 }
 
-const VarDecl *ClassDecl::getLocalUnownedExecutorProperty() const {
-  auto &C = getASTContext();
-
-  if (!isDistributedActor())
-    return nullptr;
-
-  llvm::SmallVector<ValueDecl *, 2> results;
-  this->lookupQualified(getSelfNominalTypeDecl(),
-                        DeclNameRef(C.Id_localUnownedExecutor),
-                        NL_ProtocolMembers,
-                        results);
-
-  for (auto candidate: results) {
-    if (isa<ProtocolDecl>(candidate->getDeclContext()))
-      continue;
-
-    if (VarDecl *var = dyn_cast<VarDecl>(candidate))
-      return var;
-  }
-
-  return nullptr;
-}
-
 bool ClassDecl::isRootDefaultActor() const {
   return isRootDefaultActor(getModuleContext(), ResilienceExpansion::Maximal);
 }

lib/SILOptimizer/Mandatory/LowerHopToActor.cpp

@@ -197,43 +197,11 @@ SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
     unmarkedExecutor =
       B.createBuiltin(loc, builtinName, resultType, subs, {actor});
 
-    // Otherwise, go through Actor.unownedExecutor.
-  } else if (actorType->isDistributedActor()) {
-    auto actorKind = KnownProtocolKind::DistributedActor;
-    auto actorProtocol = ctx.getProtocol(actorKind);
-    auto req = ctx.getGetUnwrapLocalDistributedActorUnownedExecutor();
-    assert(req && "Distributed library broken");
-    SILDeclRef fn(req, SILDeclRef::Kind::Func);
-
-    auto actorConf = module->lookupConformance(actorType, actorProtocol);
-    assert(actorConf &&
-           "hop_to_executor with distributed actor that doesn't conform to DistributedActor");
-
-    auto subs = SubstitutionMap::get(req->getGenericSignature(),
-                                     {actorType}, {actorConf});
-
-    // Find the unwrap function
-    FuncDecl *funcDecl = ctx.getGetUnwrapLocalDistributedActorUnownedExecutor();
-    assert(funcDecl);
-    auto funcDeclRef = SILDeclRef(funcDecl, SILDeclRef::Kind::Func);
-
-    SILFunction *unwrapExecutorFun =
-        functionBuilder.getOrCreateFunction(
-            loc, funcDeclRef, ForDefinition_t::NotForDefinition);
-    assert(unwrapExecutorFun && "no sil function!");
-    auto funcRef =
-        B.createFunctionRef(loc, unwrapExecutorFun);
-    auto witnessCall = B.createApply(loc, funcRef, subs, {actor});
-
-    // The protocol requirement returns an Optional<UnownedSerialExecutor>;
-    // extract the Builtin.Executor from it.
-    auto executorDecl = ctx.getUnownedSerialExecutorDecl();
-    auto executorProps = executorDecl->getStoredProperties();
-    assert(executorProps.size() == 1);
-    unmarkedExecutor =
-      B.createStructExtract(loc, witnessCall, executorProps[0]);
+    // Otherwise, go through (Distributed)Actor.unownedExecutor.
   } else {
-    auto actorKind = KnownProtocolKind::Actor;
+    auto actorKind = actorType->isDistributedActor() ?
+                     KnownProtocolKind::DistributedActor :
+                     KnownProtocolKind::Actor;
     auto actorProtocol = ctx.getProtocol(actorKind);
     auto req = getUnownedExecutorGetter(ctx, actorProtocol);
     assert(req && "Concurrency library broken");

lib/Sema/DerivedConformanceDistributedActor.cpp

@@ -621,13 +621,13 @@ static Expr *constructDistributedUnownedSerialExecutor(ASTContext &ctx,
 }
 
 static std::pair<BraceStmt *, bool>
-deriveBodyDistributedActor_localUnownedExecutor(AbstractFunctionDecl *getter, void *) {
-  // var localUnownedExecutor: UnownedSerialExecutor? {
+deriveBodyDistributedActor_unownedExecutor(AbstractFunctionDecl *getter, void *) {
+  // var unownedExecutor: UnownedSerialExecutor {
   //   get {
   //     guard __isLocalActor(self) else {
-  //       return nil
+  //       return buildDefaultDistributedRemoteActorExecutor(self)
   //     }
-  //     return Optional(Builtin.buildDefaultActorExecutorRef(self))
+  //     return Builtin.buildDefaultActorExecutorRef(self)
   //   }
   // }
   ASTContext &ctx = getter->getASTContext();
@@ -657,7 +657,7 @@ deriveBodyDistributedActor_localUnownedExecutor(AbstractFunctionDecl *getter, vo
   if (!initCall) return failure();
 
   // guard __isLocalActor(self) else {
-  //   return nil
+  //   return buildDefaultDistributedRemoteActorExecutor(self)
   // }
   auto isLocalActorDecl = ctx.getIsLocalDistributedActor();
   DeclRefExpr *isLocalActorExpr =
@@ -673,22 +673,71 @@ deriveBodyDistributedActor_localUnownedExecutor(AbstractFunctionDecl *getter, vo
   CallExpr *isLocalActorCall = CallExpr::createImplicit(ctx, isLocalActorExpr, argListForIsLocal);
   isLocalActorCall->setType(ctx.getBoolType());
   isLocalActorCall->setThrows(false);
-  auto returnNilIfRemoteStmt = DerivedConformance::returnNilIfFalseGuardTypeChecked(
-      ctx, isLocalActorCall, /*optionalWrappedType=*/initCall->getType());
 
+  GuardStmt* guardElseRemoteReturnExec;
+  {
+    // Find the buildDefaultDistributedRemoteActorExecutor method
+    auto buildRemoteExecutorDecl =
+        ctx.getBuildDefaultDistributedRemoteActorUnownedExecutor();
+    assert(buildRemoteExecutorDecl && "cannot find buildDefaultDistributedRemoteActorExecutor");
+    auto substitutions = SubstitutionMap::get(
+        buildRemoteExecutorDecl->getGenericSignature(),
+        [&](SubstitutableType *dependentType) {
+          if (auto gp = dyn_cast<GenericTypeParamType>(dependentType)) {
+            if (gp->getDepth() == 0 && gp->getIndex() == 0) {
+              return getter->getImplicitSelfDecl()->getType();
+            }
+          }
 
-  // Finalize preparing the unowned executor for returning.
-  auto wrappedCall = new (ctx) InjectIntoOptionalExpr(initCall, initCall->getType()->wrapInOptionalType());
+          return Type();
+        },
+        LookUpConformanceInModule(getter->getParentModule())
+    );
+    DeclRefExpr *buildRemoteExecutorExpr =
+        new (ctx) DeclRefExpr(
+            ConcreteDeclRef(buildRemoteExecutorDecl, substitutions),
+            DeclNameLoc(),/*implicit=*/true,
+            AccessSemantics::Ordinary);
+    buildRemoteExecutorExpr->setType(
+        buildRemoteExecutorDecl->getInterfaceType()
+         .subst(substitutions)
+        );
+
+    Expr *selfForBuildRemoteExecutor = DerivedConformance::createSelfDeclRef(getter);
+    selfForBuildRemoteExecutor->setType(selfType);
+    auto *argListForBuildRemoteExecutor =
+        ArgumentList::forImplicitCallTo(buildRemoteExecutorDecl->getParameters(),
+                                        /*argExprs=*/{selfForBuildRemoteExecutor}, ctx);
+    CallExpr *buildRemoteExecutorCall = CallExpr::createImplicit(ctx, buildRemoteExecutorExpr,
+                                                                 argListForBuildRemoteExecutor);
+    buildRemoteExecutorCall->setType(ctx.getUnownedSerialExecutorType());
+    buildRemoteExecutorCall->setThrows(false);
+
+    SmallVector<ASTNode, 1> statements = {
+        new(ctx) ReturnStmt(SourceLoc(), buildRemoteExecutorCall)
+    };
+
+    SmallVector<StmtConditionElement, 1> conditions = {
+        isLocalActorCall
+    };
+
+    // Build and return the complete guard statement.
+    guardElseRemoteReturnExec =
+        new(ctx) GuardStmt(SourceLoc(), ctx.AllocateCopy(conditions),
+                           BraceStmt::create(ctx, SourceLoc(), statements, SourceLoc()));
+  }
 
-  auto ret = new (ctx) ReturnStmt(SourceLoc(), wrappedCall, /*implicit*/ true);
+  // Finalize preparing the unowned executor for returning.
+  // auto wrappedCall = new (ctx) InjectIntoOptionalExpr(initCall, initCall->getType());
+  auto returnDefaultExec = new (ctx) ReturnStmt(SourceLoc(), initCall, /*implicit=*/true);
 
   auto body = BraceStmt::create(
-      ctx, SourceLoc(), { returnNilIfRemoteStmt, ret }, SourceLoc(), /*implicit=*/true);
+      ctx, SourceLoc(), { guardElseRemoteReturnExec, returnDefaultExec }, SourceLoc(), /*implicit=*/true);
   return { body, /*isTypeChecked=*/true };
 }
 
-/// Derive the declaration of DistributedActor's localUnownedExecutor property.
-static ValueDecl *deriveDistributedActor_localUnownedExecutor(DerivedConformance &derived) {
+/// Derive the declaration of DistributedActor's unownedExecutor property.
+static ValueDecl *deriveDistributedActor_unownedExecutor(DerivedConformance &derived) {
   ASTContext &ctx = derived.Context;
 
   // Retrieve the types and declarations we'll need to form this operation.
@@ -699,11 +748,10 @@ static ValueDecl *deriveDistributedActor_localUnownedExecutor(DerivedConformance
     return nullptr;
   }
   Type executorType = executorDecl->getDeclaredInterfaceType();
-  Type optionalExecutorType = executorType->wrapInOptionalType();
 
   if (auto classDecl = dyn_cast<ClassDecl>(derived.Nominal)) {
-    if (auto existing = classDecl->getLocalUnownedExecutorProperty()) {
-      if (existing->getInterfaceType()->isEqual(optionalExecutorType)) {
+    if (auto existing = classDecl->getUnownedExecutorProperty()) {
+      if (existing->getInterfaceType()->isEqual(executorType)) {
         return const_cast<VarDecl *>(existing);
       } else {
         // bad type, should be diagnosed elsewhere
@@ -713,8 +761,8 @@ static ValueDecl *deriveDistributedActor_localUnownedExecutor(DerivedConformance
   }
 
   auto propertyPair = derived.declareDerivedProperty(
-      DerivedConformance::SynthesizedIntroducer::Var, ctx.Id_localUnownedExecutor,
-      optionalExecutorType, optionalExecutorType,
+      DerivedConformance::SynthesizedIntroducer::Var, ctx.Id_unownedExecutor,
+      executorType, executorType,
       /*static*/ false, /*final*/ false);
   auto property = propertyPair.first;
   property->setSynthesized(true);
@@ -738,8 +786,8 @@ static ValueDecl *deriveDistributedActor_localUnownedExecutor(DerivedConformance
       property, asAvailableAs, ctx);
 
   auto getter =
-      derived.addGetterToReadOnlyDerivedProperty(property, optionalExecutorType);
-  getter->setBodySynthesizer(deriveBodyDistributedActor_localUnownedExecutor);
+      derived.addGetterToReadOnlyDerivedProperty(property, executorType);
+  getter->setBodySynthesizer(deriveBodyDistributedActor_unownedExecutor);
 
   // IMPORTANT: MUST BE AFTER [id, actorSystem].
   if (auto id = derived.Nominal->getDistributedActorIDProperty()) {
@@ -782,24 +830,24 @@ static void assertRequiredSynthesizedPropertyOrder(DerivedConformance &derived,
     if (auto id = Nominal->getDistributedActorIDProperty()) {
       if (auto system = Nominal->getDistributedActorSystemProperty()) {
         if (auto classDecl = dyn_cast<ClassDecl>(derived.Nominal)) {
-          if (auto localUnownedExecutor = classDecl->getLocalUnownedExecutorProperty()) {
-            int idIdx, actorSystemIdx, localUnownedExecutorIdx = 0;
+          if (auto unownedExecutor = classDecl->getUnownedExecutorProperty()) {
+            int idIdx, actorSystemIdx, unownedExecutorIdx = 0;
             int idx = 0;
             for (auto member: Nominal->getMembers()) {
               if (auto binding = dyn_cast<PatternBindingDecl>(member)) {
                 if (binding->getSingleVar()->getName() == Context.Id_id) {
                   idIdx = idx;
                 } else if (binding->getSingleVar()->getName() == Context.Id_actorSystem) {
                   actorSystemIdx = idx;
-                } else if (binding->getSingleVar()->getName() == Context.Id_localUnownedExecutor) {
-                  localUnownedExecutorIdx = idx;
+                } else if (binding->getSingleVar()->getName() == Context.Id_unownedExecutor) {
+                  unownedExecutorIdx = idx;
                 }
                 idx += 1;
               }
             }
-            if (idIdx + actorSystemIdx + localUnownedExecutorIdx >= 0 + 1 + 2) {
+            if (idIdx + actorSystemIdx + unownedExecutorIdx >= 0 + 1 + 2) {
               // we have found all the necessary fields, let's assert their order
-              assert(idIdx < actorSystemIdx < localUnownedExecutorIdx && "order of fields MUST be exact.");
+              assert(idIdx < actorSystemIdx < unownedExecutorIdx && "order of fields MUST be exact.");
             }
           }
         }
@@ -821,8 +869,8 @@ ValueDecl *DerivedConformance::deriveDistributedActor(ValueDecl *requirement) {
       derivedValue = deriveDistributedActor_id(*this);
     } else if (var->getName() == Context.Id_actorSystem) {
       derivedValue = deriveDistributedActor_actorSystem(*this);
-    } else if (var->getName() == Context.Id_localUnownedExecutor) {
-      derivedValue = deriveDistributedActor_localUnownedExecutor(*this);
+    } else if (var->getName() == Context.Id_unownedExecutor) {
+      derivedValue = deriveDistributedActor_unownedExecutor(*this);
     }
 
     assertRequiredSynthesizedPropertyOrder(*this, derivedValue);

lib/Sema/DerivedConformances.cpp

@@ -335,7 +335,11 @@ ValueDecl *DerivedConformance::getDerivableRequirement(NominalTypeDecl *nominal,
 
     // Actor.unownedExecutor
     if (name.isSimpleName(ctx.Id_unownedExecutor)) {
-      return getRequirement(KnownProtocolKind::Actor);
+      if (nominal->isDistributedActor()) {
+        return getRequirement(KnownProtocolKind::DistributedActor);
+      } else {
+        return getRequirement(KnownProtocolKind::Actor);
+      }
     }
 
     // DistributedActor.id
@@ -346,11 +350,6 @@ ValueDecl *DerivedConformance::getDerivableRequirement(NominalTypeDecl *nominal,
     if (name.isSimpleName(ctx.Id_actorSystem))
       return getRequirement(KnownProtocolKind::DistributedActor);
 
-    // DistributedActor.localUnownedExecutor
-    if (name.isSimpleName(ctx.Id_localUnownedExecutor)) {
-      return getRequirement(KnownProtocolKind::DistributedActor);
-    }
-
     return nullptr;
   }
 
@@ -693,11 +692,6 @@ GuardStmt *DerivedConformance::returnNilIfFalseGuardTypeChecked(ASTContext &C,
   statements.push_back(returnStmt);
 
   // Next, generate the condition being checked.
-//  auto cmpFuncExpr = new (C) UnresolvedDeclRefExpr(
-//      DeclNameRef(C.Id_EqualsOperator), DeclRefKind::BinaryOperator,
-//      DeclNameLoc());
-//  auto *cmpExpr = BinaryExpr::create(C, lhsExpr, cmpFuncExpr, rhsExpr,
-//      /*implicit*/ true);
   conditions.emplace_back(testExpr);
 
   // Build and return the complete guard statement.

lib/Sema/TypeCheckConcurrency.cpp

@@ -221,16 +221,6 @@ bool IsDefaultActorRequest::evaluate(
         executorProperty->getAttrs().hasSemanticsAttr(SEMANTICS_DEFAULT_ACTOR);
   }
 
-  // Maybe it was a distributed actor, let's double-check it's localUnownedExecutor property.
-  // If we synthesized that one with appropriate semantics we may still be a default actor.
-  if (!isDefaultActor && classDecl->isDistributedActor()) {
-    if (auto localExecutorProperty = classDecl->getLocalUnownedExecutorProperty()) {
-      foundExecutorPropertyImpl = true;
-      isDefaultActor = isDefaultActor ||
-          localExecutorProperty->getAttrs().hasSemanticsAttr(SEMANTICS_DEFAULT_ACTOR);
-    }
-  }
-
   // Only if we found one of the executor properties, do we return the status of default or not,
   // based on the findings of the semantics attribute of that located property.
   if (foundExecutorPropertyImpl) {

lib/Sema/TypeCheckDeclOverride.cpp

@@ -2039,7 +2039,6 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
               cast<ClassDecl>(prop->getDeclContext())->isActor() &&
               !prop->isStatic() &&
               prop->getName() == ctx.Id_unownedExecutor &&
-              prop->getName() == ctx.Id_localUnownedExecutor &&
               prop->getInterfaceType()->getAnyNominal() == ctx.getUnownedSerialExecutorDecl());
     };
 

stdlib/public/Concurrency/DiscardingTaskGroup.swift

@@ -407,7 +407,7 @@ extension DiscardingTaskGroup: Sendable { }
 /// out of the `withThrowingDiscardingTaskGroup` method when it returns.
 ///
 /// ```
-/// try await withThrowingDiscardingTaskGroup() { group in
+/// try await withThrowingDiscardingTaskGroup { group in
 ///   group.addTask { try boom(1) }
 ///   group.addTask { try boom(2, after: .seconds(5)) }
 ///   group.addTask { try boom(3, after: .seconds(5)) }
@@ -490,7 +490,7 @@ public func withThrowingDiscardingTaskGroup<GroupResult>(
 /// A throwing discarding task group becomes cancelled in one of the following ways:
 ///
 /// - when ``cancelAll()`` is invoked on it,
-/// - when an error is thrown out of the `withThrowingDiscardingTaskGroup(...) { }` closure,
+/// - when an error is thrown out of the `withThrowingDiscardingTaskGroup { ... }` closure,
 /// - when the ``Task`` running this task group is cancelled.
 ///
 /// But also, and uniquely in *discarding* task groups:
@@ -598,7 +598,7 @@ public struct ThrowingDiscardingTaskGroup<Failure: Error> {
 
   /// A Boolean value that indicates whether the group has any remaining tasks.
   ///
-  /// At the start of the body of a `withThrowingDiscardingTaskGroup(of:returning:body:)` call,
+  /// At the start of the body of a `withThrowingDiscardingTaskGroup(returning:body:)` call,
   /// the task group is always empty.
   ///
   /// It's guaranteed to be empty when returning from that body