Skip to content

Commit a08b1cb

Browse files
rjmccallrjmccall
authored
Merge pull request #81370 from rjmccall/fix-iso-def-arg-idx-calc
Fix argument index calculations for isolated default arguments
2 parents 78234e2 + 93b593b commit a08b1cb

File tree

2 files changed

+168
-5
lines changed

2 files changed

+168
-5
lines changed

‎lib/SILGen/SILGenApply.cpp

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3039,13 +3039,22 @@ static void emitDelayedArguments(SILGenFunction &SGF,
30393039
// up in parallel, with empty spots being dropped into 'args'
30403040
// wherever there's a delayed argument to insert.
30413041
//
3042+
// Note that siteArgs has exactly one empty element for each
3043+
// delayed argument, even if the argument actually expands to
3044+
// zero or multiple SIL values. In such cases, we may have to
3045+
// remove or add elements to fill in the actual argument values.
3046+
//
30423047
// Note that this also begins the formal accesses in evaluation order.
30433048
for (auto argsIt = args.begin(); argsIt != args.end(); ++argsIt) {
30443049
auto &siteArgs = *argsIt;
30453050
// NB: siteArgs.size() may change during iteration
30463051
for (size_t i = 0; i < siteArgs.size(); ) {
30473052
auto &siteArg = siteArgs[i];
30483053

3054+
// Skip slots in the argument array that we've already filled in.
3055+
// Note that this takes advantage of the "exactly one element"
3056+
// assumption above, since default arguments might have () type
3057+
// and therefore expand to no actual argument slots.
30493058
if (siteArg) {
30503059
++i;
30513060
continue;
@@ -3056,6 +3065,7 @@ static void emitDelayedArguments(SILGenFunction &SGF,
30563065

30573066
if (defaultArgIsolation && delayedArg.isDefaultArg()) {
30583067
isolatedArgs.push_back(std::make_tuple(delayedNext, argsIt, i));
3068+
++i;
30593069
if (++delayedNext == delayedArgs.end()) {
30603070
goto done;
30613071
} else {
@@ -3126,14 +3136,44 @@ static void emitDelayedArguments(SILGenFunction &SGF,
31263136
SGF.emitHopToTargetExecutor(loc, executor);
31273137
}
31283138

3129-
size_t argsEmitted = 0;
3139+
// The index saved in isolatedArgs is the index where we're
3140+
// supposed to fill in the argument in siteArgs. It should point
3141+
// to a single null element, which we will replace with zero or
3142+
// more actual argument values. This replacement can invalidate
3143+
// later indices, so we need to apply an adjustment as we go.
3144+
//
3145+
// That adjustment only applies within the right siteArgs and
3146+
// must be reset when we change siteArgs. Fortunately, emission
3147+
// is always left-to-right and never revisits a siteArgs.
3148+
size_t indexAdjustment = 0;
3149+
auto indexAdjustmentSite = args.begin();
3150+
31303151
for (auto &isolatedArg : isolatedArgs) {
31313152
auto &delayedArg = *std::get<0>(isolatedArg);
3132-
auto &siteArgs = *std::get<1>(isolatedArg);
3133-
auto argIndex = std::get<2>(isolatedArg) + argsEmitted;
3134-
auto origIndex = argIndex;
3153+
auto site = std::get<1>(isolatedArg);
3154+
auto &siteArgs = *site;
3155+
size_t argIndex = std::get<2>(isolatedArg);
3156+
3157+
// Apply the current index adjustment if we haven't changed
3158+
// sites.
3159+
if (indexAdjustmentSite == site) {
3160+
argIndex += indexAdjustment;
3161+
3162+
// Otherwise, reset the current index adjustment.
3163+
} else {
3164+
indexAdjustment = 0;
3165+
}
3166+
3167+
assert(!siteArgs[argIndex] &&
3168+
"overwriting an existing arg during delayed isolated "
3169+
"argument emission");
3170+
3171+
size_t origIndex = argIndex;
31353172
delayedArg.emit(SGF, siteArgs, argIndex);
3136-
argsEmitted += (argIndex - origIndex);
3173+
3174+
// Accumulate the adjustment. Note that the adjustment can
3175+
// be negative, and we end up relying on modular unsigned math.
3176+
indexAdjustment += (argIndex - origIndex - 1);
31373177
}
31383178
}
31393179

‎test/SILGen/isolated_default_arguments.swift

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// RUN: %target-swift-emit-silgen -Xllvm -sil-print-types -module-name test -Xllvm -sil-full-demangle -swift-version 6 %s | %FileCheck %s
2+
3+
// We weren't adjusting offsets around isolated default arguments
4+
// properly, and it broke when presented with a non-isolated or
5+
// non-defaulted argument between two isolated default arguments.
6+
7+
@MainActor
8+
func main_actor_int_x() -> Int {
9+
return 0
10+
}
11+
12+
@MainActor
13+
func main_actor_int_y() -> Int {
14+
return 0
15+
}
16+
17+
@MainActor
18+
func main_actor_int_z() -> Int {
19+
return 0
20+
}
21+
22+
@MainActor
23+
func main_actor_void() -> () {
24+
}
25+
26+
@MainActor
27+
func main_actor_int_pair() -> (Int, Int) {
28+
return (0,0)
29+
}
30+
31+
// This test breaks because the intermediate argument is `nil`, which
32+
// we treat as non-isolated.
33+
@MainActor
34+
func nonIsolatedDefaultArg(x: Int = main_actor_int_x(),
35+
y: Int? = nil,
36+
z: Int = main_actor_int_z()) {}
37+
38+
// CHECK-LABEL: sil hidden [ossa] @$s4test0A21NonIsolatedDefaultArgyyYaF :
39+
// CHECK: [[ARG1:%.*]] = enum $Optional<Int>, #Optional.none
40+
// CHECK: hop_to_executor {{%.*}} : $MainActor
41+
// CHECK-NEXT: // function_ref default argument 0 of
42+
// CHECK-NEXT: [[ARG0FN:%.*]] = function_ref @$s4test21nonIsolatedDefaultArg1x1y1zySi_SiSgSitFfA_
43+
// CHECK-NEXT: [[ARG0:%.*]] = apply [[ARG0FN]]()
44+
// CHECK-NEXT: // function_ref default argument 2 of
45+
// CHECK-NEXT: [[ARG2FN:%.*]] = function_ref @$s4test21nonIsolatedDefaultArg1x1y1zySi_SiSgSitFfA1_
46+
// CHECK-NEXT: [[ARG2:%.*]] = apply [[ARG2FN]]()
47+
// CHECK-NEXT: // function_ref test.nonIsolatedDefaultArg
48+
// CHECK-NEXT: [[FN:%.*]] = function_ref @$s4test21nonIsolatedDefaultArg1x1y1zySi_SiSgSitF :
49+
// CHECK: apply [[FN]]([[ARG0]], [[ARG1]], [[ARG2]])
50+
func testNonIsolatedDefaultArg() async {
51+
await nonIsolatedDefaultArg()
52+
}
53+
54+
// This test breaks because the intermediate argument is non-defaulted
55+
// and so gets evaluated in the non-delayed argument pass.
56+
@MainActor
57+
func isolatedDefaultArgs(x: Int = main_actor_int_x(),
58+
y: Int = main_actor_int_y(),
59+
z: Int = main_actor_int_z()) {}
60+
61+
// CHECK-LABEL: sil hidden [ossa] @$s4test0A13NonDefaultArgyyYaF :
62+
// CHECK: [[LITERAL_FN:%.*]] = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC :
63+
// CHECK-NEXT: [[ARG1:%.*]] = apply [[LITERAL_FN]](
64+
// CHECK: hop_to_executor {{%.*}} : $MainActor
65+
// CHECK-NEXT: // function_ref default argument 0 of
66+
// CHECK-NEXT: [[ARG0FN:%.*]] = function_ref @$s4test19isolatedDefaultArgs1x1y1zySi_S2itFfA_
67+
// CHECK-NEXT: [[ARG0:%.*]] = apply [[ARG0FN]]()
68+
// CHECK-NEXT: // function_ref default argument 2 of
69+
// CHECK-NEXT: [[ARG2FN:%.*]] = function_ref @$s4test19isolatedDefaultArgs1x1y1zySi_S2itFfA1_
70+
// CHECK-NEXT: [[ARG2:%.*]] = apply [[ARG2FN]]()
71+
// CHECK-NEXT: // function_ref test.isolatedDefaultArgs
72+
// CHECK-NEXT: [[FN:%.*]] = function_ref @$s4test19isolatedDefaultArgs1x1y1zySi_S2itF :
73+
// CHECK: apply [[FN]]([[ARG0]], [[ARG1]], [[ARG2]])
74+
func testNonDefaultArg() async {
75+
await isolatedDefaultArgs(y: 0)
76+
}
77+
78+
// Exercise our handling of isolated default arguments that expand to
79+
// empty or multiple arguments.
80+
@MainActor
81+
func voidIsolatedDefaultArg(x: () = main_actor_void(),
82+
y: Int = main_actor_int_y(),
83+
z: Int = main_actor_int_z()) {}
84+
85+
// CHECK-LABEL: sil hidden [ossa] @$s4test0A22VoidIsolatedDefaultArgyyYaF :
86+
// CHECK: [[LITERAL_FN:%.*]] = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC :
87+
// CHECK-NEXT: [[ARG1:%.*]] = apply [[LITERAL_FN]](
88+
// CHECK: hop_to_executor {{%.*}} : $MainActor
89+
// CHECK-NEXT: // function_ref default argument 0 of
90+
// CHECK-NEXT: [[ARG0FN:%.*]] = function_ref @$s4test22voidIsolatedDefaultArg1x1y1zyyt_S2itFfA_
91+
// CHECK-NEXT: [[ARG0:%.*]] = apply [[ARG0FN]]()
92+
// CHECK-NEXT: // function_ref default argument 2 of
93+
// CHECK-NEXT: [[ARG2FN:%.*]] = function_ref @$s4test22voidIsolatedDefaultArg1x1y1zyyt_S2itFfA1_
94+
// CHECK-NEXT: [[ARG2:%.*]] = apply [[ARG2FN]]()
95+
// CHECK-NEXT: // function_ref test.voidIsolatedDefaultArg
96+
// CHECK-NEXT: [[FN:%.*]] = function_ref @$s4test22voidIsolatedDefaultArg1x1y1zyyt_S2itF :
97+
// CHECK: apply [[FN]]([[ARG1]], [[ARG2]])
98+
func testVoidIsolatedDefaultArg() async {
99+
await voidIsolatedDefaultArg(y: 0)
100+
}
101+
102+
@MainActor
103+
func tupleIsolatedDefaultArg(x: (Int,Int) = main_actor_int_pair(),
104+
y: Int = main_actor_int_y(),
105+
z: Int = main_actor_int_z()) {}
106+
107+
// CHECK-LABEL: sil hidden [ossa] @$s4test0A23TupleIsolatedDefaultArgyyYaF :
108+
// CHECK: [[LITERAL_FN:%.*]] = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC :
109+
// CHECK-NEXT: [[ARG1:%.*]] = apply [[LITERAL_FN]](
110+
// CHECK: hop_to_executor {{%.*}} : $MainActor
111+
// CHECK-NEXT: // function_ref default argument 0 of
112+
// CHECK-NEXT: [[ARG0FN:%.*]] = function_ref @$s4test23tupleIsolatedDefaultArg1x1y1zySi_Sit_S2itFfA_
113+
// CHECK-NEXT: [[ARG0:%.*]] = apply [[ARG0FN]]()
114+
// CHECK-NEXT: ([[ARG0_0:%.*]], [[ARG0_1:%.*]]) = destructure_tuple [[ARG0]] : $(Int, Int)
115+
// CHECK-NEXT: // function_ref default argument 2 of
116+
// CHECK-NEXT: [[ARG2FN:%.*]] = function_ref @$s4test23tupleIsolatedDefaultArg1x1y1zySi_Sit_S2itFfA1_
117+
// CHECK-NEXT: [[ARG2:%.*]] = apply [[ARG2FN]]()
118+
// CHECK-NEXT: // function_ref test.tupleIsolatedDefaultArg
119+
// CHECK-NEXT: [[FN:%.*]] = function_ref @$s4test23tupleIsolatedDefaultArg1x1y1zySi_Sit_S2itF :
120+
// CHECK: apply [[FN]]([[ARG0_0]], [[ARG0_1]], [[ARG1]], [[ARG2]])
121+
func testTupleIsolatedDefaultArg() async {
122+
await tupleIsolatedDefaultArg(y: 0)
123+
}

0 commit comments

Comments
 (0)