Skip to content

[SILGen] avoid hop before autoreleased foreign error is retained #68415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5608,10 +5608,6 @@ RValue SILGenFunction::emitApply(
// plan's finish method is called, because it must happen in the
// successors of the `await_async_continuation` terminator.
resultPlan->deferExecutorBreadcrumb(std::move(breadcrumb));

} else {
// In the ordinary case, we hop back to the current executor
breadcrumb.emit(*this, loc);
}

// Pop the argument scope.
Expand Down Expand Up @@ -5684,6 +5680,14 @@ RValue SILGenFunction::emitApply(
});
}

if (!calleeTypeInfo.foreign.async) {
// For a non-foreign-async callee, we hop back to the current executor
// _after_ popping the argument scope and collecting the results. This is
// important because we may need to, for example, retain one of the results
// prior to changing actors in the case of an autorelease'd return value.
breadcrumb.emit(*this, loc);
}

SILValue bridgedForeignError;
// If there was a foreign error convention, consider it.
// TODO: maybe this should happen after managing the result if it's
Expand Down
2 changes: 2 additions & 0 deletions test/Concurrency/isolated_default_argument_eval.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ func nonisolatedAsyncCaller() async {
// CHECK: [[GETARG:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
// CHECK: hop_to_executor {{.*}} : $MainActor
// CHECK-NEXT: apply [[GETARG]]()
// CHECK-NEXT: end_borrow {{.*}} : $MainActor
// CHECK-NEXT: destroy_value {{.*}} : $MainActor
// CHECK-NEXT: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
await mainActorDefaultArg()
}
7 changes: 7 additions & 0 deletions test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,4 +336,11 @@ MAIN_ACTOR
@property(readonly) BOOL isVisible;
@end

// rdar://114049646
MAIN_ACTOR
@protocol HotdogCompetitor
- (nullable NSString *)pileOfHotdogsToEatWithLimit:(NSObject *)limit
error:(NSError * __autoreleasing *)error;
@end

