Skip to content

[Concurrency] Allow isolated default arguments to be used from across isolation boundaries. #69391

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.

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
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5263,9 +5263,8 @@ ERROR(distributed_actor_isolated_non_self_reference,none,
"distributed actor-isolated %kind0 can not be accessed from a "
"non-isolated context",
(const ValueDecl *))
ERROR(isolated_default_argument,none,
"%0 default argument cannot be synchronously evaluated from a "
"%1 context",
ERROR(isolated_default_argument_context,none,
"%0 default argument in a %1 context",
(ActorIsolation, ActorIsolation))
ERROR(conflicting_default_argument_isolation,none,
"default argument cannot be both %0 and %1",
Expand Down
14 changes: 14 additions & 0 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -4540,6 +4540,10 @@ class DefaultArgumentExpr final : public Expr {
/// default expression.
PointerUnion<DeclContext *, Expr *> ContextOrCallerSideExpr;

/// Whether this default argument is evaluated asynchronously because
/// it's isolated to the callee's isolation domain.
bool implicitlyAsync = false;

public:
explicit DefaultArgumentExpr(ConcreteDeclRef defaultArgsOwner,
unsigned paramIndex, SourceLoc loc, Type Ty,
Expand Down Expand Up @@ -4574,6 +4578,16 @@ class DefaultArgumentExpr final : public Expr {
/// argument must be written explicitly at the call-site.
ActorIsolation getRequiredIsolation() const;

/// Whether this default argument is evaluated asynchronously because
/// it's isolated to the callee's isolation domain.
bool isImplicitlyAsync() const {
return implicitlyAsync;
}

void setImplicitlyAsync() {
implicitlyAsync = true;
}

static bool classof(const Expr *E) {
return E->getKind() == ExprKind::DefaultArgument;
}
Expand Down
15 changes: 4 additions & 11 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10536,22 +10536,15 @@ ActorIsolation swift::getActorIsolationOfContext(
return getActorIsolation(vd);

// In the context of the initializing or default-value expression of a
// stored property, the isolation varies between instance and type members:
// stored property:
// - For a static stored property, the isolation matches the VarDecl.
// Static properties are initialized upon first use, so the isolation
// of the initializer must match the isolation required to access the
// property.
// - For a field of a nominal type, the expression can require a specific
// actor isolation. That default expression may only be used from inits
// that meet the required isolation.
// - For a field of a nominal type, the expression can require the same
// actor isolation as the field itself. That default expression may only
// be used from inits that meet the required isolation.
if (auto *var = dcToUse->getNonLocalVarDecl()) {
auto &ctx = dc->getASTContext();
if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues) &&
var->isInstanceMember() &&
!var->getAttrs().hasAttribute<LazyAttr>()) {
return ActorIsolation::forNonisolated();
}

return getActorIsolation(var);
}

Expand Down
20 changes: 13 additions & 7 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2518,18 +2518,21 @@ class DelayedArgument {
AbstractionPattern origResultType;
ClaimedParamsRef paramsToEmit;
SILFunctionTypeRepresentation functionRepresentation;

bool implicitlyAsync;

DefaultArgumentStorage(SILLocation loc,
ConcreteDeclRef defaultArgsOwner,
unsigned destIndex,
CanType resultType,
AbstractionPattern origResultType,
ClaimedParamsRef paramsToEmit,
SILFunctionTypeRepresentation functionRepresentation)
SILFunctionTypeRepresentation functionRepresentation,
bool implicitlyAsync)
: loc(loc), defaultArgsOwner(defaultArgsOwner), destIndex(destIndex),
resultType(resultType), origResultType(origResultType),
paramsToEmit(paramsToEmit),
functionRepresentation(functionRepresentation)
functionRepresentation(functionRepresentation),
implicitlyAsync(implicitlyAsync)
{}
};
struct BorrowedLValueStorage {
Expand Down Expand Up @@ -2656,13 +2659,15 @@ class DelayedArgument {
CanType resultType,
AbstractionPattern origResultType,
ClaimedParamsRef params,
SILFunctionTypeRepresentation functionTypeRepresentation)
SILFunctionTypeRepresentation functionTypeRepresentation,
bool implicitlyAsync)
: Kind(DefaultArgument) {
Value.emplace<DefaultArgumentStorage>(Kind, loc, defaultArgsOwner,
destIndex,
resultType,
origResultType, params,
functionTypeRepresentation);
functionTypeRepresentation,
implicitlyAsync);
}

DelayedArgument(DelayedArgument &&other)
Expand Down Expand Up @@ -3261,7 +3266,7 @@ class ArgEmitter {
defArg->getParamIndex(),
substParamType, origParamType,
claimNextParameters(numParams),
Rep);
Rep, defArg->isImplicitlyAsync());
Args.push_back(ManagedValue());

maybeEmitForeignArgument();
Expand Down Expand Up @@ -4255,7 +4260,8 @@ void DelayedArgument::emitDefaultArgument(SILGenFunction &SGF,
auto value = SGF.emitApplyOfDefaultArgGenerator(info.loc,
info.defaultArgsOwner,
info.destIndex,
info.resultType);
info.resultType,
info.implicitlyAsync);

SmallVector<ManagedValue, 4> loweredArgs;
SmallVector<DelayedArgument, 4> delayedArgs;
Expand Down
19 changes: 18 additions & 1 deletion lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2548,6 +2548,7 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc,
ConcreteDeclRef defaultArgsOwner,
unsigned destIndex,
CanType resultType,
bool implicitlyAsync,
SGFContext C) {
SILDeclRef generator
= SILDeclRef::getDefaultArgGenerator(defaultArgsOwner.getDecl(),
Expand Down Expand Up @@ -2578,8 +2579,24 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc,
emitCaptures(loc, generator, CaptureEmission::ImmediateApplication,
captures);

// The default argument might require the callee's isolation. If so,
// make sure to emit an actor hop.
//
// FIXME: Instead of hopping back and forth for each individual isolated
// default argument, we should emit one hop for all default arguments if
// any of them are isolated, and immediately enter the function after.
llvm::Optional<ActorIsolation> implicitActorHopTarget = llvm::None;
if (implicitlyAsync) {
auto *param = getParameterAt(defaultArgsOwner.getDecl(), destIndex);
auto isolation = param->getInitializerIsolation();
if (isolation.isActorIsolated()) {
implicitActorHopTarget = isolation;
}
}

return emitApply(std::move(resultPtr), std::move(argScope), loc, fnRef, subs,
captures, calleeTypeInfo, ApplyOptions(), C, llvm::None);
captures, calleeTypeInfo, ApplyOptions(), C,
implicitActorHopTarget);
}

