Skip to content

Commit 1776ce4

Browse files
committed
[region-isolation] Make sure that we can handle an async let without any captures correctly.
Previously, we assumed we would always have a partial_apply. In the case where we do not capture any values this is not true since we instead have a thin_to_thick_function.
1 parent 3c30bb1 commit 1776ce4

File tree

3 files changed

+68
-6
lines changed

3 files changed

+68
-6
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -417,13 +417,19 @@ static bool isProjectedFromAggregate(SILValue value) {
417417
return visitor.isMerge;
418418
}
419419

420-
static PartialApplyInst *findAsyncLetPartialApplyFromStart(BuiltinInst *bi) {
420+
namespace {
421+
using AsyncLetSourceValue =
422+
llvm::PointerUnion<PartialApplyInst *, ThinToThickFunctionInst *>;
423+
} // namespace
424+
425+
static std::optional<AsyncLetSourceValue>
426+
findAsyncLetPartialApplyFromStart(BuiltinInst *bi) {
421427
// If our operand is Sendable then we want to return nullptr. We only want to
422428
// return a value if we are not
423429
SILValue value = bi->getOperand(1);
424430
auto fType = value->getType().castTo<SILFunctionType>();
425431
if (fType->isSendable())
426-
return nullptr;
432+
return {};
427433

428434
SILValue temp = value;
429435
while (true) {
@@ -436,10 +442,15 @@ static PartialApplyInst *findAsyncLetPartialApplyFromStart(BuiltinInst *bi) {
436442
value = temp;
437443
}
438444

439-
return cast<PartialApplyInst>(value);
445+
// We can also get a thin_to_thick_function here if we do not capture
446+
// anything. In such a case, we just do not process the partial apply get
447+
if (auto *pai = dyn_cast<PartialApplyInst>(value))
448+
return {{pai}};
449+
return {{cast<ThinToThickFunctionInst>(value)}};
440450
}
441451

442-
static PartialApplyInst *findAsyncLetPartialApplyFromGet(ApplyInst *ai) {
452+
static std::optional<AsyncLetSourceValue>
453+
findAsyncLetPartialApplyFromGet(ApplyInst *ai) {
443454
auto *bi = cast<BuiltinInst>(FullApplySite(ai).getArgument(0));
444455
assert(*bi->getBuiltinKind() ==
445456
BuiltinValueKind::StartAsyncLetWithLocalBuffer);
@@ -1552,11 +1563,18 @@ class PartitionOpTranslator {
15521563
}
15531564

15541565
void translateAsyncLetGet(ApplyInst *ai) {
1555-
auto *pai = findAsyncLetPartialApplyFromGet(ai);
1566+
auto source = findAsyncLetPartialApplyFromGet(ai);
1567+
assert(source.has_value());
1568+
1569+
// If we didn't find a partial_apply, then we must have had a
1570+
// thin_to_thick_function meaning we did not capture anything.
1571+
if (source->is<ThinToThickFunctionInst *>())
1572+
return;
15561573

15571574
// We should always be able to derive a partial_apply since we pattern
15581575
// matched against the actual function call to swift_asyncLet_get in our
15591576
// caller.
1577+
auto *pai = source->get<PartialApplyInst *>();
15601578
assert(pai && "AsyncLet Get should always have a derivable partial_apply");
15611579

15621580
ApplySite applySite(pai);

test/Concurrency/transfernonsendable.sil

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ case none
7474
case some(T)
7575
}
7676

77+
sil @swift_asyncLet_get : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
78+
sil @swift_asyncLet_finish : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
79+
7780
/////////////////
7881
// MARK: Tests //
7982
/////////////////
@@ -144,4 +147,31 @@ bb0:
144147

145148
%9999 = tuple ()
146149
return %9999 : $()
150+
}
151+
152+
sil @implicitClosure : $@convention(thin) @Sendable @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
153+
154+
sil [ossa] @asyncLetWithThinToThickFunction : $@convention(thin) @async () -> () {
155+
bb0:
156+
%0 = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
157+
hop_to_executor %0 : $Optional<Builtin.Executor>
158+
%2 = alloc_stack $NonSendableKlass
159+
%3 = address_to_pointer %2 : $*NonSendableKlass to $Builtin.RawPointer
160+
%4 = enum $Optional<Builtin.RawPointer>, #Optional.none!enumelt
161+
%5 = function_ref @implicitClosure : $@convention(thin) @Sendable @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
162+
%6 = convert_function %5 : $@convention(thin) @Sendable @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass> to $@convention(thin) @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
163+
%7 = thin_to_thick_function %6 : $@convention(thin) @async @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass> to $@noescape @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>
164+
%8 = builtin "startAsyncLetWithLocalBuffer"<NonSendableKlass>(%4 : $Optional<Builtin.RawPointer>, %7 : $@noescape @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <NonSendableKlass>, %3 : $Builtin.RawPointer) : $Builtin.RawPointer
165+
%9 = function_ref @swift_asyncLet_get : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
166+
%10 = apply %9(%8, %3) : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
167+
hop_to_executor %0 : $Optional<Builtin.Executor>
168+
%12 = load [copy] %2 : $*NonSendableKlass
169+
destroy_value %12 : $NonSendableKlass
170+
%14 = function_ref @swift_asyncLet_finish : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
171+
%15 = apply %14(%8, %3) : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
172+
hop_to_executor %0 : $Optional<Builtin.Executor>
173+
%17 = builtin "endAsyncLetLifetime"(%8 : $Builtin.RawPointer) : $()
174+
dealloc_stack %2 : $*NonSendableKlass
175+
%19 = tuple ()
176+
return %19 : $()
147177
}

test/Concurrency/transfernonsendable_asynclet.swift

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
////////////////////////
1010

1111
/// Classes are always non-sendable, so this is non-sendable
12-
class NonSendableKlass { // expected-complete-note 92{{}}
12+
class NonSendableKlass { // expected-complete-note 94{{}}
13+
// expected-tns-note @-1 {{}}
1314
var field: NonSendableKlass? = nil
1415
var field2: NonSendableKlass? = nil
1516

@@ -38,6 +39,7 @@ func useInOut<T>(_ x: inout T) {}
3839
func useValue<T>(_ x: T) -> T { x }
3940
func useValueWrapInOptional<T>(_ x: T) -> T? { x }
4041

42+
@MainActor func returnValueFromMain<T>() async -> T { fatalError() }
4143
@MainActor func transferToMain<T>(_ t: T) async {}
4244
@MainActor func transferToMainInt<T>(_ t: T) async -> Int { 5 }
4345
@MainActor func transferToMainIntOpt<T>(_ t: T) async -> Int? { 5 }
@@ -735,3 +737,15 @@ func asyncLet_Let_NormalUse_Simple2() async {
735737
let _ = await y
736738
useValue(x)
737739
}
740+
741+
func asyncLetWithoutCapture() async {
742+
// Make sure that we setup y correctly as fresh.
743+
//
744+
// NOTE: Error below will go away in next commit.
745+
async let x: NonSendableKlass = await returnValueFromMain()
746+
// expected-warning @-1 {{non-sendable type 'NonSendableKlass' returned by implicitly asynchronous call to main actor-isolated function cannot cross actor boundary}}
747+
let y = await x
748+
await transferToMain(y) // expected-tns-warning {{transferring value of non-Sendable type 'NonSendableKlass' from nonisolated context to main actor-isolated context}}
749+
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
750+
useValue(y) // expected-tns-note {{access here could race}}
751+
}

0 commit comments

Comments
 (0)