averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

Revert "[sema] Work around a double curry thunk actor isolation inference bug that is a knock on effect of ced96aa5cd653f834d2a8293ead8cf46649202cb."

Browse Source
This commit is contained in:
Pavel Yaskevich
2025-06-23 21:37:30 -07:00
committed by GitHub
parent d0b34c2b67
commit 767b93775d
5 changed files with 15 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
lib/Sema/CSApply.cpp
View File

@@ -1313,21 +1313,6 @@ namespace {
return callExpr;
}
std::optional<ActorIsolation>
getIsolationFromFunctionType(FunctionType *thunkTy) {
switch (thunkTy->getIsolation().getKind()) {
case FunctionTypeIsolation::Kind::NonIsolated:
case FunctionTypeIsolation::Kind::Parameter:
case FunctionTypeIsolation::Kind::Erased:
return {};
case FunctionTypeIsolation::Kind::GlobalActor:
return ActorIsolation::forGlobalActor(
thunkTy->getIsolation().getGlobalActorType());
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
return ActorIsolation::forCallerIsolationInheriting();
}
}
/// Build a "{ args in base.fn(args) }" single-expression curry thunk.
///
/// \param baseExpr The base expression to be captured, if warranted.
@@ -1370,8 +1355,19 @@ namespace {
// we do not visit the inner autoclosure in the ActorIsolation checker
// meaning that we do not properly call setActorIsolation on the
// AbstractClosureExpr that we produce here.
if (auto isolation = getIsolationFromFunctionType(thunkTy)) {
thunk->setActorIsolation(*isolation);
switch (thunkTy->getIsolation().getKind()) {
case FunctionTypeIsolation::Kind::NonIsolated:
case FunctionTypeIsolation::Kind::Parameter:
case FunctionTypeIsolation::Kind::Erased:
break;
case FunctionTypeIsolation::Kind::GlobalActor:
thunk->setActorIsolation(ActorIsolation::forGlobalActor(
thunkTy->getIsolation().getGlobalActorType()));
break;
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
thunk->setActorIsolation(
ActorIsolation::forCallerIsolationInheriting());
break;
}
cs.cacheType(thunk);
@@ -1540,31 +1536,6 @@ namespace {
cs.cacheType(selfOpenedRef);
}
auto outerActorIsolation = [&]() -> std::optional<ActorIsolation> {
auto resultType = outerThunkTy->getResult();
// Look through all optionals.
while (auto opt = resultType->getOptionalObjectType())
resultType = opt;
auto f =
getIsolationFromFunctionType(resultType->castTo<FunctionType>());
if (!f)
return {};
// If we have a non-async function and our "inferred" isolation is
// caller isolation inheriting, then we do not infer isolation and just
// use the default. The reason why we are doing this is:
//
// 1. nonisolated for synchronous functions is equivalent to
// nonisolated(nonsending).
//
// 2. There is a strong invariant in the compiler today that caller
// isolation inheriting is always async. By using nonisolated here, we
// avoid breaking that invariant.
if (!outerThunkTy->isAsync() && f->isCallerIsolationInheriting())
return {};
return f;
}();
Expr *outerThunkBody = nullptr;
// For an @objc optional member or a member found via dynamic lookup,
@@ -1595,11 +1566,6 @@ namespace {
auto *innerThunk = buildSingleCurryThunk(
selfOpenedRef, memberRef, cast<DeclContext>(member),
outerThunkTy->getResult()->castTo<FunctionType>(), memberLocator);
assert((!outerActorIsolation ||
innerThunk->getActorIsolation().getKind() ==
outerActorIsolation->getKind()) &&
"If we have an isolation for our double curry thunk it should "
"match our single curry thunk");
// Rewrite the body to close the existential if warranted.
if (hasOpenedExistential) {
@@ -1621,11 +1587,6 @@ namespace {
outerThunk->setThunkKind(AutoClosureExpr::Kind::DoubleCurryThunk);
outerThunk->setParameterList(
ParameterList::create(ctx, SourceLoc(), selfParamDecl, SourceLoc()));
if (outerActorIsolation) {
outerThunk->setActorIsolation(*outerActorIsolation);
}
cs.cacheType(outerThunk);
return outerThunk;

72
lib/Sema/TypeCheckConcurrency.cpp
View File

@@ -3219,65 +3219,6 @@ namespace {
return LazyInitializerWalking::InAccessor;
}
/// This function is a stripped down version of checkApply that only is
/// applied to curry thunks generated by the type checker that explicitly
/// have isolation put upon them by the typechecker to work around a bug in
/// 6.2. We do not perform any sort of actual inference... we only use it to
/// mark the apply as being isolation crossing if we have an autoclosure
/// with mismatching isolation.
///
/// We take advantage that we only can have two types of isolation on such
/// an autoclosure, global actor isolation and nonisolated(nonsending).
///
/// For more information, see the comment in buildSingleCurryThunk.
void perform62AutoclosureCurryThunkChecking(ApplyExpr *apply,
AutoClosureExpr *fn) {
// The isolation of the context that we are in.
std::optional<ActorIsolation> contextIsolation;
auto getContextIsolation = [&]() -> ActorIsolation {
if (contextIsolation)
return *contextIsolation;
auto declContext = const_cast<DeclContext *>(getDeclContext());
contextIsolation =
getInnermostIsolatedContext(declContext, getClosureActorIsolation);
return *contextIsolation;
};
std::optional<ActorIsolation> unsatisfiedIsolation;
// NOTE: Normally autoclosures did not have ActorIsolation set on it since
// we do not visit the function of the partial apply due to a bug. The
// only reason why it is set is b/c we are explicitly setting this in the
// type checker when we generate the single and double curry thunks.
auto fnTypeIsolation = fn->getActorIsolation();
if (fnTypeIsolation.isGlobalActor()) {
Type globalActor = fnTypeIsolation.getGlobalActor();
if (!(getContextIsolation().isGlobalActor() &&
getContextIsolation().getGlobalActor()->isEqual(globalActor)))
unsatisfiedIsolation = ActorIsolation::forGlobalActor(globalActor);
}
// If there was no unsatisfied actor isolation, we're done.
if (!unsatisfiedIsolation)
return;
// Record whether the callee isolation or the context isolation
// is preconcurrency, which is used later to downgrade errors to
// warnings in minimal checking.
auto calleeDecl = apply->getCalledValue(/*skipFunctionConversions=*/true);
bool preconcurrency =
getContextIsolation().preconcurrency() ||
(calleeDecl && getActorIsolation(calleeDecl).preconcurrency());
unsatisfiedIsolation =
unsatisfiedIsolation->withPreconcurrency(preconcurrency);
// At this point, we know a jump is made to the callee that yields
// an isolation requirement unsatisfied by the calling context, so
// set the unsatisfiedIsolationJump fields of the ApplyExpr appropriately
apply->setIsolationCrossing(getContextIsolation(), *unsatisfiedIsolation);
}
PreWalkResult<Pattern *> walkToPatternPre(Pattern *pattern) override {
// Walking into patterns leads to nothing good because then we
// end up visiting the AccessorDecls of a top-level
@@ -3395,19 +3336,6 @@ namespace {
partialApply->base->walk(*this);
// See if we have an autoclosure as our function. If so, check if we
// have a difference in isolation. If so, make this apply an
// isolation crossing apply.
//
// NOTE: This is just a work around for 6.2 to make checking of
// double curry thunks work correctly in the face of us not
// performing full type checking of autoclosures that are functions
// of the apply. We are doing this to make sure that we do not
// increase the surface area too much.
if (auto *fn = dyn_cast<AutoClosureExpr>(apply->getFn())) {
perform62AutoclosureCurryThunkChecking(apply, fn);
}
return Action::SkipNode(expr);
}
}

21
test/Concurrency/double_curry_thunk.swift
View File

@@ -1,21 +0,0 @@
// RUN: %target-swift-frontend -typecheck -strict-concurrency=complete -target %target-swift-5.1-abi-triple %s
// REQUIRES: concurrency
// We used to crash on this when processing double curry thunks. Make sure that
// we do not do crash in the future.
extension AsyncStream {
@Sendable func myCancel() {
}
func myNext2(_ continuation: UnsafeContinuation<Element?, Never>) {
}
func myNext() async -> Element? {
await withTaskCancellationHandler {
unsafe await withUnsafeContinuation {
unsafe myNext2($0)
}
} onCancel: { [myCancel] in
myCancel()
}
}
}

7
test/Concurrency/transfernonsendable.swift
View File

@@ -2084,14 +2084,11 @@ func inferLocationOfCapturedTaskIsolatedSelfCorrectly() {
func d() {
a.block = c // expected-warning {{converting non-Sendable function value to '@MainActor @Sendable () -> Void' may introduce data races}}
// expected-tns-warning @-1 {{non-Sendable '@MainActor () -> ()'-typed result can not be returned from main actor-isolated function to nonisolated context}}
// expected-tns-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
// expected-tns-warning @-3 {{sending 'self' risks causing data races}}
// expected-tns-note @-4 {{task-isolated 'self' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
// expected-tns-warning @-1 {{sending 'self' risks causing data races}}
// expected-tns-note @-2 {{task-isolated 'self' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
}
@MainActor
func c() {}
}
}

19
test/Concurrency/transfernonsendable_global_actor.swift
View File

@@ -338,22 +338,3 @@ func localCaptureDataRace5() {
x = 2 // expected-tns-note {{access can happen concurrently}}
}
func inferLocationOfCapturedActorIsolatedSelfCorrectly() {
class A {
var block: @MainActor () -> Void = {}
}
@CustomActor
class B {
let a = A()
func d() {
a.block = c // expected-warning {{converting non-Sendable function value to '@MainActor @Sendable () -> Void' may introduce data races}}
// expected-warning @-1 {{non-Sendable '@MainActor () -> ()'-typed result can not be returned from main actor-isolated function to global actor 'CustomActor'-isolated context}}
// expected-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
}
@MainActor
func c() {}
}
}