Fix argument index calculations for isolated default arguments

lib/SILGen/SILGenApply.cpp

@@ -3039,13 +3039,22 @@ static void emitDelayedArguments(SILGenFunction &SGF,
   // up in parallel, with empty spots being dropped into 'args'
   // wherever there's a delayed argument to insert.
   //
+  // Note that siteArgs has exactly one empty element for each
+  // delayed argument, even if the argument actually expands to
+  // zero or multiple SIL values. In such cases, we may have to
+  // remove or add elements to fill in the actual argument values.
+  //
   // Note that this also begins the formal accesses in evaluation order.
   for (auto argsIt = args.begin(); argsIt != args.end(); ++argsIt) {
     auto &siteArgs = *argsIt;
     // NB: siteArgs.size() may change during iteration
     for (size_t i = 0; i < siteArgs.size(); ) {
       auto &siteArg = siteArgs[i];
 
+      // Skip slots in the argument array that we've already filled in.
+      // Note that this takes advantage of the "exactly one element"
+      // assumption above, since default arguments might have () type
+      // and therefore expand to no actual argument slots.
       if (siteArg) {
         ++i;
         continue;
@@ -3056,6 +3065,7 @@ static void emitDelayedArguments(SILGenFunction &SGF,
 
       if (defaultArgIsolation && delayedArg.isDefaultArg()) {
         isolatedArgs.push_back(std::make_tuple(delayedNext, argsIt, i));
+        ++i;
         if (++delayedNext == delayedArgs.end()) {
           goto done;
         } else {
@@ -3126,14 +3136,44 @@ static void emitDelayedArguments(SILGenFunction &SGF,
       SGF.emitHopToTargetExecutor(loc, executor);
     }
 
-    size_t argsEmitted = 0;
+    // The index saved in isolatedArgs is the index where we're
+    // supposed to fill in the argument in siteArgs. It should point
+    // to a single null element, which we will replace with zero or
+    // more actual argument values. This replacement can invalidate
+    // later indices, so we need to apply an adjustment as we go.
+    //
+    // That adjustment only applies within the right siteArgs and
+    // must be reset when we change siteArgs. Fortunately, emission
+    // is always left-to-right and never revisits a siteArgs.
+    size_t indexAdjustment = 0;
+    auto indexAdjustmentSite = args.begin();
+
     for (auto &isolatedArg : isolatedArgs) {
       auto &delayedArg = *std::get<0>(isolatedArg);
-      auto &siteArgs = *std::get<1>(isolatedArg);
-      auto argIndex = std::get<2>(isolatedArg) + argsEmitted;
-      auto origIndex = argIndex;
+      auto site = std::get<1>(isolatedArg);
+      auto &siteArgs = *site;
+      size_t argIndex = std::get<2>(isolatedArg);
+
+      // Apply the current index adjustment if we haven't changed
+      // sites.
+      if (indexAdjustmentSite == site) {
+        argIndex += indexAdjustment;
+
+      // Otherwise, reset the current index adjustment.
+      } else {
+        indexAdjustment = 0;
+      }
+
+      assert(!siteArgs[argIndex] &&
+             "overwriting an existing arg during delayed isolated "
+             "argument emission");
+
+      size_t origIndex = argIndex;
       delayedArg.emit(SGF, siteArgs, argIndex);
-      argsEmitted += (argIndex - origIndex);
+
+      // Accumulate the adjustment. Note that the adjustment can
+      // be negative, and we end up relying on modular unsigned math.
+      indexAdjustment += (argIndex - origIndex - 1);
     }
   }
 

test/SILGen/isolated_default_arguments.swift

@@ -0,0 +1,123 @@
+// RUN: %target-swift-emit-silgen -Xllvm -sil-print-types -module-name test -Xllvm -sil-full-demangle -swift-version 6 %s | %FileCheck %s
+
+// We weren't adjusting offsets around isolated default arguments
+// properly, and it broke when presented with a non-isolated or
+// non-defaulted argument between two isolated default arguments.
+
+@MainActor
+func main_actor_int_x() -> Int {
+  return 0
+}
+
+@MainActor
+func main_actor_int_y() -> Int {
+  return 0
+}
+
+@MainActor
+func main_actor_int_z() -> Int {
+  return 0
+}
+
+@MainActor
+func main_actor_void() -> () {
+}
+
+@MainActor
+func main_actor_int_pair() -> (Int, Int) {
+  return (0,0)
+}
+
+// This test breaks because the intermediate argument is `nil`, which
+// we treat as non-isolated.
+@MainActor
+func nonIsolatedDefaultArg(x: Int = main_actor_int_x(),
+                           y: Int? = nil,
+                           z: Int = main_actor_int_z()) {}
+
+// CHECK-LABEL: sil hidden [ossa] @$s4test0A21NonIsolatedDefaultArgyyYaF :
+// CHECK:         [[ARG1:%.*]] = enum $Optional<Int>, #Optional.none
+// CHECK:         hop_to_executor {{%.*}} : $MainActor
+// CHECK-NEXT:    // function_ref default argument 0 of
+// CHECK-NEXT:    [[ARG0FN:%.*]] = function_ref @$s4test21nonIsolatedDefaultArg1x1y1zySi_SiSgSitFfA_
+// CHECK-NEXT:    [[ARG0:%.*]] = apply [[ARG0FN]]()
+// CHECK-NEXT:    // function_ref default argument 2 of
+// CHECK-NEXT:    [[ARG2FN:%.*]] = function_ref @$s4test21nonIsolatedDefaultArg1x1y1zySi_SiSgSitFfA1_
+// CHECK-NEXT:    [[ARG2:%.*]] = apply [[ARG2FN]]()
+// CHECK-NEXT:    // function_ref test.nonIsolatedDefaultArg
+// CHECK-NEXT:    [[FN:%.*]] = function_ref @$s4test21nonIsolatedDefaultArg1x1y1zySi_SiSgSitF :
+// CHECK:         apply [[FN]]([[ARG0]], [[ARG1]], [[ARG2]])
+func testNonIsolatedDefaultArg() async {
+  await nonIsolatedDefaultArg()
+}
+
+// This test breaks because the intermediate argument is non-defaulted
+// and so gets evaluated in the non-delayed argument pass.
+@MainActor
+func isolatedDefaultArgs(x: Int = main_actor_int_x(),
+                         y: Int = main_actor_int_y(),
+                         z: Int = main_actor_int_z()) {}
+
+// CHECK-LABEL: sil hidden [ossa] @$s4test0A13NonDefaultArgyyYaF :
+// CHECK:         [[LITERAL_FN:%.*]] = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC :
+// CHECK-NEXT:    [[ARG1:%.*]] = apply [[LITERAL_FN]](
+// CHECK:         hop_to_executor {{%.*}} : $MainActor
+// CHECK-NEXT:    // function_ref default argument 0 of
+// CHECK-NEXT:    [[ARG0FN:%.*]] = function_ref @$s4test19isolatedDefaultArgs1x1y1zySi_S2itFfA_
+// CHECK-NEXT:    [[ARG0:%.*]] = apply [[ARG0FN]]()
+// CHECK-NEXT:    // function_ref default argument 2 of
+// CHECK-NEXT:    [[ARG2FN:%.*]] = function_ref @$s4test19isolatedDefaultArgs1x1y1zySi_S2itFfA1_
+// CHECK-NEXT:    [[ARG2:%.*]] = apply [[ARG2FN]]()
+// CHECK-NEXT:    // function_ref test.isolatedDefaultArgs
+// CHECK-NEXT:    [[FN:%.*]] = function_ref @$s4test19isolatedDefaultArgs1x1y1zySi_S2itF :
+// CHECK:         apply [[FN]]([[ARG0]], [[ARG1]], [[ARG2]])
+func testNonDefaultArg() async {
+  await isolatedDefaultArgs(y: 0)
+}
+
+// Exercise our handling of isolated default arguments that expand to
+// empty or multiple arguments.
+@MainActor
+func voidIsolatedDefaultArg(x: () = main_actor_void(),
+                            y: Int = main_actor_int_y(),
+                            z: Int = main_actor_int_z()) {}
+
+// CHECK-LABEL: sil hidden [ossa] @$s4test0A22VoidIsolatedDefaultArgyyYaF :
+// CHECK:         [[LITERAL_FN:%.*]] = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC :
+// CHECK-NEXT:    [[ARG1:%.*]] = apply [[LITERAL_FN]](
+// CHECK:         hop_to_executor {{%.*}} : $MainActor
+// CHECK-NEXT:    // function_ref default argument 0 of
+// CHECK-NEXT:    [[ARG0FN:%.*]] = function_ref @$s4test22voidIsolatedDefaultArg1x1y1zyyt_S2itFfA_
+// CHECK-NEXT:    [[ARG0:%.*]] = apply [[ARG0FN]]()
+// CHECK-NEXT:    // function_ref default argument 2 of
+// CHECK-NEXT:    [[ARG2FN:%.*]] = function_ref @$s4test22voidIsolatedDefaultArg1x1y1zyyt_S2itFfA1_
+// CHECK-NEXT:    [[ARG2:%.*]] = apply [[ARG2FN]]()
+// CHECK-NEXT:    // function_ref test.voidIsolatedDefaultArg
+// CHECK-NEXT:    [[FN:%.*]] = function_ref @$s4test22voidIsolatedDefaultArg1x1y1zyyt_S2itF :
+// CHECK:         apply [[FN]]([[ARG1]], [[ARG2]])
+func testVoidIsolatedDefaultArg() async {
+  await voidIsolatedDefaultArg(y: 0)
+}
+
+@MainActor
+func tupleIsolatedDefaultArg(x: (Int,Int) = main_actor_int_pair(),
+                             y: Int = main_actor_int_y(),
+                             z: Int = main_actor_int_z()) {}
+
+// CHECK-LABEL: sil hidden [ossa] @$s4test0A23TupleIsolatedDefaultArgyyYaF :
+// CHECK:         [[LITERAL_FN:%.*]] = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC :
+// CHECK-NEXT:    [[ARG1:%.*]] = apply [[LITERAL_FN]](
+// CHECK:         hop_to_executor {{%.*}} : $MainActor
+// CHECK-NEXT:    // function_ref default argument 0 of
+// CHECK-NEXT:    [[ARG0FN:%.*]] = function_ref @$s4test23tupleIsolatedDefaultArg1x1y1zySi_Sit_S2itFfA_
+// CHECK-NEXT:    [[ARG0:%.*]] = apply [[ARG0FN]]()
+// CHECK-NEXT:    ([[ARG0_0:%.*]], [[ARG0_1:%.*]]) = destructure_tuple [[ARG0]] : $(Int, Int)
+// CHECK-NEXT:    // function_ref default argument 2 of
+// CHECK-NEXT:    [[ARG2FN:%.*]] = function_ref @$s4test23tupleIsolatedDefaultArg1x1y1zySi_Sit_S2itFfA1_
+// CHECK-NEXT:    [[ARG2:%.*]] = apply [[ARG2FN]]()
+// CHECK-NEXT:    // function_ref test.tupleIsolatedDefaultArg
+// CHECK-NEXT:    [[FN:%.*]] = function_ref @$s4test23tupleIsolatedDefaultArg1x1y1zySi_Sit_S2itF :
+// CHECK:         apply [[FN]]([[ARG0_0]], [[ARG0_1]], [[ARG1]], [[ARG2]])
+func testTupleIsolatedDefaultArg() async {
+  await tupleIsolatedDefaultArg(y: 0)
+}