diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 267bc27164c19..061f350a80430 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5077,6 +5077,9 @@ ERROR(actor_isolated_non_self_reference,none, "from a non-isolated autoclosure}3", (DescriptiveDeclKind, DeclName, unsigned, unsigned, Type, ActorIsolation)) +ERROR(subscript_mutation_outside_actor,none, + "%0 %1 %2 cannot be mutated from outside the actor", + (ActorIsolation, DescriptiveDeclKind, DeclName)) ERROR(distributed_actor_isolated_non_self_reference,none, "distributed actor-isolated %0 %1 can not be accessed from a " "non-isolated context", diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 58d2a4b6ad9a7..6884cd7dd2cf0 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2041,48 +2041,41 @@ namespace { contextStack.push_back(dc); } - /// Searches the applyStack from back to front for the inner-most CallExpr - /// and marks that CallExpr as implicitly async. - /// - /// NOTE: Crashes if no CallExpr was found. - /// - /// For example, for global actor function `curryAdd`, if we have: - /// ((curryAdd 1) 2) - /// then we want to mark the inner-most CallExpr, `(curryAdd 1)`. - /// - /// The same goes for calls to member functions, such as calc.add(1, 2), - /// aka ((add calc) 1 2), looks like this: - /// - /// (call_expr - /// (dot_syntax_call_expr - /// (declref_expr add) - /// (declref_expr calc)) - /// (tuple_expr - /// ...)) - /// - /// and we reach up to mark the CallExpr. - void markNearestCallAsImplicitly(llvm::Optional setAsync, - bool setThrows = false, - bool setDistributedThunk = false) { - assert(applyStack.size() > 0 && "not contained within an Apply?"); - - const auto End = applyStack.rend(); - for (auto I = applyStack.rbegin(); I != End; ++I) - if (auto call = dyn_cast(*I)) { - if (setAsync) { - call->setImplicitlyAsync(*setAsync); - } - if (setThrows) { - call->setImplicitlyThrows(true); - }else { - call->setImplicitlyThrows(false); - } - if (setDistributedThunk) { - call->setShouldApplyDistributedThunk(true); - } - return; + /// Mark the given expression + void markImplicitlyPromoted( + Expr *expr, + llvm::Optional setAsync, + bool setThrows = false, + bool setDistributedThunk = false) { + if (auto apply = dyn_cast(expr)) { + if (setAsync) { + apply->setImplicitlyAsync(*setAsync); + } + + apply->setImplicitlyThrows(setThrows); + if (setDistributedThunk) { + apply->setShouldApplyDistributedThunk(true); + } + + return; + } + + if (auto lookup = dyn_cast(expr)) { + assert(usageEnv(lookup) == VarRefUseEnv::Read); + + if (setAsync) + lookup->setImplicitlyAsync(*setAsync); + lookup->setImplicitlyThrows(setThrows); + + if (setDistributedThunk) { + if (auto memberRef = dyn_cast(lookup)) + memberRef->setAccessViaDistributedThunk(); } - llvm_unreachable("expected a CallExpr in applyStack!"); + + return; + } + + llvm_unreachable("Node cannot be implicitly promoted"); } bool shouldWalkCaptureInitializerExpressions() override { return true; } @@ -2153,6 +2146,19 @@ namespace { if (auto load = dyn_cast(expr)) recordMutableVarParent(load, load->getSubExpr()); + if (auto subscript = dyn_cast(expr)) { + if (ConcreteDeclRef subscriptDecl = subscript->getMember()) { + Type type = subscriptDecl.getDecl()->getInterfaceType().subst( + subscriptDecl.getSubstitutions()); + if (auto fnType = type->getAs()) { + (void)checkApply( + subscript, subscriptDecl, subscript->getBase(), + fnType, subscript->getArgs()); + } + } + return Action::Continue(expr); + } + if (auto lookup = dyn_cast(expr)) { checkReference(lookup->getBase(), lookup->getMember(), lookup->getLoc(), /*partialApply*/ llvm::None, lookup); @@ -2207,7 +2213,14 @@ namespace { } } else { // Check the call itself. - (void)checkApply(apply); + if (auto fnExprType = getType(apply->getFn())) { + if (auto fnType = fnExprType->getAs()) { + (void)checkApply( + apply, + apply->getCalledValue(/*skipFunctionConversions=*/true), + apply->getFn(), fnType, apply->getArgs()); + } + } } } @@ -2591,8 +2604,6 @@ namespace { } } - // FIXME: Subscript? - // This is either non-distributed variable, subscript, or something else. ctx.Diags.diagnose(declLoc, diag::distributed_actor_isolated_non_self_reference, @@ -2697,14 +2708,11 @@ namespace { } /// Check actor isolation for a particular application. - bool checkApply(ApplyExpr *apply) { - auto fnExprType = getType(apply->getFn()); - if (!fnExprType) - return false; - - auto fnType = fnExprType->getAs(); - if (!fnType) - return false; + bool checkApply( + Expr *apply, ConcreteDeclRef callee, Expr *base, + AnyFunctionType *fnType, ArgumentList *args + ) { + SourceLoc loc = apply->getLoc(); // The isolation of the context we're in. llvm::Optional contextIsolation; @@ -2718,17 +2726,22 @@ namespace { return *contextIsolation; }; + // Determine whether the calle itself is async. + auto calleeDecl = callee.getDecl(); + bool isCalleeAsync = calleeDecl + ? isAsyncDecl(calleeDecl) + : fnType->getExtInfo().isAsync(); + // Default the call options to allow promotion to async, if it will be // warranted. ActorReferenceResult::Options callOptions; - if (!fnType->getExtInfo().isAsync()) + if (!isCalleeAsync) callOptions |= ActorReferenceResult::Flags::AsyncPromotion; // Determine from the callee whether actor isolation is unsatisfied. llvm::Optional unsatisfiedIsolation; bool mayExitToNonisolated = true; Expr *argForIsolatedParam = nullptr; - auto calleeDecl = apply->getCalledValue(/*skipFunctionConversions=*/true); if (Type globalActor = fnType->getGlobalActor()) { // If the function type is global-actor-qualified, determine whether // we are within that global actor already. @@ -2739,8 +2752,35 @@ namespace { } mayExitToNonisolated = false; + } else if (calleeDecl && + calleeDecl->getAttrs() + .hasAttribute()) { + // The callee always inherits the executor. + return false; + } else if (calleeDecl && isa(calleeDecl)) { + auto isolatedActor = getIsolatedActor(base); + auto result = ActorReferenceResult::forReference( + callee, base->getLoc(), getDeclContext(), + kindOfUsage(calleeDecl, apply), + isolatedActor, llvm::None, llvm::None, getClosureActorIsolation); + switch (result) { + case ActorReferenceResult::SameConcurrencyDomain: + break; + + case ActorReferenceResult::ExitsActorToNonisolated: + unsatisfiedIsolation = ActorIsolation::forIndependent(); + break; + + case ActorReferenceResult::EntersActor: + unsatisfiedIsolation = result.isolation; + break; + } + + callOptions = result.options; + mayExitToNonisolated = false; + argForIsolatedParam = base; } else if (auto *selfApplyFn = dyn_cast( - apply->getFn()->getValueProvidingExpr())) { + base->getValueProvidingExpr())) { // If we're calling a member function, check whether the function // itself is isolated. auto memberFn = selfApplyFn->getFn()->getValueProvidingExpr(); @@ -2768,10 +2808,6 @@ namespace { calleeDecl = memberRef->first.getDecl(); argForIsolatedParam = selfApplyFn->getBase(); } - } else if (calleeDecl && - calleeDecl->getAttrs() - .hasAttribute()) { - return false; } // Check for isolated parameters. @@ -2780,7 +2816,6 @@ namespace { if (!fnType->getParams()[paramIdx].isIsolated()) continue; - auto *args = apply->getArgs(); if (paramIdx >= args->size()) continue; @@ -2802,7 +2837,7 @@ namespace { unsatisfiedIsolation = ActorIsolation::forActorInstanceParameter(nominal, paramIdx); - if (!fnType->getExtInfo().isAsync()) + if (!isCalleeAsync) callOptions |= ActorReferenceResult::Flags::AsyncPromotion; mayExitToNonisolated = false; @@ -2811,7 +2846,7 @@ namespace { // If we're calling an async function that's nonisolated, and we're in // an isolated context, then we're exiting the actor context. - if (mayExitToNonisolated && fnType->isAsync() && + if (mayExitToNonisolated && isCalleeAsync && getContextIsolation().isActorIsolated()) unsatisfiedIsolation = ActorIsolation::forIndependent(); @@ -2827,7 +2862,7 @@ namespace { if (requiresAsync && !getDeclContext()->isAsyncContext()) { if (calleeDecl) { ctx.Diags.diagnose( - apply->getLoc(), diag::actor_isolated_call_decl, + loc, diag::actor_isolated_call_decl, *unsatisfiedIsolation, calleeDecl->getDescriptiveKind(), calleeDecl->getName(), getContextIsolation()) @@ -2837,7 +2872,7 @@ namespace { calleeDecl->getName()); } else { ctx.Diags.diagnose( - apply->getLoc(), diag::actor_isolated_call, *unsatisfiedIsolation, + loc, diag::actor_isolated_call, *unsatisfiedIsolation, getContextIsolation()) .warnUntilSwiftVersionIf(getContextIsolation().preconcurrency(), 6); } @@ -2859,7 +2894,7 @@ namespace { if (unsatisfiedIsolation->isDistributedActor() && !(calleeDecl && isa(calleeDecl))) { auto distributedAccess = checkDistributedAccess( - apply->getFn()->getLoc(), calleeDecl, argForIsolatedParam); + loc, calleeDecl, argForIsolatedParam); if (!distributedAccess) return true; @@ -2868,8 +2903,23 @@ namespace { // Mark as implicitly async/throws/distributed thunk as needed. if (requiresAsync || setThrows || usesDistributedThunk) { - markNearestCallAsImplicitly( - unsatisfiedIsolation, setThrows, usesDistributedThunk); + // For a subscript, make sure it's a read-only access. + if (calleeDecl && isa(calleeDecl)) { + auto useKind = usageEnv(cast(apply)); + if (useKind != VarRefUseEnv::Read) { + ctx.Diags.diagnose(loc, diag::subscript_mutation_outside_actor, + *unsatisfiedIsolation, + calleeDecl->getDescriptiveKind(), + calleeDecl->getName()); + calleeDecl->diagnose( + diag::actor_mutable_state, calleeDecl->getDescriptiveKind()); + + return true; + } + } + + markImplicitlyPromoted( + apply, unsatisfiedIsolation, setThrows, usesDistributedThunk); } // Check for sendability of the parameter types. @@ -2878,9 +2928,9 @@ namespace { const auto ¶m = params[paramIdx]; // Dig out the location of the argument. - SourceLoc argLoc = apply->getLoc(); + SourceLoc argLoc = loc; Type argType; - if (auto argList = apply->getArgs()) { + if (auto argList = args) { auto arg = argList->get(paramIdx); if (arg.getStartLoc().isValid()) argLoc = arg.getStartLoc(); @@ -2904,9 +2954,9 @@ namespace { // Check for sendability of the result type. if (diagnoseNonSendableTypes( - fnType->getResult(), getDeclContext(), apply->getLoc(), + fnType->getResult(), getDeclContext(), loc, diag::non_sendable_call_result_type, - apply->isImplicitlyAsync().has_value(), + requiresAsync, *unsatisfiedIsolation)) return true; diff --git a/test/Concurrency/global_actor_from_ordinary_context.swift b/test/Concurrency/global_actor_from_ordinary_context.swift index 8668691c1c481..b00ac8736b084 100644 --- a/test/Concurrency/global_actor_from_ordinary_context.swift +++ b/test/Concurrency/global_actor_from_ordinary_context.swift @@ -37,7 +37,7 @@ func referenceGlobalActor() async { // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }} _ = a[1] // expected-note{{subscript access is 'async'}} - a[0] = 1 // expected-error{{global actor 'SomeGlobalActor'-isolated subscript 'subscript(_:)' can not be mutated from a non-isolated context}} + a[0] = 1 // expected-error{{global actor 'SomeGlobalActor'-isolated subscript 'subscript(_:)' cannot be mutated from outside the actor}} // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }} _ = 32 + a[1] // expected-note@:12{{subscript access is 'async'}} @@ -51,10 +51,12 @@ func referenceGlobalActor2() { } +// expected-note@+2 {{add '@SomeGlobalActor' to make global function 'referenceAsyncGlobalActor()' part of global actor 'SomeGlobalActor'}} // expected-note@+1 {{add 'async' to function 'referenceAsyncGlobalActor()' to make it asynchronous}} {{33-33= async}} func referenceAsyncGlobalActor() { - let y = asyncGlobalActFn + let y = asyncGlobalActFn // expected-note{{calls to let 'y' from outside of its actor context are implicitly asynchronous}} y() // expected-error{{'async' call in a function that does not support concurrency}} + // expected-error@-1{{call to global actor 'SomeGlobalActor'-isolated let 'y' in a synchronous nonisolated context}} } @@ -111,7 +113,7 @@ func fromAsync() async { let y = asyncGlobalActFn // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }} - y() // expected-note{{call is 'async'}} + y() // expected-note{{calls to let 'y' from outside of its actor context are implicitly asynchronous}} let a = Alex() let fn = a.method @@ -124,7 +126,7 @@ func fromAsync() async { // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }} _ = a[1] // expected-note{{subscript access is 'async'}} _ = await a[1] - a[0] = 1 // expected-error{{global actor 'SomeGlobalActor'-isolated subscript 'subscript(_:)' can not be mutated from a non-isolated context}} + a[0] = 1 // expected-error{{global actor 'SomeGlobalActor'-isolated subscript 'subscript(_:)' cannot be mutated from outside the actor}} } // expected-note@+1{{mutation of this var is only permitted within the actor}} diff --git a/test/Concurrency/sendable_checking.swift b/test/Concurrency/sendable_checking.swift index 2bfe6b521b352..f7071e0957b96 100644 --- a/test/Concurrency/sendable_checking.swift +++ b/test/Concurrency/sendable_checking.swift @@ -223,10 +223,19 @@ class SubWUnsafeSubscript : SuperWUnsafeSubscript { extension MyActor { func f(_: Any) { } func g(_: () -> Void) { } + + subscript (value: Any) -> String { "\(value)" } + subscript (value: () -> Void) -> String { + value() + return "hello" + } } @available(SwiftStdlib 5.1, *) func testConversionsAndSendable(a: MyActor, s: any Sendable, f: @Sendable () -> Void) async { await a.f(s) await a.g(f) + + _ = await a[s] + _ = await a[f] }