Skip to content

Commit a2b2324

Browse files
committed
Use @_unsafeInheritExecutor forms of with*Continuation from @_unsafeInheritExecutor functions
The move from `@_unsafeInheritExecutor` to `#isolation` for the with*Continuation breaks code that is using `@_unsafeInheritExecutor` and calling these APIs. This originally caused silent breakage (which manifest as runtime crashes), and is now detected by the compiler as an error. However, despite `@_unsafeInheritExecutor` being an unsafe, not-intended-to-be-user-facing feature, it is indeed being used, along with these APIs. Introduce _unsafeInheritExecutor_-prefixed versions of the `with*Continuation` and `withTaskCancellationHandler` APIs into the _Concurrency library that use `@_unsafeInheritExecutor`. Then, teach the type checker to swap in these _unsafeInheritExecutor_-prefixed versions in lieu of the originals when they are called from an `@_unsafeInheritExecutor` function. This allows existing code using `@_unsafeInheritExecutor` with these APIs to continue working as it has before, albeit with a warning that `@_unsafeInheritExecutor` has been removed. Fixes rdar://131151376.
1 parent 45d4d97 commit a2b2324

File tree

8 files changed

+149
-9
lines changed

8 files changed

+149
-9
lines changed

lib/Sema/PreCheckExpr.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
//===----------------------------------------------------------------------===//
1717

1818
#include "TypeChecker.h"
19+
#include "TypeCheckConcurrency.h"
1920
#include "TypeCheckType.h"
2021
#include "TypoCorrection.h"
2122
#include "swift/AST/ASTVisitor.h"
@@ -738,6 +739,14 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
738739
}
739740
}
740741

742+
// If we are in an @_unsafeInheritExecutor context, swap out
743+
// declarations for their _unsafeInheritExecutor_ counterparts if they
744+
// exist.
745+
if (enclosingUnsafeInheritsExecutor(DC)) {
746+
introduceUnsafeInheritExecutorReplacements(
747+
DC, UDRE->getNameLoc().getBaseNameLoc(), ResultValues);
748+
}
749+
741750
return buildRefExpr(ResultValues, DC, UDRE->getNameLoc(),
742751
UDRE->isImplicit(), UDRE->getFunctionRefKind());
743752
}

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,6 +2093,62 @@ void swift::replaceUnsafeInheritExecutorWithDefaultedIsolationParam(
20932093
diag.fixItInsert(insertionLoc, newParameterText);
20942094
}
20952095

