From 27fd038b5ecbbadf73883345137c451ec2bc56a5 Mon Sep 17 00:00:00 2001 From: Konrad 'ktoso' Malawski Date: Fri, 4 Jul 2025 10:20:08 +0900 Subject: [PATCH 1/4] [Concurrency] Fix nonisolated(nonsending) interaction with #isolation --- lib/SILGen/SILGenExpr.cpp | 29 ++++++-- lib/Sema/TypeCheckConcurrency.cpp | 7 +- ...macro_in_nonisolated_nonsending_func.swift | 72 +++++++++++++++++++ 3 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 56c9c2ec0cdee..7200d362287c1 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -7287,15 +7287,30 @@ RValue RValueEmitter::visitMacroExpansionExpr(MacroExpansionExpr *E, RValue RValueEmitter::visitCurrentContextIsolationExpr( CurrentContextIsolationExpr *E, SGFContext C) { - // If we are in an actor initializer that is isolated to self, the - // current isolation is flow-sensitive; use that instead of the - // synthesized expression. - if (auto ctor = dyn_cast_or_null( - SGF.F.getDeclRef().getDecl())) { - auto isolation = getActorIsolation(ctor); - if (ctor->isDesignatedInit() && + auto afd = + dyn_cast_or_null(SGF.F.getDeclRef().getDecl()); + if (afd) { + auto isolation = getActorIsolation(afd); + if (isolation == ActorIsolation::CallerIsolationInheriting) { + auto *isolatedArg = SGF.F.maybeGetIsolatedArgument(); + assert(isolatedArg && + "Caller Isolation Inheriting without isolated parameter"); + ManagedValue isolatedMV; + if (isolatedArg->getOwnershipKind() == OwnershipKind::Guaranteed) { + isolatedMV = ManagedValue::forBorrowedRValue(isolatedArg); + } else { + isolatedMV = ManagedValue::forUnmanagedOwnedValue(isolatedArg); + } + return RValue(SGF, E, isolatedMV); + } + + auto ctor = dyn_cast_or_null(afd); + if (ctor && ctor->isDesignatedInit() && isolation == ActorIsolation::ActorInstance && isolation.getActorInstance() == ctor->getImplicitSelfDecl()) { + // If we are in an actor initializer that is isolated to self, the + // current isolation is flow-sensitive; use that instead of the + // synthesized expression. auto isolationValue = SGF.emitFlowSensitiveSelfIsolation(E, isolation); return RValue(SGF, E, isolationValue); diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 22ec5ab814e78..ef0ca26bbce2a 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -4393,10 +4393,15 @@ namespace { case ActorIsolation::Unspecified: case ActorIsolation::Nonisolated: - case ActorIsolation::CallerIsolationInheriting: case ActorIsolation::NonisolatedUnsafe: actorExpr = new (ctx) NilLiteralExpr(loc, /*implicit=*/false); break; + case ActorIsolation::CallerIsolationInheriting: + // For caller isolation this expression will be replaced in SILGen + // because we're adding an implicit isolated parameter that #isolated + // must resolve to, but cannot do so during AST expansion quite yet. + actorExpr = new (ctx) NilLiteralExpr(loc, /*implicit=*/false); + break; } diff --git a/test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift b/test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift new file mode 100644 index 0000000000000..0365c6df828dd --- /dev/null +++ b/test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift @@ -0,0 +1,72 @@ +// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s --dump-input=always + +// REQUIRES: concurrency +// REQUIRES: executable_test +// REQUIRES: concurrency_runtime + +// rdar://78109470 +// UNSUPPORTED: back_deployment_runtime +// UNSUPPORTED: freestanding + +import _Concurrency +import StdlibUnittest + +@main struct Main { + static func main() async { + await explicitIsolatedParam() + // CHECK: go - explicitIsolatedParam + // CHECK: outerIsolation = #isolation = Optional(Swift.MainActor) + // CHECK: Task{} #isolation = nil + // CHECK: Task{ [outerIsolation] } outerIsolation = Optional(Swift.MainActor), #iso = Optional(Swift.MainActor) + // CHECK: done - explicitIsolatedParam + + print() + await nonisolatedNonsending() + // CHECK: go - nonisolatedNonsending + // CHECK: outerIsolation = #isolation = Optional(Swift.MainActor) + // CHECK: Task{} #isolation = nil + // CHECK: Task{ [outerIsolation] } outerIsolation = Optional(Swift.MainActor), #iso = nil + // CHECK: done - nonisolatedNonsending + } +} + +func explicitIsolatedParam(isolation: isolated (any Actor)? = #isolation) async { + print("go - \(#function)") + MainActor.assertIsolated() + + let outerIsolation = #isolation + print("outerIsolation = #isolation = \(String(describing: outerIsolation))") + + await Task { + let iso = #isolation + print("Task{} #isolation = \(String(describing: iso))") + }.value + + await Task { + let iso = #isolation + print("Task{ [outerIsolation] } outerIsolation = \(String(describing: isolation)), #iso = \(String(describing: iso))") + }.value + + print("done - \(#function)") +} + + +nonisolated(nonsending) func nonisolatedNonsending() async { + print("go - \(#function)") + MainActor.assertIsolated() + + let outerIsolation = #isolation + print("outerIsolation = #isolation = \(String(describing: outerIsolation))") // WRONG; this is nil today + + await Task { + let iso = #isolation + print("Task{} #isolation = \(String(describing: iso))") + }.value + + await Task { + let iso = #isolation + print("Task{ [outerIsolation] } outerIsolation = \(String(describing: outerIsolation)), #iso = \(String(describing: iso))") + }.value + + print("done - \(#function)") +} From 07ccd73c5098c02015b6d5fb945a1cd08296c203 Mon Sep 17 00:00:00 2001 From: Konrad 'ktoso' Malawski Date: Mon, 7 Jul 2025 11:34:51 +0900 Subject: [PATCH 2/4] [Concurrency] #isolation+nonsending nonisolated - review followup and SIL test --- lib/SILGen/SILGenExpr.cpp | 24 +++++++++--------- ...lated_nonsending_isolation_macro_sil.swift | 25 +++++++++++++++++++ 2 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 test/Concurrency/isolated_nonsending_isolation_macro_sil.swift diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 7200d362287c1..f33cc526bd2d8 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -7291,6 +7291,18 @@ RValue RValueEmitter::visitCurrentContextIsolationExpr( dyn_cast_or_null(SGF.F.getDeclRef().getDecl()); if (afd) { auto isolation = getActorIsolation(afd); + auto ctor = dyn_cast_or_null(afd); + if (ctor && ctor->isDesignatedInit() && + isolation == ActorIsolation::ActorInstance && + isolation.getActorInstance() == ctor->getImplicitSelfDecl()) { + // If we are in an actor initializer that is isolated to self, the + // current isolation is flow-sensitive; use that instead of the + // synthesized expression. + auto isolationValue = + SGF.emitFlowSensitiveSelfIsolation(E, isolation); + return RValue(SGF, E, isolationValue); + } + if (isolation == ActorIsolation::CallerIsolationInheriting) { auto *isolatedArg = SGF.F.maybeGetIsolatedArgument(); assert(isolatedArg && @@ -7303,18 +7315,6 @@ RValue RValueEmitter::visitCurrentContextIsolationExpr( } return RValue(SGF, E, isolatedMV); } - - auto ctor = dyn_cast_or_null(afd); - if (ctor && ctor->isDesignatedInit() && - isolation == ActorIsolation::ActorInstance && - isolation.getActorInstance() == ctor->getImplicitSelfDecl()) { - // If we are in an actor initializer that is isolated to self, the - // current isolation is flow-sensitive; use that instead of the - // synthesized expression. - auto isolationValue = - SGF.emitFlowSensitiveSelfIsolation(E, isolation); - return RValue(SGF, E, isolationValue); - } } return visit(E->getActor(), C); diff --git a/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift b/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift new file mode 100644 index 0000000000000..718706d5dc8e1 --- /dev/null +++ b/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift @@ -0,0 +1,25 @@ +// RUN: %target-swift-frontend -parse-as-library -emit-sil %s | %FileCheck %s + +// REQUIRES: concurrency + +nonisolated(nonsending) func nonisolatedNonsending() async { + let iso = #isolation + take(iso: iso) +} + +func take(iso: (any Actor)?) {} + +// CHECK-LABEL: // nonisolatedNonsending() +// CHECK-NEXT: // Isolation: caller_isolation_inheriting +// CHECK-NEXT: sil hidden @$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional) -> () { +// CHECK: bb0(%0 : $Optional): +// CHECK-NEXT: hop_to_executor %0 // id: %1 +// CHECK-NEXT: retain_value %0 // id: %2 +// CHECK-NEXT: debug_value %0, let, name "iso" // id: %3 +// CHECK-NEXT: // function_ref take(iso:) +// CHECK-NEXT: %4 = function_ref @$s39isolated_nonsending_isolation_macro_sil4take3isoyScA_pSg_tF : $@convention(thin) (@guaranteed Optional) -> () // user: %5 +// CHECK-NEXT: %5 = apply %4(%0) : $@convention(thin) (@guaranteed Optional) -> () +// CHECK-NEXT: release_value %0 // id: %6 +// CHECK-NEXT: %7 = tuple () // user: %8 +// CHECK-NEXT: return %7 // id: %8 +// CHECK-NEXT: } // end sil function '$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF' \ No newline at end of file From 71f1be9a4cca20d05ea73503ce9390dd98b9175d Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 8 Jul 2025 06:25:08 +0900 Subject: [PATCH 3/4] Remove unnecessary comment in test --- .../Runtime/isolated_macro_in_nonisolated_nonsending_func.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift b/test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift index 0365c6df828dd..2e27408655b37 100644 --- a/test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift +++ b/test/Concurrency/Runtime/isolated_macro_in_nonisolated_nonsending_func.swift @@ -56,7 +56,7 @@ nonisolated(nonsending) func nonisolatedNonsending() async { MainActor.assertIsolated() let outerIsolation = #isolation - print("outerIsolation = #isolation = \(String(describing: outerIsolation))") // WRONG; this is nil today + print("outerIsolation = #isolation = \(String(describing: outerIsolation))") await Task { let iso = #isolation From 1d04e113498293bb3666cb7e0ccf4b1c8b49cad0 Mon Sep 17 00:00:00 2001 From: Konrad 'ktoso' Malawski Date: Tue, 8 Jul 2025 10:03:41 +0900 Subject: [PATCH 4/4] [Concurrency] Use ASSERT for to also assert in release builds Small cleanup from code review; we'd crash either way after this line, so might be better to always ASSERT more nicely. --- lib/SILGen/SILGenConcurrency.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/SILGen/SILGenConcurrency.cpp b/lib/SILGen/SILGenConcurrency.cpp index ca0f92fa3f823..f303895fff701 100644 --- a/lib/SILGen/SILGenConcurrency.cpp +++ b/lib/SILGen/SILGenConcurrency.cpp @@ -75,7 +75,7 @@ setExpectedExecutorForParameterIsolation(SILGenFunction &SGF, // argument. if (actorIsolation.getKind() == ActorIsolation::CallerIsolationInheriting) { auto *isolatedArg = SGF.F.maybeGetIsolatedArgument(); - assert(isolatedArg && + ASSERT(isolatedArg && "Caller Isolation Inheriting without isolated parameter"); ManagedValue isolatedMV; if (isolatedArg->getOwnershipKind() == OwnershipKind::Guaranteed) {