Skip to content

[5.10][SE-0411] Promote IsolatedDefaultValues from an experimental feature to an upcoming feature. #70839

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 10 commits into from
Jan 11, 2024
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
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,30 @@
> **Note**\
> This is in reverse chronological order, so newer entries are added to the top.

## Swift 5.10

* [SE-0411][]:

Default value expressions can now have the same isolation as the enclosing
function or the corresponding stored property:

```swift
@MainActor
func requiresMainActor() -> Int { ... }

class C {
@MainActor
var x: Int = requiresMainActor()
}

@MainActor func defaultArg(value: Int = requiresMainActor()) { ... }
```

For isolated default values of stored properties, the implicit initialization
only happens in the body of an `init` with the same isolation. This closes
an important data-race safety hole where global-actor-isolated default values
could inadvertently run synchronously from outside the actor.

## Swift 5.9.2

* [SE-0407][]:
Expand Down Expand Up @@ -9846,6 +9870,7 @@ using the `.dynamicType` member to retrieve the type of an expression should mig
[SE-0394]: https://github.com/apple/swift-evolution/blob/main/proposals/0394-swiftpm-expression-macros.md
[SE-0397]: https://github.com/apple/swift-evolution/blob/main/proposals/0397-freestanding-declaration-macros.md
[SE-0407]: https://github.com/apple/swift-evolution/blob/main/proposals/0407-member-macro-conformances.md
[SE-0411]: https://github.com/apple/swift-evolution/blob/main/proposals/0411-isolated-default-values.md
[#64927]: <https://github.com/apple/swift/issues/64927>
[#42697]: <https://github.com/apple/swift/issues/42697>
[#42728]: <https://github.com/apple/swift/issues/42728>
Expand Down
11 changes: 8 additions & 3 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5200,9 +5200,14 @@ 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(conflicting_stored_property_isolation,none,
"%select{default|memberwise}0 initializer for %1 cannot be both %2 and %3",
(bool, Type, ActorIsolation, ActorIsolation))
NOTE(property_requires_actor,none,
"initializer for %0 %1 is %2",
(DescriptiveDeclKind, Identifier, ActorIsolation))
ERROR(isolated_default_argument_context,none,
"%0 default value 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 @@ -4513,6 +4513,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 @@ -4547,6 +4551,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
4 changes: 1 addition & 3 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ UPCOMING_FEATURE(BareSlashRegexLiterals, 354, 6)
UPCOMING_FEATURE(DeprecateApplicationMain, 383, 6)
UPCOMING_FEATURE(ImportObjcForwardDeclarations, 384, 6)
UPCOMING_FEATURE(DisableOutwardActorInference, 401, 6)
UPCOMING_FEATURE(IsolatedDefaultValues, 411, 6)
UPCOMING_FEATURE(GlobalConcurrency, 412, 6)

UPCOMING_FEATURE(ExistentialAny, 335, 7)
Expand Down Expand Up @@ -222,9 +223,6 @@ EXPERIMENTAL_FEATURE(StrictConcurrency, true)
/// Defer Sendable checking to SIL diagnostic phase.
EXPERIMENTAL_FEATURE(SendNonSendable, false)

/// Allow default values to require isolation at the call-site.
EXPERIMENTAL_FEATURE(IsolatedDefaultValues, false)

/// Enable extended callbacks (with additional parameters) to be used when the
/// "playground transform" is enabled.
EXPERIMENTAL_FEATURE(PlaygroundExtendedCallbacks, true)
Expand Down
21 changes: 11 additions & 10 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10289,6 +10289,7 @@ ActorIsolation swift::getActorIsolationOfContext(
DeclContext *dc,
llvm::function_ref<ActorIsolation(AbstractClosureExpr *)>
getClosureActorIsolation) {
auto &ctx = dc->getASTContext();
auto dcToUse = dc;
// Defer bodies share actor isolation of their enclosing context.
if (auto FD = dyn_cast<FuncDecl>(dcToUse)) {
Expand All @@ -10300,21 +10301,21 @@ 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(/*unsafe=*/false);
}
// If IsolatedDefaultValues are enabled, treat this context as having
// unspecified isolation. We'll compute the required isolation for
// the initializer and validate that it matches the isolation of the
// var itself in the DefaultInitializerIsolation request.
if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues))
return ActorIsolation::forUnspecified();

return getActorIsolation(var);
}
Expand Down
5 changes: 3 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -976,8 +976,9 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
Opts.StrictConcurrencyLevel = StrictConcurrency::Minimal;
}

// StrictConcurrency enables all data-race safety upcoming features.
if (Opts.hasFeature(Feature::StrictConcurrency)) {
// StrictConcurrency::Complete enables all data-race safety features.
if (Opts.StrictConcurrencyLevel == StrictConcurrency::Complete) {
Opts.Features.insert(Feature::IsolatedDefaultValues);
Opts.Features.insert(Feature::GlobalConcurrency);
}

Expand Down
118 changes: 110 additions & 8 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 @@ -2690,6 +2695,28 @@ class DelayedArgument {
return LV().Loc;
}

bool isDefaultArg() const {
return Kind == DefaultArgument;
}

SILLocation getDefaultArgLoc() const {
assert(isDefaultArg());
auto storage = Value.get<DefaultArgumentStorage>(Kind);
return storage.loc;
}

llvm::Optional<ActorIsolation> getIsolation() const {
if (!isDefaultArg())
return llvm::None;

auto storage = Value.get<DefaultArgumentStorage>(Kind);
if (!storage.implicitlyAsync)
return llvm::None;

auto callee = storage.defaultArgsOwner.getDecl();
return getActorIsolation(callee);
}

void emit(SILGenFunction &SGF, SmallVectorImpl<ManagedValue> &args,
size_t &argIndex) {
switch (Kind) {
Expand Down Expand Up @@ -2915,6 +2942,31 @@ static void emitDelayedArguments(SILGenFunction &SGF,
MutableArrayRef<SmallVector<ManagedValue, 4>> args) {
assert(!delayedArgs.empty());

// If any of the delayed arguments are isolated default arguments,
// argument evaluation happens in the following order:
//
// 1. Left-to-right evalution of explicit r-value arguments
// 2. Left-to-right evaluation of formal access arguments
// 3. Hop to the callee's isolation domain
// 4. Left-to-right evaluation of default arguments

// So, if any delayed arguments are isolated, all default arguments
// are collected during the first pass over the delayed arguments,
// and emitted separately after a hop to the callee's isolation domain.

llvm::Optional<ActorIsolation> defaultArgIsolation;
for (auto &arg : delayedArgs) {
if (auto isolation = arg.getIsolation()) {
defaultArgIsolation = isolation;
break;
}
}

SmallVector<std::tuple<
/*delayedArgIt*/decltype(delayedArgs)::iterator,
/*siteArgsIt*/decltype(args)::iterator,
/*index*/size_t>, 2> isolatedArgs;

SmallVector<std::pair<SILValue, SILLocation>, 4> emittedInoutArgs;
auto delayedNext = delayedArgs.begin();

Expand All @@ -2923,7 +2975,8 @@ static void emitDelayedArguments(SILGenFunction &SGF,
// wherever there's a delayed argument to insert.
//
// Note that this also begins the formal accesses in evaluation order.
for (auto &siteArgs : args) {
for (auto argsIt = args.begin(); argsIt != args.end(); ++argsIt) {
auto &siteArgs = *argsIt;
// NB: siteArgs.size() may change during iteration
for (size_t i = 0; i < siteArgs.size(); ) {
auto &siteArg = siteArgs[i];
Expand All @@ -2936,6 +2989,15 @@ static void emitDelayedArguments(SILGenFunction &SGF,
assert(delayedNext != delayedArgs.end());
auto &delayedArg = *delayedNext;

if (defaultArgIsolation && delayedArg.isDefaultArg()) {
isolatedArgs.push_back(std::make_tuple(delayedNext, argsIt, i));
if (++delayedNext == delayedArgs.end()) {
goto done;
} else {
continue;
}
}

// Emit the delayed argument and replace it in the arguments array.
delayedArg.emit(SGF, siteArgs, i);

Expand All @@ -2956,6 +3018,45 @@ static void emitDelayedArguments(SILGenFunction &SGF,

done:

if (defaultArgIsolation) {
assert(SGF.F.isAsync());
assert(!isolatedArgs.empty());

auto &firstArg = *std::get<0>(isolatedArgs[0]);
auto loc = firstArg.getDefaultArgLoc();

SILValue executor;
switch (*defaultArgIsolation) {
case ActorIsolation::GlobalActor:
case ActorIsolation::GlobalActorUnsafe:
executor = SGF.emitLoadGlobalActorExecutor(
defaultArgIsolation->getGlobalActor());
break;

case ActorIsolation::ActorInstance:
llvm_unreachable("default arg cannot be actor instance isolated");

case ActorIsolation::Unspecified:
case ActorIsolation::Nonisolated:
case ActorIsolation::NonisolatedUnsafe:
llvm_unreachable("Not isolated");
}

// Hop to the target isolation domain once to evaluate all
// default arguments.
SGF.emitHopToTargetExecutor(loc, executor);

size_t argsEmitted = 0;
for (auto &isolatedArg : isolatedArgs) {
auto &delayedArg = *std::get<0>(isolatedArg);
auto &siteArgs = *std::get<1>(isolatedArg);
auto argIndex = std::get<2>(isolatedArg) + argsEmitted;
auto origIndex = argIndex;
delayedArg.emit(SGF, siteArgs, argIndex);
argsEmitted += (argIndex - origIndex);
}
}

// Check to see if we have multiple inout arguments which obviously
// alias. Note that we could do this in a later SILDiagnostics pass
// as well: this would be stronger (more equivalences exposed) but
Expand Down Expand Up @@ -3261,7 +3362,7 @@ class ArgEmitter {
defArg->getParamIndex(),
substParamType, origParamType,
claimNextParameters(numParams),
Rep);
Rep, defArg->isImplicitlyAsync());
Args.push_back(ManagedValue());

maybeEmitForeignArgument();
Expand Down Expand Up @@ -4255,7 +4356,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
1 change: 1 addition & 0 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2533,6 +2533,7 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc,
ConcreteDeclRef defaultArgsOwner,
unsigned destIndex,
CanType resultType,
bool implicitlyAsync,
SGFContext C) {
SILDeclRef generator
= SILDeclRef::getDefaultArgGenerator(defaultArgsOwner.getDecl(),
Expand Down
1 change: 1 addition & 0 deletions lib/SILGen/SILGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -1905,6 +1905,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
ConcreteDeclRef defaultArgsOwner,
unsigned destIndex,
CanType resultType,
bool implicitlyAsync,
SGFContext C = SGFContext());

RValue emitApplyOfStoredPropertyInitializer(
Expand Down
Loading