RValue SILGenFunction::emitApplyOfStoredPropertyInitializer(
Expand Down
1 change: 1 addition & 0 deletions lib/SILGen/SILGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -1907,6 +1907,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
ConcreteDeclRef defaultArgsOwner,
unsigned destIndex,
CanType resultType,
bool implicitlyAsync,
SGFContext C = SGFContext());

RValue emitApplyOfStoredPropertyInitializer(
Expand Down
9 changes: 8 additions & 1 deletion lib/Sema/CodeSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,14 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,

if (ICK == ImplicitConstructorKind::Memberwise) {
ctor->setIsMemberwiseInitializer();
addNonIsolatedToSynthesized(decl, ctor);

// FIXME: If 'IsolatedDefaultValues' is enabled, the memberwise init
// should be 'nonisolated' if none of the memberwise-initialized properties
// are global actor isolated and have non-Sendable type, and none of the
// initial values require global actor isolation.
if (!ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues)) {
addNonIsolatedToSynthesized(decl, ctor);
}
}

// If we are defining a default initializer for a class that has a superclass,
Expand Down
26 changes: 18 additions & 8 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1921,7 +1921,17 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
// Determine the type of the argument, ignoring any implicit
// conversions that could have stripped sendability.
if (Expr *argExpr = arg.getExpr()) {
argType = argExpr->findOriginalType();
argType = argExpr->findOriginalType();

// If this is a default argument expression, don't check Sendability
// if the argument is evaluated in the callee's isolation domain.
if (auto *defaultExpr = dyn_cast<DefaultArgumentExpr>(argExpr)) {
auto argIsolation = defaultExpr->getRequiredIsolation();
auto calleeIsolation = isolationCrossing->getCalleeIsolation();
if (argIsolation == calleeIsolation) {
continue;
}
}
}
}

