Merge pull request #72332 from hborla/isolated-default-value-crash

[Concurrency] Don't attempt to hop to executor inside default argument generators and stored property initializers.

Author: hborla

lib/SILGen/SILGenApply.cpp

@@ -3045,35 +3045,47 @@ static void emitDelayedArguments(SILGenFunction &SGF,
 done:
 
   if (defaultArgIsolation) {
-    assert(SGF.F.isAsync());
     assert(!isolatedArgs.empty());
 
-    auto &firstArg = *std::get<0>(isolatedArgs[0]);
-    auto loc = firstArg.getDefaultArgLoc();
+    // Only hop to the default arg isolation if the callee is async.
+    // If we're in a synchronous function, the isolation has to match,
+    // so no hop is required. This is enforced by the actor isolation
+    // checker.
+    //
+    // FIXME: Note that we don't end up in this situation for user-written
+    // synchronous functions, because the default argument is only considered
+    // isolated to the callee if the call crosses an isolation boundary. We
+    // do end up here for default argument generators and stored property
+    // initializers. An alternative (and better) approach is to formally model
+    // those generator functions as isolated.
+    if (SGF.F.isAsync()) {
+      auto &firstArg = *std::get<0>(isolatedArgs[0]);
+      auto loc = firstArg.getDefaultArgLoc();
+
+      SILValue executor;
+      switch (*defaultArgIsolation) {
+      case ActorIsolation::GlobalActor:
+        executor = SGF.emitLoadGlobalActorExecutor(
+            defaultArgIsolation->getGlobalActor());
+        break;
 
-    SILValue executor;
-    switch (*defaultArgIsolation) {
-    case ActorIsolation::GlobalActor:
-      executor = SGF.emitLoadGlobalActorExecutor(
-          defaultArgIsolation->getGlobalActor());
-      break;
+      case ActorIsolation::ActorInstance:
+        llvm_unreachable("default arg cannot be actor instance isolated");
 
-    case ActorIsolation::ActorInstance:
-      llvm_unreachable("default arg cannot be actor instance isolated");
+      case ActorIsolation::Erased:
+        llvm_unreachable("default arg cannot have erased isolation");
 
-    case ActorIsolation::Erased:
-      llvm_unreachable("default arg cannot have erased isolation");
+      case ActorIsolation::Unspecified:
+      case ActorIsolation::Nonisolated:
+      case ActorIsolation::NonisolatedUnsafe:
+        llvm_unreachable("Not isolated");
+      }
 
-    case ActorIsolation::Unspecified:
-    case ActorIsolation::Nonisolated:
-    case ActorIsolation::NonisolatedUnsafe:
-      llvm_unreachable("Not isolated");
+      // Hop to the target isolation domain once to evaluate all
+      // default arguments.
+      SGF.emitHopToTargetExecutor(loc, executor);
     }
 
-    // Hop to the target isolation domain once to evaluate all
-    // default arguments.
-    SGF.emitHopToTargetExecutor(loc, executor);
-
     size_t argsEmitted = 0;
     for (auto &isolatedArg : isolatedArgs) {
       auto &delayedArg = *std::get<0>(isolatedArg);

test/Concurrency/isolated_default_argument_eval.swift

@@ -6,8 +6,12 @@
 @MainActor
 func requiresMainActor() -> Int { 0 }
 
+// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval19mainActorDefaultArg5valueS2i_tFfA_
+@discardableResult
 @MainActor
-func mainActorDefaultArg(value: Int = requiresMainActor()) {}
+func mainActorDefaultArg(value: Int = requiresMainActor()) -> Int {
+  value
+}
 
 @MainActor
 func mainActorMultiDefaultArg(x: Int = requiresMainActor(),
@@ -25,7 +29,7 @@ func mainActorMultiDefaultArg(x: Int = requiresMainActor(),
 func nonisolatedAsyncCaller() async {
   // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
   // CHECK: hop_to_executor {{.*}} : $MainActor
-  // CHECK: [[GET_VALUE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
+  // CHECK: [[GET_VALUE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueS2i_tFfA_
   // CHECK-NEXT: apply [[GET_VALUE]]()
   // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
   await mainActorDefaultArg()
@@ -68,3 +72,20 @@ func passInoutWithDefault() async {
   // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
   await isolatedDefaultInoutMix(x: &x, y: argValue)
 }
+
+// default argument 0 of noSuspensionInDefaultArgGenerator(_:)
+// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval33noSuspensionInDefaultArgGeneratoryySiFfA_
+// CHECK: [[NESTED_DEFAULT_REF:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueS2i_tFfA_ : $@convention(thin) () -> Int
+// CHECK-NEXT: [[NESTED_DEFAULT:%[0-9]+]] = apply [[NESTED_DEFAULT_REF]]() : $@convention(thin) () -> Int
+// CHECK: [[DEFAULT_REF:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueS2i_tF : $@convention(thin) (Int) -> Int
+// CHECK-NEXT: [[RESULT:%[0-9]+]] = apply [[DEFAULT_REF]]([[NESTED_DEFAULT]]) : $@convention(thin) (Int) -> Int
+// CHECK-NEXT: return [[RESULT]] : $Int
+
+// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval33noSuspensionInDefaultArgGeneratoryySiF
+@MainActor func noSuspensionInDefaultArgGenerator(
+  _ x: Int = mainActorDefaultArg()
+) {}
+
+func testNoSuspensionInDefaultArgGenerator() async {
+  await noSuspensionInDefaultArgGenerator()
+}