2096+
/// Whether this declaration context is in the _Concurrency module.
2097+
static bool inConcurrencyModule(const DeclContext *dc) {
2098+
return dc->getParentModule()->getName().str().equals("_Concurrency");
2099+
}
2100+
2101+
void swift::introduceUnsafeInheritExecutorReplacements(
2102+
const DeclContext *dc, SourceLoc loc, SmallVectorImpl<ValueDecl *> &decls) {
2103+
if (decls.empty())
2104+
return;
2105+
2106+
auto isReplaceable = [&](ValueDecl *decl) {
2107+
return isa<FuncDecl>(decl) && inConcurrencyModule(decl->getDeclContext()) &&
2108+
decl->getDeclContext()->isModuleScopeContext();
2109+
};
2110+
2111+
// Make sure at least some of the entries are functions in the _Concurrency
2112+
// module.
2113+
ModuleDecl *concurrencyModule = nullptr;
2114+
for (auto decl: decls) {
2115+
if (isReplaceable(decl)) {
2116+
concurrencyModule = decl->getDeclContext()->getParentModule();
2117+
break;
2118+
}
2119+
}
2120+
if (!concurrencyModule)
2121+
return;
2122+
2123+
// Dig out the name.
2124+
auto baseName = decls.front()->getName().getBaseName();
2125+
if (baseName.isSpecial())
2126+
return;
2127+
2128+
// Look for entities with the _unsafeInheritExecutor_ prefix on the name.
2129+
ASTContext &ctx = decls.front()->getASTContext();
2130+
Identifier newIdentifier = ctx.getIdentifier(
2131+
("_unsafeInheritExecutor_" + baseName.getIdentifier().str()).str());
2132+
2133+
NameLookupOptions lookupOptions = defaultUnqualifiedLookupOptions;
2134+
LookupResult lookup = TypeChecker::lookupUnqualified(
2135+
const_cast<DeclContext *>(dc), DeclNameRef(newIdentifier), loc,
2136+
lookupOptions);
2137+
if (!lookup)
2138+
return;
2139+
2140+
// Drop all of the _Concurrency entries in favor of the ones found by this
2141+
// lookup.
2142+
decls.erase(std::remove_if(decls.begin(), decls.end(), [&](ValueDecl *decl) {
2143+
return isReplaceable(decl);
2144+
}),
2145+
decls.end());
2146+
for (const auto &lookupResult: lookup) {
2147+
if (auto decl = lookupResult.getValueDecl())
2148+
decls.push_back(decl);
2149+
}
2150+
}
2151+
20962152
/// Check if it is safe for the \c globalActor qualifier to be removed from
20972153
/// \c ty, when the function value of that type is isolated to that actor.
20982154
///
@@ -3847,9 +3903,7 @@ namespace {
38473903
std::tie(diagLoc, inDefaultArgument) = adjustPoundIsolationDiagLoc(
38483904
isolationExpr, getDeclContext()->getParentModule());
38493905

3850-
bool inConcurrencyModule = getDeclContext()->getParentModule()->getName()
3851-
.str().equals("_Concurrency");
3852-
3906+
bool inConcurrencyModule = ::inConcurrencyModule(getDeclContext());
38533907
auto diag = ctx.Diags.diagnose(diagLoc,
38543908
diag::isolation_in_inherits_executor,
38553909
inDefaultArgument);

lib/Sema/TypeCheckConcurrency.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,18 @@ AbstractFunctionDecl *enclosingUnsafeInheritsExecutor(const DeclContext *dc);
656656
void replaceUnsafeInheritExecutorWithDefaultedIsolationParam(
657657
AbstractFunctionDecl *func, InFlightDiagnostic &diag);
658658

659+
/// Replace any functions in this list that were found in the _Concurrency
660+
/// module and have _unsafeInheritExecutor_-prefixed versions with those
661+
/// _unsafeInheritExecutor_-prefixed versions.
662+
///
663+
/// This function is an egregious hack that allows us to introduce the
664+
/// #isolation-based versions of functions into the concurrency library
665+
/// without breaking clients that use @_unsafeInheritExecutor. Since those
666+
/// clients can't use #isolation (it doesn't work with @_unsafeInheritExecutor),
667+
/// we route them to the @_unsafeInheritExecutor versions implicitly.
668+
void introduceUnsafeInheritExecutorReplacements(
669+
const DeclContext *dc, SourceLoc loc, SmallVectorImpl<ValueDecl *> &decls);
670+
659671
} // end namespace swift
660672

