From 49d6399a88d44f8e034231ed04a362bdd114a994 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Tue, 24 Oct 2023 22:16:00 -0700 Subject: [PATCH 1/2] [Concurrency] Allow isolated default arguments to be used from across isolation boundaries. --- include/swift/AST/DiagnosticsSema.def | 5 +- include/swift/AST/Expr.h | 14 +++++ lib/AST/Decl.cpp | 15 ++--- lib/SILGen/SILGenApply.cpp | 20 ++++--- lib/SILGen/SILGenExpr.cpp | 19 +++++- lib/SILGen/SILGenFunction.h | 1 + lib/Sema/CodeSynthesis.cpp | 9 ++- lib/Sema/TypeCheckConcurrency.cpp | 26 +++++--- lib/Sema/TypeCheckDeclPrimary.cpp | 13 +++- .../isolated_default_arguments.swift | 59 ++++++++++--------- ...solated_default_arguments_serialized.swift | 1 - 11 files changed, 122 insertions(+), 60 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 09600c32237c5..ec500dcd6c022 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -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", diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index de03ed1d8a9da..669a42e911281 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -4540,6 +4540,10 @@ class DefaultArgumentExpr final : public Expr { /// default expression. PointerUnion 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, @@ -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; } diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 45bd19636fdd2..86696f76f63a8 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -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()) { - return ActorIsolation::forNonisolated(); - } - return getActorIsolation(var); } diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 011ec191995d0..06b38b8feef00 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -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 { @@ -2656,13 +2659,15 @@ class DelayedArgument { CanType resultType, AbstractionPattern origResultType, ClaimedParamsRef params, - SILFunctionTypeRepresentation functionTypeRepresentation) + SILFunctionTypeRepresentation functionTypeRepresentation, + bool implicitlyAsync) : Kind(DefaultArgument) { Value.emplace(Kind, loc, defaultArgsOwner, destIndex, resultType, origResultType, params, - functionTypeRepresentation); + functionTypeRepresentation, + implicitlyAsync); } DelayedArgument(DelayedArgument &&other) @@ -3261,7 +3266,7 @@ class ArgEmitter { defArg->getParamIndex(), substParamType, origParamType, claimNextParameters(numParams), - Rep); + Rep, defArg->isImplicitlyAsync()); Args.push_back(ManagedValue()); maybeEmitForeignArgument(); @@ -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 loweredArgs; SmallVector delayedArgs; diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 373b9009518c2..580dac67f7c42 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -2548,6 +2548,7 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc, ConcreteDeclRef defaultArgsOwner, unsigned destIndex, CanType resultType, + bool implicitlyAsync, SGFContext C) { SILDeclRef generator = SILDeclRef::getDefaultArgGenerator(defaultArgsOwner.getDecl(), @@ -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 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( diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index c4353dc9a2125..eaf7cb761689b 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -1907,6 +1907,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction ConcreteDeclRef defaultArgsOwner, unsigned destIndex, CanType resultType, + bool implicitlyAsync, SGFContext C = SGFContext()); RValue emitApplyOfStoredPropertyInitializer( diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 683e1f8fc2cb1..bcf9410e29ea2 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -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, diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index f3de70f03b3cb..e3b992217b1a0 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -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(argExpr)) { + auto argIsolation = defaultExpr->getRequiredIsolation(); + auto calleeIsolation = isolationCrossing->getCalleeIsolation(); + if (argIsolation == calleeIsolation) { + continue; + } + } } } @@ -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 @@ -2195,6 +2207,7 @@ namespace { requiredIsolationLoc, diag::conflicting_default_argument_isolation, isolation->second, refinedIsolation); + requiredIsolation.clear(); return true; } } @@ -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); @@ -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. diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index a8023a676e150..b93e0b000ecff 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -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; diff --git a/test/Concurrency/isolated_default_arguments.swift b/test/Concurrency/isolated_default_arguments.swift index 8ae99b295a706..d863f9505b4ca 100644 --- a/test/Concurrency/isolated_default_arguments.swift +++ b/test/Concurrency/isolated_default_arguments.swift @@ -20,16 +20,28 @@ func requiresMainActor() -> Int { 0 } @SomeGlobalActor func requiresSomeGlobalActor() -> Int { 0 } +// expected-error@+1 {{main actor-isolated default argument in a nonisolated context}} +func mainActorDefaultArgInvalid(value: Int = requiresMainActor()) {} + +func mainActorClosureInvalid( + closure: () -> Int = { // expected-error {{main actor-isolated default argument in a nonisolated context}} + requiresMainActor() + } +) {} + +// expected-note@+2 {{calls to global function 'mainActorDefaultArg(value:)' from outside of its actor context are implicitly asynchronous}} +@MainActor func mainActorDefaultArg(value: Int = requiresMainActor()) {} -func mainActorClosure( +// expected-note@+1 {{calls to global function 'mainActorClosure(closure:)' from outside of its actor context are implicitly asynchronous}} +@MainActor func mainActorClosure( closure: () -> Int = { requiresMainActor() } ) {} func mainActorClosureCall( - closure: Int = { + closure: Int = { // expected-error {{main actor-isolated default argument in a nonisolated context}} requiresMainActor() }() ) {} @@ -40,40 +52,29 @@ func mainActorClosureCall( mainActorClosureCall() } +// expected-note@+1 2 {{add '@MainActor' to make global function 'nonisolatedCaller()' part of global actor 'MainActor'}} func nonisolatedCaller() { - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} + // expected-error@+1 {{call to main actor-isolated global function 'mainActorDefaultArg(value:)' in a synchronous nonisolated context}} mainActorDefaultArg() - // FIXME: confusing error message. - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} + // expected-error@+1 {{call to main actor-isolated global function 'mainActorClosure(closure:)' in a synchronous nonisolated context}} mainActorClosure() - - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} - mainActorClosureCall() } func nonisolatedAsyncCaller() async { - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} + // expected-error@+2 {{expression is 'async' but is not marked with 'await'}} + // expected-note@+1 {{calls to global function 'mainActorDefaultArg(value:)' from outside of its actor context are implicitly asynchronous}} mainActorDefaultArg() - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} + // expected-error@+2 {{expression is 'async' but is not marked with 'await'}} + // expected-note@+1 {{calls to global function 'mainActorClosure(closure:)' from outside of its actor context are implicitly asynchronous}} mainActorClosure() - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} - mainActorClosureCall() - await mainActorDefaultArg(value: requiresMainActor()) - // 'await' doesn't help closures in default arguments; the calling context needs a matching - // actor isolation for that isolation to be inferred for the closure value. - - // expected-error@+2 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} - // expected-warning@+1 {{no 'async' operations occur within 'await' expression}} await mainActorClosure() - // expected-error@+2 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} - // expected-warning@+1 {{no 'async' operations occur within 'await' expression}} - await mainActorClosureCall() + mainActorClosureCall() } func conflictingIsolationDefaultArg( @@ -125,6 +126,7 @@ func closureWithIsolatedParam( } ) {} +// expected-note@+2 3 {{calls to initializer 'init(required:x:y:)' from outside of its actor context are implicitly asynchronous}} @MainActor struct S1 { var required: Int @@ -136,6 +138,7 @@ struct S1 { static var z: Int = requiresMainActor() } +// expected-note@+2 3 {{calls to initializer 'init(required:x:y:)' from outside of its actor context are implicitly asynchronous}} @SomeGlobalActor struct S2 { var required: Int @@ -156,32 +159,34 @@ struct S3 { func initializeFromMainActor() { _ = S1(required: 10) - // expected-error@+1 {{global actor 'SomeGlobalActor'-isolated default argument cannot be synchronously evaluated from a main actor-isolated context}} + // expected-error@+1 {{call to global actor 'SomeGlobalActor'-isolated initializer 'init(required:x:y:)' in a synchronous main actor-isolated context}} _ = S2(required: 10) } @SomeGlobalActor func initializeFromSomeGlobalActor() { - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a global actor 'SomeGlobalActor'-isolated context}} + // expected-error@+1 {{call to main actor-isolated initializer 'init(required:x:y:)' in a synchronous global actor 'SomeGlobalActor'-isolated context}} _ = S1(required: 10) _ = S2(required: 10) } +// expected-note@+2 {{add '@MainActor' to make global function 'initializeFromNonisolated()' part of global actor 'MainActor'}} +// expected-note@+1 {{add '@SomeGlobalActor' to make global function 'initializeFromNonisolated()' part of global actor 'SomeGlobalActor'}} func initializeFromNonisolated() { - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} + // expected-error@+1 {{call to main actor-isolated initializer 'init(required:x:y:)' in a synchronous nonisolated context}} _ = S1(required: 10) - // expected-error@+1 {{global actor 'SomeGlobalActor'-isolated default argument cannot be synchronously evaluated from a nonisolated context}} + // expected-error@+1 {{call to global actor 'SomeGlobalActor'-isolated initializer 'init(required:x:y:)' in a synchronous nonisolated context}} _ = S2(required: 10) } extension A { func initializeFromActorInstance() { - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a actor-isolated context}} + // expected-error@+1 {{call to main actor-isolated initializer 'init(required:x:y:)' in a synchronous actor-isolated context}} _ = S1(required: 10) - // expected-error@+1 {{global actor 'SomeGlobalActor'-isolated default argument cannot be synchronously evaluated from a actor-isolated context}} + // expected-error@+1 {{call to global actor 'SomeGlobalActor'-isolated initializer 'init(required:x:y:)' in a synchronous actor-isolated context}} _ = S2(required: 10) } } diff --git a/test/Concurrency/isolated_default_arguments_serialized.swift b/test/Concurrency/isolated_default_arguments_serialized.swift index b284fef283c9d..f80d45df91033 100644 --- a/test/Concurrency/isolated_default_arguments_serialized.swift +++ b/test/Concurrency/isolated_default_arguments_serialized.swift @@ -16,7 +16,6 @@ func mainActorCaller() { } func nonisolatedCaller() async { - // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}} await useMainActorDefault() await useMainActorDefault(mainActorDefaultArg()) From f44868484298319671ce7af2073528912f00a7af Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Wed, 25 Oct 2023 20:46:53 -0700 Subject: [PATCH 2/2] [Concurrency] Add a SILGen test for isolated default arguments. --- .../isolated_default_argument_eval.swift | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/Concurrency/isolated_default_argument_eval.swift diff --git a/test/Concurrency/isolated_default_argument_eval.swift b/test/Concurrency/isolated_default_argument_eval.swift new file mode 100644 index 0000000000000..13b1eafb753c0 --- /dev/null +++ b/test/Concurrency/isolated_default_argument_eval.swift @@ -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 + // 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 + await mainActorDefaultArg() +}