Skip to content

Commit b229082

Browse files
committed
[Concurrency] Handle nested decl contexts while computing the required isolation
for a default argument, and improve diagnostics for conflicting isolation. (cherry picked from commit 88b2332)
1 parent 1dacc88 commit b229082

File tree

3 files changed

+78
-27
lines changed

3 files changed

+78
-27
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5196,6 +5196,9 @@ ERROR(isolated_default_argument,none,
51965196
"%0 default argument cannot be synchronously evaluated from a "
51975197
"%1 context",
51985198
(ActorIsolation, ActorIsolation))
5199+
ERROR(conflicting_default_argument_isolation,none,
5200+
"default argument cannot be both %0 and %1",
5201+
(ActorIsolation, ActorIsolation))
51995202
ERROR(distributed_actor_needs_explicit_distributed_import,none,
52005203
"'Distributed' module not imported, required for 'distributed actor'",
52015204
())

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1949,11 +1949,11 @@ namespace {
19491949
llvm::function_ref<ActorIsolation(AbstractClosureExpr *)>
19501950
getClosureActorIsolation;
19511951

1952-
bool shouldRefineIsolation = false;
1952+
SourceLoc requiredIsolationLoc;
19531953

19541954
/// Used under the mode to compute required actor isolation for
19551955
/// an expression or function.
1956-
ActorIsolation requiredIsolation = ActorIsolation::forNonisolated();
1956+
llvm::SmallDenseMap<const DeclContext *, ActorIsolation> requiredIsolation;
19571957

19581958
/// Keeps track of the capture context of variables that have been
19591959
/// explicitly captured in closures.
@@ -2147,31 +2147,60 @@ namespace {
21472147
}
21482148

21492149
bool refineRequiredIsolation(ActorIsolation refinedIsolation) {
2150-
if (!shouldRefineIsolation)
2150+
if (requiredIsolationLoc.isInvalid())
21512151
return false;
21522152

2153-
if (auto *closure = dyn_cast<AbstractClosureExpr>(getDeclContext())) {
2154-
// We cannot infer a more specific actor isolation for a @Sendable
2155-
// closure. It is an error to cast away actor isolation from a function
2156-
// type, but this is okay for non-Sendable closures because they cannot
2157-
// leave the isolation domain they're created in anyway.
2158-
if (closure->isSendable())
2159-
return false;
2153+
auto infersIsolationFromContext =
2154+
[](const DeclContext *dc) -> bool {
2155+
// Isolation for declarations is based solely on explicit
2156+
// annotations; only infer isolation for initializer expressions
2157+
// and closures.
2158+
if (dc->getAsDecl())
2159+
return false;
2160+
2161+
if (auto *closure = dyn_cast<AbstractClosureExpr>(dc)) {
2162+
// We cannot infer a more specific actor isolation for a Sendable
2163+
// closure. It is an error to cast away actor isolation from a
2164+
// function type, but this is okay for non-Sendable closures
2165+
// because they cannot leave the isolation domain they're created
2166+
// in anyway.
2167+
if (closure->isSendable())
2168+
return false;
21602169

2161-
if (closure->getActorIsolation().isActorIsolated())
2170+
if (closure->getActorIsolation().isActorIsolated())
2171+
return false;
2172+
}
2173+
2174+
return true;
2175+
};
2176+
2177+
// For the call to require the given actor isolation, every DeclContext
2178+
// in the current context stack must require the same isolation. If
2179+
// along the way to the innermost context, we find a DeclContext that
2180+
// has a different isolation (e.g. it's a local function that does not
2181+
// recieve isolation from its decl context), then the expression cannot
2182+
// require a different isolation.
2183+
for (auto *dc : contextStack) {
2184+
if (!infersIsolationFromContext(dc))
21622185
return false;
2163-
}
21642186

2165-
// To refine the required isolation, the existing requirement
2166-
// must either be 'nonisolated' or exactly the same as the
2167-
// new refinement.
2168-
if (requiredIsolation == ActorIsolation::Nonisolated ||
2169-
requiredIsolation == refinedIsolation) {
2170-
requiredIsolation = refinedIsolation;
2171-
return true;
2187+
// To refine the required isolation, the existing requirement
2188+
// must either be 'nonisolated' or exactly the same as the
2189+
// new refinement.
2190+
auto isolation = requiredIsolation.find(dc);
2191+
if (isolation == requiredIsolation.end() ||
2192+
isolation->second == ActorIsolation::Nonisolated) {
2193+
requiredIsolation[dc] = refinedIsolation;
2194+
} else if (isolation->second != refinedIsolation) {
2195+
dc->getASTContext().Diags.diagnose(
2196+
requiredIsolationLoc,
2197+
diag::conflicting_default_argument_isolation,
2198+
isolation->second, refinedIsolation);
2199+
return true;
2200+
}
21722201
}
21732202

2174-
return false;
2203+
return true;
21752204
}
21762205

21772206
void checkDefaultArgument(DefaultArgumentExpr *expr) {
@@ -2265,11 +2294,12 @@ namespace {
22652294
ActorIsolation computeRequiredIsolation(Expr *expr) {
22662295
auto &ctx = getDeclContext()->getASTContext();
22672296

2268-
shouldRefineIsolation =
2269-
ctx.LangOpts.hasFeature(Feature::IsolatedDefaultArguments);
2297+
if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultArguments))
2298+
requiredIsolationLoc = expr->getLoc();
2299+
22702300
expr->walk(*this);
2271-
shouldRefineIsolation = false;
2272-
return requiredIsolation;
2301+
requiredIsolationLoc = SourceLoc();
2302+
return requiredIsolation[getDeclContext()];
22732303
}
22742304

22752305
/// Searches the applyStack from back to front for the inner-most CallExpr

test/Concurrency/isolated_default_arguments.swift

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ actor SomeGlobalActor {
1313
static let shared = SomeGlobalActor()
1414
}
1515

16+
// expected-note@+2 2 {{calls to global function 'requiresMainActor()' from outside of its actor context are implicitly asynchronous}}
1617
@MainActor
1718
func requiresMainActor() -> Int { 0 }
1819

19-
// expected-note@+2 2 {{calls to global function 'requiresSomeGlobalActor()' from outside of its actor context are implicitly asynchronous}}
2020
@SomeGlobalActor
2121
func requiresSomeGlobalActor() -> Int { 0 }
2222

@@ -77,7 +77,7 @@ func nonisolatedAsyncCaller() async {
7777
}
7878

7979
func conflictingIsolationDefaultArg(
80-
// expected-error@+1 {{call to global actor 'SomeGlobalActor'-isolated global function 'requiresSomeGlobalActor()' in a synchronous nonisolated context}}
80+
// expected-error@+1 {{default argument cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}}
8181
value: (Int, Int) = (requiresMainActor(), requiresSomeGlobalActor())
8282
) {}
8383

@@ -100,9 +100,27 @@ func closureIsolationMismatch(
100100
) {}
101101

102102
func conflictingClosureIsolation(
103+
// expected-error@+1 {{default argument cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}}
103104
closure: () -> Void = {
104105
_ = requiresMainActor()
105-
// expected-error@+1 {{call to global actor 'SomeGlobalActor'-isolated global function 'requiresSomeGlobalActor()' in a synchronous nonisolated context}}
106106
_ = requiresSomeGlobalActor()
107107
}
108108
) {}
109+
110+
func isolationInLocalFunction(
111+
closure: () -> Void = {
112+
nonisolated func local() -> Int {
113+
// expected-error@+1 {{call to main actor-isolated global function 'requiresMainActor()' in a synchronous nonisolated context}}
114+
requiresMainActor()
115+
}
116+
}
117+
) {}
118+
119+
actor A {}
120+
121+
func closureWithIsolatedParam(
122+
closure: (isolated A) -> Void = { _ in
123+
// expected-error@+1 {{call to main actor-isolated global function 'requiresMainActor()' in a synchronous actor-isolated context}}
124+
_ = requiresMainActor()
125+
}
126+
) {}

0 commit comments

Comments
 (0)