661673
namespace llvm {

stdlib/public/Concurrency/CheckedContinuation.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,10 @@ public func withCheckedContinuation<T>(
300300
}
301301

302302
@available(SwiftStdlib 5.1, *)
303-
@usableFromInline
304303
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
305304
@_unavailableInEmbedded
306305
@_silgen_name("$ss23withCheckedContinuation8function_xSS_yScCyxs5NeverOGXEtYalF")
307-
internal func __abi_withCheckedContinuation<T>(
306+
public func _unsafeInheritExecutor_withCheckedContinuation<T>(
308307
function: String = #function,
309308
_ body: (CheckedContinuation<T, Never>) -> Void
310309
) async -> T {
@@ -361,11 +360,10 @@ public func withCheckedThrowingContinuation<T>(
361360
}
362361

363362
@available(SwiftStdlib 5.1, *)
364-
@usableFromInline
365363
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
366364
@_unavailableInEmbedded
367365
@_silgen_name("$ss31withCheckedThrowingContinuation8function_xSS_yScCyxs5Error_pGXEtYaKlF")
368-
internal func __abi_withCheckedThrowingContinuation<T>(
366+
public func _unsafeInheritExecutor_withCheckedThrowingContinuation<T>(
369367
function: String = #function,
370368
_ body: (CheckedContinuation<T, Error>) -> Void
371369
) async throws -> T {

stdlib/public/Concurrency/PartialAsyncTask.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,32 @@ public func withUnsafeThrowingContinuation<T>(
730730
}
731731
}
732732

733+
// HACK: a version of withUnsafeContinuation that uses the unsafe
734+
// @_unsafeInheritExecutor.
735+
@available(SwiftStdlib 5.1, *)
736+
@_alwaysEmitIntoClient
737+
@_unsafeInheritExecutor
738+
public func _unsafeInheritExecutor_withUnsafeContinuation<T>(
739+
_ fn: (UnsafeContinuation<T, Never>) -> Void
740+
) async -> T {
741+
return await Builtin.withUnsafeContinuation {
742+
fn(UnsafeContinuation<T, Never>($0))
743+
}
744+
}
745+
746+
// HACK: a version of withUnsafeThrowingContinuation that uses the unsafe
747+
// @_unsafeInheritExecutor.
748+
@available(SwiftStdlib 5.1, *)
749+
@_alwaysEmitIntoClient
750+
@_unsafeInheritExecutor
751+
public func _unsafeInheritExecutor_withUnsafeThrowingContinuation<T>(
752+
_ fn: (UnsafeContinuation<T, Error>) -> Void
753+
) async throws -> sending T {
754+
return try await Builtin.withUnsafeThrowingContinuation {
755+
fn(UnsafeContinuation<T, Error>($0))
756+
}
757+
}
758+
733759
/// A hack to mark an SDK that supports swift_continuation_await.
734760
@available(SwiftStdlib 5.1, *)
735761
@_alwaysEmitIntoClient

stdlib/public/Concurrency/SourceCompatibilityShims.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,17 @@ public func withTaskCancellationHandler<T>(
6262
try await withTaskCancellationHandler(operation: operation, onCancel: handler)
6363
}
6464

65+
@available(SwiftStdlib 5.1, *)
66+
@_alwaysEmitIntoClient
67+
@_unsafeInheritExecutor
68+
@available(*, deprecated, renamed: "withTaskCancellationHandler(operation:onCancel:)")
69+
public func _unsafeInheritExecutor_withTaskCancellationHandler<T>(
70+
handler: @Sendable () -> Void,
71+
operation: () async throws -> T
72+
) async rethrows -> T {
73+
try await withTaskCancellationHandler(operation: operation, onCancel: handler)
74+
}
75+
6576
@available(SwiftStdlib 5.1, *)
6677
extension Task where Success == Never, Failure == Never {
6778
@available(*, deprecated, message: "`Task.withCancellationHandler` has been replaced by `withTaskCancellationHandler` and will be removed shortly.")

stdlib/public/Concurrency/TaskCancellation.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,8 @@ public func withTaskCancellationHandler<T>(
8484

8585
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
8686
@available(SwiftStdlib 5.1, *)
87-
@usableFromInline
8887
@_silgen_name("$ss27withTaskCancellationHandler9operation8onCancelxxyYaKXE_yyYbXEtYaKlF")
89-
internal func __abi_withTaskCancellationHandler<T>(
88+
public func _unsafeInheritExecutor_withTaskCancellationHandler<T>(
9089
operation: () async throws -> T,
9190
onCancel handler: @Sendable () -> Void
9291
) async rethrows -> T {

test/Concurrency/unsafe_inherit_executor.swift

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,34 @@ func unsafeCallerAvoidsNewLoop(x: some AsyncSequence<Int, Never>) async throws {
7878

7979
for try await _ in x { }
8080
}
81+
82+
// -------------------------------------------------------------------------
83+
// Type checker hack to use _unsafeInheritExecutor_-prefixed versions of
84+
// some concurrency library functions.
85+
// -------------------------------------------------------------------------
86+
87+
@_unsafeInheritExecutor
88+
func unsafeCallerAvoidsNewLoop() async throws {
89+
// expected-warning@-1{{@_unsafeInheritExecutor attribute is deprecated; consider an 'isolated' parameter defaulted to '#isolation' instead}}
90+
91+
_ = await withUnsafeContinuation { (continuation: UnsafeContinuation<Int, Never>) in
92+
continuation.resume(returning: 5)
93+
}
94+
95+
_ = try await withUnsafeThrowingContinuation { (continuation: UnsafeContinuation<Int, any Error>) in
96+
continuation.resume(returning: 5)
97+
}
98+
99+
_ = await withCheckedContinuation { (continuation: CheckedContinuation<Int, Never>) in
100+
continuation.resume(returning: 5)
101+
}
102+
103+
_ = try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Int, any Error>) in
104+
continuation.resume(returning: 5)
105+
}
106+
107+
_ = await withTaskCancellationHandler {
108+
5
109+
} onCancel: {
110+
}
111+
}

0 commit comments

Comments
 (0)