Merge pull request #64740 from ktoso/pick-cleanup-hop-to-distributed-custom-executor

Author: ktoso

include/swift/AST/ASTContext.h

@@ -891,6 +891,9 @@ class ASTContext final {
   /// Get the back-deployed availability for concurrency.
   AvailabilityContext getBackDeployedConcurrencyAvailability();
 
+  /// The the availability since when distributed actors are able to have custom executors.
+  AvailabilityContext getConcurrencyDistributedActorWithCustomExecutorAvailability();
+
   /// Get the runtime availability of support for differentiation.
   AvailabilityContext getDifferentiationAvailability();
 
@@ -934,6 +937,14 @@ class ASTContext final {
   /// compiler for the target platform.
   AvailabilityContext getSwift57Availability();
 
+  /// Get the runtime availability of features introduced in the Swift 5.8
+  /// compiler for the target platform.
+  AvailabilityContext getSwift58Availability();
+
+  /// Get the runtime availability of features introduced in the Swift 5.9
+  /// compiler for the target platform.
+  AvailabilityContext getSwift59Availability();
+
   // Note: Update this function if you add a new getSwiftXYAvailability above.
   /// Get the runtime availability for a particular version of Swift (5.0+).
   AvailabilityContext

include/swift/AST/Availability.h

@@ -329,6 +329,11 @@ class AvailabilityContext {
   bool isAvailableAsSPI() const {
     return SPI && *SPI;
   }
+
+  /// Returns a representation of this range as a string for debugging purposes.
+  std::string getAsString() const {
+    return "AvailabilityContext(" + OSVersion.getAsString() + (isAvailableAsSPI() ? ", spi" : "") + ")";
+  }
 };
 
 

include/swift/AST/KnownSDKDecls.def

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

lib/AST/Availability.cpp

@@ -489,6 +489,10 @@ AvailabilityContext ASTContext::getBackDeployedConcurrencyAvailability() {
   return getSwift51Availability();
 }
 
+AvailabilityContext ASTContext::getConcurrencyDistributedActorWithCustomExecutorAvailability() {
+  return getSwift59Availability();
+}
+
 AvailabilityContext ASTContext::getDifferentiationAvailability() {
   return getSwiftFutureAvailability();
 }
@@ -642,6 +646,28 @@ AvailabilityContext ASTContext::getSwift57Availability() {
   }
 }
 
+AvailabilityContext ASTContext::getSwift58Availability() {
+  auto target = LangOpts.Target;
+
+  if (target.isMacOSX()) {
+    return AvailabilityContext(
+        VersionRange::allGTE(llvm::VersionTuple(13, 3, 0)));
+  } else if (target.isiOS()) {
+    return AvailabilityContext(
+        VersionRange::allGTE(llvm::VersionTuple(16, 4, 0)));
+  } else if (target.isWatchOS()) {
+    return AvailabilityContext(
+        VersionRange::allGTE(llvm::VersionTuple(9, 4, 0)));
+  } else {
+    return AvailabilityContext::alwaysAvailable();
+  }
+}
+
+AvailabilityContext ASTContext::getSwift59Availability() {
+  // TODO: Update Availability impl when Swift 5.9 is released
+  return getSwiftFutureAvailability();
+}
+
 AvailabilityContext ASTContext::getSwiftFutureAvailability() {
   auto target = LangOpts.Target;
 
@@ -671,6 +697,8 @@ ASTContext::getSwift5PlusAvailability(llvm::VersionTuple swiftVersion) {
     case 5: return getSwift55Availability();
     case 6: return getSwift56Availability();
     case 7: return getSwift57Availability();
+    case 8: return getSwift58Availability();
+    case 9: return getSwift59Availability();
     default: break;
     }
   }

lib/SILOptimizer/Mandatory/LowerHopToActor.cpp

@@ -15,6 +15,7 @@
 #include "swift/SIL/SILFunction.h"
 #include "swift/SIL/Dominance.h"
 #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
+#include "swift/SILOptimizer/Utils/SILOptFunctionBuilder.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "llvm/ADT/ScopedHashTable.h"
 
@@ -49,19 +50,26 @@ namespace {
 class LowerHopToActor {
   SILFunction *F;
   DominanceInfo *Dominance;
+  SILOptFunctionBuilder &functionBuilder;
 
   /// A map from an actor value to the executor we've derived for it.
   llvm::ScopedHashTable<SILValue, SILValue> ExecutorForActor;
 
   bool processHop(HopToExecutorInst *hop);
   bool processExtract(ExtractExecutorInst *extract);
 
-  SILValue emitGetExecutor(SILBuilderWithScope &B, SILLocation loc,
+  SILValue emitGetExecutor(SILBuilderWithScope &B,
+                           SILLocation loc,
                            SILValue actor, bool makeOptional);
 
 public:
-  LowerHopToActor(SILFunction *f, DominanceInfo *dominance)
-    : F(f), Dominance(dominance) { }
+  LowerHopToActor(SILFunction *f,
+                  SILOptFunctionBuilder &FunctionBuilder,
+                  DominanceInfo *dominance)
+    : F(f),
+      Dominance(dominance),
+      functionBuilder(FunctionBuilder)
+      { }
 
   /// The entry point to the transformation.
   bool run();
@@ -154,28 +162,6 @@ static AccessorDecl *getUnownedExecutorGetter(ASTContext &ctx,
   return nullptr;
 }
 
-static AccessorDecl *getUnwrapLocalUnownedExecutorGetter(ASTContext &ctx,
-                                              ProtocolDecl *actorProtocol) {
-  for (auto member: actorProtocol->getAllMembers()) { // FIXME: remove this, just go to the extension
-    if (auto var = dyn_cast<VarDecl>(member)) {
-      if (var->getName() == ctx.Id__unwrapLocalUnownedExecutor)
-        return var->getAccessor(AccessorKind::Get);
-    }
-  }
-
-  for (auto extension: actorProtocol->getExtensions()) {
-    for (auto member: extension->getAllMembers()) {
-      if (auto var = dyn_cast<VarDecl>(member)) {
-        if (var->getName() == ctx.Id__unwrapLocalUnownedExecutor) {
-          return var->getAccessor(AccessorKind::Get);
-        }
-      }
-    }
-  }
-
-  return nullptr;
-}
-
 SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
                                           SILLocation loc, SILValue actor,
                                           bool makeOptional) {
@@ -199,6 +185,9 @@ SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
   // If the actor type is a default actor, go ahead and devirtualize here.
   auto module = F->getModule().getSwiftModule();
   SILValue unmarkedExecutor;
+
+  // Determine if the actor is a "default actor" in which case we'll build a default
+  // actor executor ref inline, rather than calling out to the user-provided executor function.
   if (isDefaultActorType(actorType, module, F->getResilienceExpansion())) {
     auto builtinName = ctx.getIdentifier(
       getBuiltinName(BuiltinValueKind::BuildDefaultActorExecutorRef));
@@ -212,7 +201,7 @@ SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
   } else if (actorType->isDistributedActor()) {
     auto actorKind = KnownProtocolKind::DistributedActor;
     auto actorProtocol = ctx.getProtocol(actorKind);
-    auto req = getUnwrapLocalUnownedExecutorGetter(ctx, actorProtocol);
+    auto req = ctx.getGetUnwrapLocalDistributedActorUnownedExecutor();
     assert(req && "Distributed library broken");
     SILDeclRef fn(req, SILDeclRef::Kind::Func);
 
@@ -222,12 +211,19 @@ SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
 
     auto subs = SubstitutionMap::get(req->getGenericSignature(),
                                      {actorType}, {actorConf});
-    auto fnType = F->getModule().Types.getConstantFunctionType(*F, fn);
 
-    auto witness =
-      B.createWitnessMethod(loc, actorType, actorConf, fn,
-                            SILType::getPrimitiveObjectType(fnType));
-    auto witnessCall = B.createApply(loc, witness, subs, {actor});
+    // 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.
@@ -286,7 +282,8 @@ class LowerHopToActorPass : public SILFunctionTransform {
   void run() override {
     auto fn = getFunction();
     auto domTree = getAnalysis<DominanceAnalysis>()->get(fn);
-    LowerHopToActor pass(getFunction(), domTree);
+    auto functionBuilder = SILOptFunctionBuilder(*this);
+    LowerHopToActor pass(getFunction(), functionBuilder, domTree);
     if (pass.run())
       invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
   }

lib/Sema/TypeCheckConcurrency.cpp

@@ -182,6 +182,27 @@ bool IsDefaultActorRequest::evaluate(
   if (!classDecl->isActor())
     return false;
 
+  // Distributed actors were not able to have custom executors until Swift 5.9,
+  // so in order to avoid wrongly treating a resilient distributed actor from another
+  // module as not-default we need to handle this case explicitly.
+  if (classDecl->isDistributedActor()) {
+    ASTContext &ctx = classDecl->getASTContext();
+    auto customExecutorAvailability =
+        ctx.getConcurrencyDistributedActorWithCustomExecutorAvailability();
+
+    auto actorAvailability = TypeChecker::overApproximateAvailabilityAtLocation(
+        classDecl->getStartLoc(),
+        classDecl);
+
+    if (!actorAvailability.isContainedIn(customExecutorAvailability)) {
+      // Any 'distributed actor' declared with availability lower than the
+      // introduction of custom executors for distributed actors, must be treated as default actor,
+      // even if it were to declared the unowned executor property, as older compilers
+      // do not have the the logic to handle that case.
+      return true;
+    }
+  }
+
   // If the class is resilient from the perspective of the module
   // module, it's not a default actor.
   if (classDecl->isForeign() || classDecl->isResilient(M, expansion))

stdlib/public/Distributed/DistributedActor.swift

@@ -278,19 +278,15 @@ public protocol DistributedActor: AnyActor, Identifiable, Hashable
   /// - Parameter system: `system` which should be used to resolve the `identity`, and be associated with the returned actor
   static func resolve(id: ID, using system: ActorSystem) throws -> Self
 
-  // FIXME: figure out how to remove this so LowerHopToActor can call the extension method directly on the protocol
-  @available(SwiftStdlib 5.9, *)
-  var _unwrapLocalUnownedExecutor: UnownedSerialExecutor { get }
 }
 
-
 @available(SwiftStdlib 5.9, *)
-extension DistributedActor {
-
-  @available(SwiftStdlib 5.9, *)
-  public var _unwrapLocalUnownedExecutor: UnownedSerialExecutor {
-    self.localUnownedExecutor!
+public func _getUnwrapLocalDistributedActorUnownedExecutor(_ actor: some DistributedActor) -> UnownedSerialExecutor {
+  guard let executor = actor.localUnownedExecutor else {
+    fatalError("Expected distributed actor executor to be not nil!")
   }
+
+  return executor
 }
 
 // ==== Hashable conformance ---------------------------------------------------

test/Distributed/Runtime/distributed_actor_assume_executor.swift

@@ -39,6 +39,7 @@ func check(actor: MainDistributedFriend) {
   checkAssumeMainActor(actor: actor)
 }
 
+@available(SwiftStdlib 5.9, *)
 distributed actor MainDistributedFriend {
   nonisolated var localUnownedExecutor: UnownedSerialExecutor? {
     print("get unowned executor")

test/Distributed/Runtime/distributed_actor_custom_executor_availability.swift

@@ -0,0 +1,121 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
+// RUN: %target-build-swift -Xfrontend -disable-availability-checking -parse-as-library -I %t %s %S/../Inputs/FakeDistributedActorSystems.swift -o %t/a.out
+// RUN: %target-codesign %t/a.out
+// RUN:  %target-run %t/a.out
+
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: distributed
+// REQUIRES: concurrency_runtime
+// UNSUPPORTED: back_deployment_runtime
+
+// UNSUPPORTED: back_deploy_concurrency
+// UNSUPPORTED: use_os_stdlib
+// UNSUPPORTED: freestanding
+
+import StdlibUnittest
+import Distributed
+import FakeDistributedActorSystems
+
+@available(SwiftStdlib 5.7, *)
+typealias DefaultDistributedActorSystem = LocalTestingDistributedActorSystem
+
+@available(SwiftStdlib 5.7, *)
+distributed actor FiveSevenActor_NothingExecutor {
+  nonisolated var localUnownedExecutor: UnownedSerialExecutor? {
+    print("get unowned executor")
+    return MainActor.sharedUnownedExecutor
+  }
+
+  distributed func test(x: Int) async throws {
+    print("executed: \(#function)")
+    defer {
+      print("done executed: \(#function)")
+    }
+    assumeOnMainActorExecutor {
+      // ignore
+    }
+  }
+}
+
+@available(SwiftStdlib 5.9, *)
+distributed actor FiveNineActor_NothingExecutor {
+//  @available(SwiftStdlib 5.9, *) // because of `localUnownedExecutor`
+  nonisolated var localUnownedExecutor: UnownedSerialExecutor? {
+    print("get unowned executor")
+    return MainActor.sharedUnownedExecutor
+  }
+
+  distributed func test(x: Int) async throws {
+    print("executed: \(#function)")
+    defer {
+      print("done executed: \(#function)")
+    }
+    assumeOnMainActorExecutor {
+      // ignore
+    }
+  }
+}
+
+@available(SwiftStdlib 5.7, *)
+distributed actor FiveSevenActor_FiveNineExecutor {
+  @available(SwiftStdlib 5.9, *)
+  nonisolated var localUnownedExecutor: UnownedSerialExecutor? {
+    print("get unowned executor")
+    return MainActor.sharedUnownedExecutor
+  }
+
+  distributed func test(x: Int) async throws {
+    print("executed: \(#function)")
+    defer {
+      print("done executed: \(#function)")
+    }
+    assumeOnMainActorExecutor {
+      // ignore
+    }
+  }
+}
+
+@main struct Main {
+  static func main() async {
+    if #available(SwiftStdlib 5.9, *) {
+      let tests = TestSuite("DistributedActorExecutorAvailability")
+
+      let system = LocalTestingDistributedActorSystem()
+
+      #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
+      tests.test("5.7 actor, no availability executor property => no custom executor") {
+        expectCrashLater(withMessage: "Fatal error: Incorrect actor executor assumption; Expected 'MainActor' executor.")
+        try! await FiveSevenActor_NothingExecutor(actorSystem: system).test(x: 42)
+      }
+
+      tests.test("5.9 actor, no availability executor property => custom executor") {
+        try! await FiveNineActor_NothingExecutor(actorSystem: system).test(x: 42)
+      }
+
+      tests.test("5.7 actor, 5.9 executor property => no custom executor") {
+        expectCrashLater(withMessage: "Fatal error: Incorrect actor executor assumption; Expected 'MainActor' executor.")
+        try! await FiveSevenActor_FiveNineExecutor(actorSystem: system).test(x: 42)
+      }
+      #else
+      // On non-apple platforms the SDK comes with the toolchains,
+      // so the feature works because we're executing in a 5.9 context already,
+      // which otherwise could not have been compiled
+      tests.test("non apple platform: 5.7 actor, no availability executor property => no custom executor") {
+        try! await FiveSevenActor_NothingExecutor(actorSystem: system).test(x: 42)
+      }
+
+      tests.test("non apple platform: 5.9 actor, no availability executor property => custom executor") {
+        try! await FiveNineActor_NothingExecutor(actorSystem: system).test(x: 42)
+      }
+
+      tests.test("non apple platform: 5.7 actor, 5.9 executor property => no custom executor") {
+        try! await FiveSevenActor_FiveNineExecutor(actorSystem: system).test(x: 42)
+      }
+      #endif
+
+      await runAllTestsAsync()
+    }
+  }
+}