#pragma clang assume_nonnull end
6 changes: 3 additions & 3 deletions test/SILGen/async_initializer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ enum Birb {
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thick Cat.Type) -> @owned Cat
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: } // end sil function '$s12initializers7makeCatyyYaF'
func makeCat() async {
_ = await Cat(name: "Socks")
Expand All @@ -142,7 +142,7 @@ func makeCat() async {
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thin Dog.Type) -> Dog
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: } // end sil function '$s12initializers7makeDogyyYaF'
func makeDog() async {
_ = await Dog(name: "Lassie")
Expand All @@ -153,7 +153,7 @@ func makeDog() async {
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thin Birb.Type) -> Birb
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
// CHECK: } // end sil function '$s12initializers8makeBirbyyYaF'
func makeBirb() async {
_ = await Birb(name: "Chirpy")
Expand Down
13 changes: 7 additions & 6 deletions test/SILGen/hop_to_executor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ actor MyActor {
// CHECK: [[BORROWED_SELF:%[0-9]+]] = begin_borrow %0 : $MyActor
// CHECK: hop_to_executor [[BORROWED_SELF]] : $MyActor
// CHECK: = apply {{.*}} : $@convention(method) @async (Int, @guaranteed MyActor) -> ()
// CHECK-NEXT: hop_to_executor [[BORROWED_SELF]] : $MyActor
// CHECK: hop_to_executor [[BORROWED_SELF]] : $MyActor
// CHECK: } // end sil function '$s4test7MyActorC0A22ConsumingAsyncFunctionyyYaF'
__consuming func testConsumingAsyncFunction() async {
await callee(p)
Expand Down Expand Up @@ -225,8 +225,8 @@ actor BlueActorImpl {
// CHECK: [[METH:%[0-9]+]] = class_method [[REDBORROW]] : $RedActorImpl, #RedActorImpl.hello : (isolated RedActorImpl) -> (Int) -> (), $@convention(method) (Int, @guaranteed RedActorImpl) -> ()
// CHECK: hop_to_executor [[REDBORROW]] : $RedActorImpl
// CHECK-NEXT: = apply [[METH]]([[INTARG]], [[REDBORROW]]) : $@convention(method) (Int, @guaranteed RedActorImpl) -> ()
// CHECK-NEXT: end_borrow [[REDBORROW]] : $RedActorImpl
// CHECK-NEXT: hop_to_executor [[BLUE]] : $BlueActorImpl
// CHECK: end_borrow [[REDBORROW]] : $RedActorImpl
// CHECK: destroy_value [[REDMOVE]] : $RedActorImpl
// CHECK: } // end sil function '$s4test13BlueActorImplC14createAndGreetyyYaF'
func createAndGreet() async {
Expand Down Expand Up @@ -272,9 +272,9 @@ struct BlueActor {
// CHECK: hop_to_executor [[REDEXE]] : $RedActorImpl
// ---- now invoke redFn, hop back to Blue, and clean-up ----
// CHECK-NEXT: {{%[0-9]+}} = apply [[CALLEE]]([[ARG]]) : $@convention(thin) (Int) -> ()
// CHECK-NEXT: end_borrow [[REDEXE]] : $RedActorImpl
// CHECK-NEXT: destroy_value [[R]] : $RedActorImpl
// CHECK-NEXT: hop_to_executor [[BLUEEXE]] : $BlueActorImpl
// CHECK: end_borrow [[REDEXE]] : $RedActorImpl
// CHECK: destroy_value [[R]] : $RedActorImpl
// CHECK: end_borrow [[BLUEEXE]] : $BlueActorImpl
// CHECK: destroy_value [[B]] : $BlueActorImpl
// CHECK: } // end sil function '$s4test6blueFnyyYaF'
Expand All @@ -288,8 +288,9 @@ struct BlueActor {
// CHECK: [[BORROW:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $RedActorImpl
// CHECK-NEXT: hop_to_executor [[BORROW]] : $RedActorImpl
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}) : $@convention(thin) (Int) -> ()
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]]
// CHECK-NEXT: end_borrow [[BORROW]] : $RedActorImpl
// CHECK-NEXT: destroy_value
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]]
// CHECK: } // end sil function '$s4test20unspecifiedAsyncFuncyyYaF'
func unspecifiedAsyncFunc() async {
await redFn(200)
Expand All @@ -316,7 +317,7 @@ func anotherUnspecifiedAsyncFunc(_ red : RedActorImpl) async {
// CHECK: hop_to_executor [[RED:%[0-9]+]] : $RedActorImpl
// CHECK-NEXT: begin_borrow
// CHECK-NEXT: apply
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC:%[0-9]+]] : $Optional<Builtin.Executor>
// CHECK: hop_to_executor [[GENERIC_EXEC:%[0-9]+]] : $Optional<Builtin.Executor>
func testGlobalActorFuncValue(_ fn: @RedActor () -> Void) async {
await fn()
}
Expand Down
2 changes: 1 addition & 1 deletion test/SILGen/hop_to_executor_async_prop.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ func accessSweaterOfSweater(cat : Cat) async -> Sweater {

// CHECK: hop_to_executor [[GLOBAL_CAT]] : $Cat
// CHECK: [[THE_STRING:%[0-9]+]] = apply [[GETTER]]([[CAT]]) : $@convention(method) (@guaranteed Cat) -> @owned String
// CHECK: hop_to_executor [[GENERIC_EXEC]]
// CHECK: end_borrow [[GLOBAL_CAT]] : $Cat
// CHECK: destroy_value [[GLOBAL_CAT_REF]] : $Cat
// CHECK: hop_to_executor [[GENERIC_EXEC]]
// CHECK: return [[THE_STRING]] : $String
// CHECK: } // end sil function '$s4test26accessGlobalIsolatedMember3catSSAA3CatC_tYaF'
func accessGlobalIsolatedMember(cat : Cat) async -> String {
Expand Down
18 changes: 18 additions & 0 deletions test/SILGen/objc_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,21 @@ extension OptionalMemberLookups {
await self.generateMaybe!()
}
}


// CHECK-LABEL: sil {{.*}} @$s10objc_async12checkHotdogsySSSgx_So8NSObjectCtYaKSo16HotdogCompetitorRzlF
// CHECK: hop_to_executor {{.*}} : $MainActor
// CHECK: [[AUTO_REL_STR:%.*]] = apply {{.*}}<some HotdogCompetitor>({{.*}}) : $@convention(objc_method)
// CHECK: [[UNMANAGED_OPTIONAL:%.*]] = load [trivial] {{.*}} : $*@sil_unmanaged Optional<NSError>
// CHECK: [[MANAGED_OPTIONAL:%.*]] = unmanaged_to_ref [[UNMANAGED_OPTIONAL]] : $@sil_unmanaged Optional<NSError> to $Optional<NSError>
// CHECK: [[RETAINED_OPTIONAL:%.*]] = copy_value [[MANAGED_OPTIONAL]] : $Optional<NSError>
// CHECK: [[MARKED:%.*]] = mark_dependence [[RETAINED_OPTIONAL]] : $Optional<NSError> on {{.*}} : $*Optional<NSError> // user: %32
// CHECK: assign [[MARKED]] to {{.*}} : $*Optional<NSError>
// CHECK: destroy_value {{.*}} : $MainActor
// CHECK: dealloc_stack {{.*}} : $*AutoreleasingUnsafeMutablePointer<Optional<NSError>>
// CHECK: dealloc_stack {{.*}} : $*@sil_unmanaged Optional<NSError>
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
// CHECK: switch_enum
func checkHotdogs(_ v: some HotdogCompetitor, _ timeLimit: NSObject) async throws -> String? {
return try await v.pileOfHotdogsToEat(withLimit: timeLimit)
}
4 changes: 2 additions & 2 deletions test/SILGen/toplevel_globalactorvars.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ await printFromMyActor(value: a)
// CHECK: [[ACTORREF:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MyActorImpl
// CHECK: hop_to_executor [[ACTORREF]] : $MyActorImpl
// CHECK: {{%[0-9]+}} = apply [[PRINTFROMMYACTOR_FUNC]]([[AGLOBAL]])
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
// CHECK: end_borrow [[ACTORREF]]
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]

if a < 10 {
// CHECK: [[AACCESS:%[0-9]+]] = begin_access [read] [dynamic] [[AREF]] : $*Int
Expand Down Expand Up @@ -120,6 +120,6 @@ if a < 10 {
// CHECK: [[ACTORREF:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MyActorImpl
// CHECK: hop_to_executor [[ACTORREF]] : $MyActorImpl
// CHECK: {{%[0-9]+}} = apply [[PRINTFROMMYACTOR_FUNC]]([[AGLOBAL]])
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
// CHECK: end_borrow [[ACTORREF]]
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
}