Skip to content

[Concurrency] Fix a false-positive metatype capture diagnostic. #81860

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.

Sign up for GitHub

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 2 commits into from
May 30, 2025
Merged

[Concurrency] Fix a false-positive metatype capture diagnostic. #81860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c283ce1
[Concurrency] Remove some dead code from `mayExecuteConcurrentlyWith`.
hborla May 30, 2025
f0e77c7
[Concurrency] Don't diagnose metatype captures in closures that are
hborla May 30, 2025
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
34 changes: 10 additions & 24 deletions lib/Sema/TypeCheckConcurrency.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2543,7 +2543,8 @@ namespace {
/// Determine whether code in the given use context might execute
/// concurrently with code in the definition context.
bool mayExecuteConcurrentlyWith(
const DeclContext *useContext, const DeclContext *defContext);
const DeclContext *useContext, const DeclContext *defContext,
bool includeSending = false);

/// If the subexpression is a reference to a mutable local variable from a
/// different context, record its parent. We'll query this as part of
Expand Down Expand Up @@ -3051,12 +3052,9 @@ namespace {
}
}

// FIXME: When passing to a sending parameter, should this be handled
// by region isolation? Or should it always be handled by region
// isolation?
if (mayExecuteConcurrentlyWith(
localFunc.getAsDeclContext(), getDeclContext()) ||
(explicitClosure && explicitClosure->isPassedToSendingParameter())) {
localFunc.getAsDeclContext(), getDeclContext(),
/*includeSending*/true)) {
auto innermostGenericDC = localFunc.getAsDeclContext();
while (innermostGenericDC && !innermostGenericDC->isGenericContext())
innermostGenericDC = innermostGenericDC->getParent();
Expand Down Expand Up @@ -4816,13 +4814,12 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
}

bool ActorIsolationChecker::mayExecuteConcurrentlyWith(
const DeclContext *useContext, const DeclContext *defContext) {
const DeclContext *useContext, const DeclContext *defContext,
bool includeSending) {
// Fast path for when the use and definition contexts are the same.
if (useContext == defContext)
return false;

bool isolatedStateMayEscape = false;

auto useIsolation = getActorIsolationOfContext(
const_cast<DeclContext *>(useContext), getClosureActorIsolation);
if (useIsolation.isActorIsolated()) {
Expand All @@ -4840,16 +4837,6 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith(
if (ctx.LangOpts.hasFeature(Feature::GlobalActorIsolatedTypesUsability) &&
regionIsolationEnabled && useIsolation.isGlobalActor())
return false;

// If the local function is not Sendable, its isolation differs
// from that of the context, and both contexts are actor isolated,
// then capturing non-Sendable values allows the closure to stash
// those values into actor-isolated state. The original context
// may also stash those values into isolated state, enabling concurrent
// access later on.
isolatedStateMayEscape =
(!regionIsolationEnabled &&
useIsolation.isActorIsolated() && defIsolation.isActorIsolated());
}

// Walk the context chain from the use to the definition.
Expand All @@ -4859,18 +4846,17 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith(
if (closure->isSendable())
return true;

if (isolatedStateMayEscape)
return true;
if (auto *explicitClosure = dyn_cast<ClosureExpr>(closure)) {
if (includeSending && explicitClosure->isPassedToSendingParameter())
return true;
}
}

if (auto func = dyn_cast<FuncDecl>(useContext)) {
if (func->isLocalCapture()) {
// If the function is @Sendable... it can be run concurrently.
if (func->isSendable())
return true;

if (isolatedStateMayEscape)
return true;
}
}

Expand Down
13 changes: 13 additions & 0 deletions test/Concurrency/sendable_metatype_typecheck.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,16 @@ extension TestUnapplied {
func testUnappliedWithOpetator<T: Comparable>(v: TestUnapplied<T>) {
v<=>(>) // expected-error {{converting non-Sendable function value to '@Sendable (T, T) -> Bool' may introduce data races}}
}

protocol P {}

func acceptClosure(_: () -> Void) {}

@MainActor
func f<T: P>(_: T.Type) {
acceptClosure {
Task {
_ = T.self // okay to capture T.Type in this closure.
}
}
}