Skip to content

Commit ac2f57e

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 8b3a36d commit ac2f57e

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"
@@ -737,6 +738,14 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
737738
}
738739
}
739740

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

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2097,6 +2097,62 @@ void swift::replaceUnsafeInheritExecutorWithDefaultedIsolationParam(
20972097
diag.fixItInsert(insertionLoc, newParameterText);
20982098
}
20992099

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

3848-
bool inConcurrencyModule = getDeclContext()->getParentModule()->getName()
3849-
.str().equals("_Concurrency");
3850-
3904+
bool inConcurrencyModule = ::inConcurrencyModule(getDeclContext());
38513905
auto diag = ctx.Diags.diagnose(diagLoc,
38523906
diag::isolation_in_inherits_executor,
38533907
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)