Skip to content

Commit 95a7107

Browse files
hborlaHolly Borla
authored andcommitted
[Concurrency] Allow default arguments to require actor isolation.
Type checking a default argument expression will compute the required actor isolation for evaluating that argument value synchronously. Actor isolation checking is deferred to the caller; it is an error to use a default argument from across isolation domains. Currently gated behind -enable-experimental-feature IsolatedDefaultArguments.
1 parent 57f08da commit 95a7107

13 files changed

+276
-2
lines changed

include/swift/AST/Decl.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6415,6 +6415,11 @@ class ParamDecl : public VarDecl {
64156415
/// at the call site in order to have the correct context information.
64166416
Expr *getTypeCheckedDefaultExpr() const;
64176417

6418+
/// The actor isolation required of the caller in order to use the
6419+
/// default argument for this parameter. If the required isolation is
6420+
/// not met, an argument must be written explicitly at the call-site.
6421+
ActorIsolation getDefaultArgumentIsolation() const;
6422+
64186423
/// Retrieve the potentially un-type-checked default argument expression for
64196424
/// this parameter, which can be queried for information such as its source
64206425
/// range and textual representation. Returns \c nullptr if there is no

include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5236,6 +5236,10 @@ ERROR(distributed_actor_isolated_non_self_reference,none,
52365236
"distributed actor-isolated %kind0 can not be accessed from a "
52375237
"non-isolated context",
52385238
(const ValueDecl *))
5239+
ERROR(isolated_default_argument,none,
5240+
"%0 default argument cannot be synchronously evaluated from a "
5241+
"%1 context",
5242+
(ActorIsolation, ActorIsolation))
52395243
ERROR(distributed_actor_needs_explicit_distributed_import,none,
52405244
"'Distributed' module not imported, required for 'distributed actor'",
52415245
())

include/swift/AST/Expr.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4569,6 +4569,11 @@ class DefaultArgumentExpr final : public Expr {
45694569
/// expression within the context of the call site.
45704570
Expr *getCallerSideDefaultExpr() const;
45714571

4572+
/// Get the required actor isolation for evaluating this default argument
4573+
/// synchronously. If the caller does not meet the required isolation, the
4574+
/// argument must be written explicitly at the call-site.
4575+
ActorIsolation getRequiredIsolation() const;
4576+
45724577
static bool classof(const Expr *E) {
45734578
return E->getKind() == ExprKind::DefaultArgument;
45744579
}

include/swift/AST/TypeCheckRequests.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2934,6 +2934,24 @@ class DefaultArgumentTypeRequest
29342934
void cacheResult(Type type) const;
29352935
};
29362936

2937+
/// Compute the actor isolation needed to synchronously evaluate the
2938+
/// default argument for the given parameter.
2939+
class DefaultArgumentIsolation
2940+
: public SimpleRequest<DefaultArgumentIsolation,
2941+
ActorIsolation(ParamDecl *),
2942+
RequestFlags::Cached> {
2943+
public:
2944+
using SimpleRequest::SimpleRequest;
2945+
2946+
private:
2947+
friend SimpleRequest;
2948+
2949+
ActorIsolation evaluate(Evaluator &evaluator, ParamDecl *param) const;
2950+
2951+
public:
2952+
bool isCached() const { return true; }
2953+
};
2954+
29372955
/// Computes the fully type-checked caller-side default argument within the
29382956
/// context of the call site that it will be inserted into.
29392957
class CallerSideDefaultArgExprRequest

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ SWIFT_REQUEST(TypeChecker, DefaultArgumentExprRequest,
6262
Expr *(ParamDecl *), SeparatelyCached, NoLocationInfo)
6363
SWIFT_REQUEST(TypeChecker, DefaultArgumentTypeRequest,
6464
Type(ParamDecl *), SeparatelyCached, NoLocationInfo)
65+
SWIFT_REQUEST(TypeChecker, DefaultArgumentIsolation,
66+
ActorIsolation(ParamDecl *), Cached, NoLocationInfo)
6567
SWIFT_REQUEST(TypeChecker, DefaultArgumentInitContextRequest,
6668
Initializer *(ParamDecl *), SeparatelyCached, NoLocationInfo)
6769
SWIFT_REQUEST(TypeChecker, DefaultDefinitionTypeRequest,

include/swift/Basic/Features.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ EXPERIMENTAL_FEATURE(SendNonSendable, false)
222222
/// Within strict concurrency, narrow global variable isolation requirements.
223223
EXPERIMENTAL_FEATURE(GlobalConcurrency, false)
224224

225+
/// Allow default arguments to require isolation at the call-site.
226+
EXPERIMENTAL_FEATURE(IsolatedDefaultArguments, false)
227+
225228
/// Enable extended callbacks (with additional parameters) to be used when the
226229
/// "playground transform" is enabled.
227230
EXPERIMENTAL_FEATURE(PlaygroundExtendedCallbacks, true)

lib/AST/ASTPrinter.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3551,6 +3551,10 @@ static bool usesFeatureSendNonSendable(Decl *decl) {
35513551

35523552
static bool usesFeatureGlobalConcurrency(Decl *decl) { return false; }
35533553

3554+
static bool usesFeatureIsolatedDefaultArguments(Decl *decl) {
3555+
return false;
3556+
}
3557+
35543558
static bool usesFeaturePlaygroundExtendedCallbacks(Decl *decl) {
35553559
return false;
35563560
}

lib/AST/Decl.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8263,6 +8263,14 @@ Type ParamDecl::getTypeOfDefaultExpr() const {
82638263
return Type();
82648264
}
82658265

8266+
ActorIsolation ParamDecl::getDefaultArgumentIsolation() const {
8267+
auto &ctx = getASTContext();
8268+
return evaluateOrDefault(
8269+
ctx.evaluator,
8270+
DefaultArgumentIsolation{const_cast<ParamDecl *>(this)},
8271+
ActorIsolation::forNonisolated());
8272+
}
8273+
82668274
void ParamDecl::setDefaultExpr(Expr *E, bool isTypeChecked) {
82678275
if (!DefaultValueAndFlags.getPointer()) {
82688276
if (!E) return;

lib/AST/Expr.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,6 +1606,12 @@ Expr *DefaultArgumentExpr::getCallerSideDefaultExpr() const {
16061606
new (ctx) ErrorExpr(getSourceRange(), getType()));
16071607
}
16081608

1609+
ActorIsolation
1610+
DefaultArgumentExpr::getRequiredIsolation() const {
1611+
auto *param = getParamDecl();
1612+
return param->getDefaultArgumentIsolation();
1613+
}
1614+
16091615
ValueDecl *ApplyExpr::getCalledValue(bool skipFunctionConversions) const {
16101616
return ::getCalledValue(Fn, skipFunctionConversions);
16111617
}

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,6 +1939,12 @@ namespace {
19391939
llvm::function_ref<ActorIsolation(AbstractClosureExpr *)>
19401940
getClosureActorIsolation;
19411941

1942+
bool shouldRefineIsolation = false;
1943+
1944+
/// Used under the mode to compute required actor isolation for
1945+
/// an expression or function.
1946+
ActorIsolation requiredIsolation = ActorIsolation::forNonisolated();
1947+
19421948
/// Keeps track of the capture context of variables that have been
19431949
/// explicitly captured in closures.
19441950
llvm::SmallDenseMap<VarDecl *, TinyPtrVector<const DeclContext *>>
@@ -2123,6 +2129,65 @@ namespace {
21232129
}
21242130
}
21252131

2132+
bool refineRequiredIsolation(ActorIsolation refinedIsolation) {
2133+
if (!shouldRefineIsolation)
2134+
return false;
2135+
2136+
if (auto *closure = dyn_cast<AbstractClosureExpr>(getDeclContext())) {
2137+
// We cannot infer a more specific actor isolation for a @Sendable
2138+
// closure. It is an error to cast away actor isolation from a function
2139+
// type, but this is okay for non-Sendable closures because they cannot
2140+
// leave the isolation domain they're created in anyway.
2141+
if (closure->isSendable())
2142+
return false;
2143+
2144+
if (closure->getActorIsolation().isActorIsolated())
2145+
return false;
2146+
}
2147+
2148+
// To refine the required isolation, the existing requirement
2149+
// must either be 'nonisolated' or exactly the same as the
2150+
// new refinement.
2151+
if (requiredIsolation == ActorIsolation::Nonisolated ||
2152+
requiredIsolation == refinedIsolation) {
2153+
requiredIsolation = refinedIsolation;
2154+
return true;
2155+
}
2156+
2157+
return false;
2158+
}
2159+
2160+
void checkDefaultArgument(DefaultArgumentExpr *expr) {
2161+
// Check the context isolation against the required isolation for
2162+
// evaluating the default argument synchronously. If the default
2163+
// argument must be evaluated asynchronously, it must be written
2164+
// explicitly in the argument list with 'await'.
2165+
auto requiredIsolation = expr->getRequiredIsolation();
2166+
auto contextIsolation = getInnermostIsolatedContext(
2167+
getDeclContext(), getClosureActorIsolation);
2168+
2169+
if (requiredIsolation == contextIsolation)
2170+
return;
2171+
2172+
switch (requiredIsolation) {
2173+
// Nonisolated is okay from any caller isolation because
2174+
// default arguments cannot have any async calls.
2175+
case ActorIsolation::Unspecified:
2176+
case ActorIsolation::Nonisolated:
2177+
return;
2178+
2179+
case ActorIsolation::GlobalActor:
2180+
case ActorIsolation::GlobalActorUnsafe:
2181+
case ActorIsolation::ActorInstance:
2182+
break;
2183+
}
2184+
2185+
auto &ctx = getDeclContext()->getASTContext();
2186+
ctx.Diags.diagnose(
2187+
expr->getLoc(), diag::isolated_default_argument,
2188+
requiredIsolation, contextIsolation);
2189+
}
2190+
21262191
/// Check closure captures for Sendable violations.
21272192
void checkLocalCaptures(AnyFunctionRef localFunc) {
21282193
SmallVector<CapturedValue, 2> captures;
@@ -2180,6 +2245,16 @@ namespace {
21802245
contextStack.push_back(dc);
21812246
}
21822247

2248+
ActorIsolation computeRequiredIsolation(Expr *expr) {
2249+
auto &ctx = getDeclContext()->getASTContext();
2250+
2251+
shouldRefineIsolation =
2252+
ctx.LangOpts.hasFeature(Feature::IsolatedDefaultArguments);
2253+
expr->walk(*this);
2254+
shouldRefineIsolation = false;
2255+
return requiredIsolation;
2256+
}
2257+
21832258
/// Searches the applyStack from back to front for the inner-most CallExpr
21842259
/// and marks that CallExpr as implicitly async.
21852260
///
@@ -2375,6 +2450,10 @@ namespace {
23752450
checkFunctionConversion(funcConv);
23762451
}
23772452

2453+
if (auto *defaultArg = dyn_cast<DefaultArgumentExpr>(expr)) {
2454+
checkDefaultArgument(defaultArg);
2455+
}
2456+
23782457
return Action::Continue(expr);
23792458
}
23802459

@@ -2920,6 +2999,9 @@ namespace {
29202999
if (!unsatisfiedIsolation)
29213000
return false;
29223001

3002+
if (refineRequiredIsolation(*unsatisfiedIsolation))
3003+
return false;
3004+
29233005
// At this point, we know a jump is made to the callee that yields
29243006
// an isolation requirement unsatisfied by the calling context, so
29253007
// set the unsatisfiedIsolationJump fields of the ApplyExpr appropriately
@@ -3255,6 +3337,14 @@ namespace {
32553337

32563338
case AsyncMarkingResult::SyncContext:
32573339
case AsyncMarkingResult::NotFound:
3340+
// If we found an implicitly async reference in a sync
3341+
// context and we're computing the required isolation for
3342+
// an expression, the calling context requires the isolation
3343+
// of the reference.
3344+
if (refineRequiredIsolation(result.isolation)) {
3345+
return false;
3346+
}
3347+
32583348
// Complain about access outside of the isolation domain.
32593349
auto useKind = static_cast<unsigned>(
32603350
kindOfUsage(decl, context).value_or(VarRefUseEnv::Read));
@@ -3467,6 +3557,12 @@ void swift::checkInitializerActorIsolation(Initializer *init, Expr *expr) {
34673557
expr->walk(checker);
34683558
}
34693559

3560+
ActorIsolation
3561+
swift::computeRequiredIsolation(Initializer *init, Expr *expr) {
3562+
ActorIsolationChecker checker(init);
3563+
return checker.computeRequiredIsolation(expr);
3564+
}
3565+
34703566
void swift::checkEnumElementActorIsolation(
34713567
EnumElementDecl *element, Expr *expr) {
34723568
ActorIsolationChecker checker(element);

lib/Sema/TypeCheckConcurrency.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ void checkInitializerActorIsolation(Initializer *init, Expr *expr);
6060
void checkEnumElementActorIsolation(EnumElementDecl *element, Expr *expr);
6161
void checkPropertyWrapperActorIsolation(VarDecl *wrappedVar, Expr *expr);
6262

63+
/// Compute the actor isolation required in order to evaluate the given
64+
/// initializer expression synchronously.
65+
ActorIsolation computeRequiredIsolation(Initializer *init, Expr *expr);
66+
6367
/// States the reason for checking the Sendability of a given declaration.
6468
enum class SendableCheckReason {
6569
/// A reference to an actor from outside that actor.

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,8 @@ static void checkDefaultArguments(ParameterList *params) {
10691069
for (auto *param : *params) {
10701070
auto ifacety = param->getInterfaceType();
10711071
auto *expr = param->getTypeCheckedDefaultExpr();
1072+
(void)param->getDefaultArgumentIsolation();
1073+
10721074
if (!ifacety->hasPlaceholder()) {
10731075
continue;
10741076
}
@@ -1149,13 +1151,22 @@ Expr *DefaultArgumentExprRequest::evaluate(Evaluator &evaluator,
11491151
// Walk the checked initializer and contextualize any closures
11501152
// we saw there.
11511153
TypeChecker::contextualizeInitializer(dc, initExpr);
1152-
1153-
checkInitializerActorIsolation(dc, initExpr);
11541154
TypeChecker::checkInitializerEffects(dc, initExpr);
11551155

11561156
return initExpr;
11571157
}
11581158

1159+
ActorIsolation
1160+
DefaultArgumentIsolation::evaluate(Evaluator &evaluator,
1161+
ParamDecl *param) const {
1162+
if (!param->hasDefaultExpr())
1163+
return ActorIsolation::forUnspecified();
1164+
1165+
auto *dc = param->getDefaultArgumentInitContext();
1166+
auto *initExpr = param->getTypeCheckedDefaultExpr();
1167+
return computeRequiredIsolation(dc, initExpr);
1168+
}
1169+
11591170
Type DefaultArgumentTypeRequest::evaluate(Evaluator &evaluator,
11601171
ParamDecl *param) const {
11611172
if (auto *expr = param->getTypeCheckedDefaultExpr()) {

0 commit comments

Comments
 (0)