Expand Down Expand Up @@ -2180,8 +2190,10 @@ namespace {
// recieve isolation from its decl context), then the expression cannot
// require a different isolation.
for (auto *dc : contextStack) {
if (!infersIsolationFromContext(dc))
if (!infersIsolationFromContext(dc)) {
requiredIsolation.clear();
return false;
}

// To refine the required isolation, the existing requirement
// must either be 'nonisolated' or exactly the same as the
Expand All @@ -2195,6 +2207,7 @@ namespace {
requiredIsolationLoc,
diag::conflicting_default_argument_isolation,
isolation->second, refinedIsolation);
requiredIsolation.clear();
return true;
}
}
Expand All @@ -2205,8 +2218,8 @@ namespace {
void checkDefaultArgument(DefaultArgumentExpr *expr) {
// Check the context isolation against the required isolation for
// evaluating the default argument synchronously. If the default
// argument must be evaluated asynchronously, it must be written
// explicitly in the argument list with 'await'.
// argument must be evaluated asynchronously, record that in the
// expression node.
auto requiredIsolation = expr->getRequiredIsolation();
auto contextIsolation = getInnermostIsolatedContext(
getDeclContext(), getClosureActorIsolation);
Expand All @@ -2227,10 +2240,7 @@ namespace {
break;
}

auto &ctx = getDeclContext()->getASTContext();
ctx.Diags.diagnose(
expr->getLoc(), diag::isolated_default_argument,
requiredIsolation, contextIsolation);
expr->setImplicitlyAsync();
}

/// Check closure captures for Sendable violations.
Expand Down
13 changes: 12 additions & 1 deletion lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,18 @@ static void checkDefaultArguments(ParameterList *params) {
for (auto *param : *params) {
auto ifacety = param->getInterfaceType();
auto *expr = param->getTypeCheckedDefaultExpr();
(void)param->getInitializerIsolation();

// If the default argument has isolation, it must match the
// isolation of the decl context.
auto defaultArgIsolation = param->getInitializerIsolation();
if (defaultArgIsolation.isActorIsolated()) {
auto *dc = param->getDeclContext();
auto enclosingIsolation = getActorIsolationOfContext(dc);
if (enclosingIsolation != defaultArgIsolation) {
param->diagnose(diag::isolated_default_argument_context,
defaultArgIsolation, enclosingIsolation);
}
}

if (!ifacety->hasPlaceholder()) {
continue;
Expand Down
25 changes: 25 additions & 0 deletions test/Concurrency/isolated_default_argument_eval.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RUN: %target-swift-emit-silgen -I %t -disable-availability-checking -strict-concurrency=complete -enable-experimental-feature IsolatedDefaultValues -parse-as-library %s | %FileCheck %s

// REQUIRES: concurrency
// REQUIRES: asserts

@MainActor
func requiresMainActor() -> Int { 0 }

@MainActor
func mainActorDefaultArg(value: Int = requiresMainActor()) {}

// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval15mainActorCalleryyF
@MainActor func mainActorCaller() {
mainActorDefaultArg()
}

// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval22nonisolatedAsyncCalleryyYaF
func nonisolatedAsyncCaller() async {
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
// CHECK: [[GETARG:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
// CHECK: hop_to_executor {{.*}} : $MainActor
// CHECK-NEXT: apply [[GETARG]]()
// CHECK-NEXT: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
await mainActorDefaultArg()
}
Loading