diff --git a/include/swift/AST/DiagnosticsSIL.def b/include/swift/AST/DiagnosticsSIL.def index a91cad7008069..23541856d13b3 100644 --- a/include/swift/AST/DiagnosticsSIL.def +++ b/include/swift/AST/DiagnosticsSIL.def @@ -1026,6 +1026,10 @@ NOTE(regionbasedisolation_named_isolated_closure_yields_race, none, "%0%1 is captured by a %2 closure. %2 uses in closure may race against later %3 uses", (StringRef, Identifier, ActorIsolation, ActorIsolation)) +NOTE(regionbasedisolation_named_transfer_nt_asynclet_capture, none, + "sending %1 %0 into async let risks causing data races between nonisolated and %1 uses", + (Identifier, StringRef)) + // Misc Error. ERROR(regionbasedisolation_task_or_actor_isolated_transferred, none, "task or actor isolated value cannot be sent", ()) diff --git a/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp b/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp index 3903c100eafa2..e8a43b2e7609d 100644 --- a/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp +++ b/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp @@ -111,7 +111,7 @@ getTransferringApplyCalleeInfo(SILInstruction *inst) { return {}; auto *decl = declRef->getDecl(); - if (!decl->hasName()) + if (!decl || !decl->hasName()) return {}; return {{decl->getDescriptiveKind(), decl->getName()}}; @@ -1217,6 +1217,10 @@ class TransferNonTransferrableDiagnosticEmitter { return getTransferringApplyCalleeInfo(info.transferredOperand->getUser()); } + SILLocation getLoc() const { + return info.transferredOperand->getUser()->getLoc(); + } + /// Return the isolation region info for \p getNonTransferrableValue(). SILDynamicMergedIsolationInfo getIsolationRegionInfo() const { return info.isolationRegionInfo; @@ -1292,6 +1296,24 @@ class TransferNonTransferrableDiagnosticEmitter { .limitBehaviorIf(getBehaviorLimit()); } + void emitNamedAsyncLetCapture(SILLocation loc, Identifier name, + SILIsolationInfo transferredValueIsolation) { + assert(!getIsolationRegionInfo().isDisconnected() && + "Should never be disconnected?!"); + emitNamedOnlyError(loc, name); + + SmallString<64> descriptiveKindStr; + { + llvm::raw_svector_ostream os(descriptiveKindStr); + getIsolationRegionInfo().printForDiagnostics(os); + } + + diagnoseNote(loc, + diag::regionbasedisolation_named_transfer_nt_asynclet_capture, + name, descriptiveKindStr) + .limitBehaviorIf(getBehaviorLimit()); + } + void emitNamedIsolation(SILLocation loc, Identifier name, ApplyIsolationCrossing isolationCrossing) { emitNamedOnlyError(loc, name); @@ -1402,6 +1424,8 @@ class TransferNonTransferrableDiagnosticEmitter { }; class TransferNonTransferrableDiagnosticInferrer { + struct AutoClosureWalker; + TransferNonTransferrableDiagnosticEmitter diagnosticEmitter; public: @@ -1454,6 +1478,74 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply( return false; } +/// This walker visits an AutoClosureExpr and looks for uses of a specific +/// captured value. We want to error on the uses in the autoclosure. +struct TransferNonTransferrableDiagnosticInferrer::AutoClosureWalker + : ASTWalker { + TransferNonTransferrableDiagnosticEmitter &foundTypeInfo; + ValueDecl *targetDecl; + SILIsolationInfo targetDeclIsolationInfo; + SmallPtrSet visitedCallExprDeclRefExprs; + SILLocation captureLoc; + bool isAsyncLet; + + AutoClosureWalker(TransferNonTransferrableDiagnosticEmitter &foundTypeInfo, + ValueDecl *targetDecl, + SILIsolationInfo targetDeclIsolationInfo, + SILLocation captureLoc, bool isAsyncLet) + : foundTypeInfo(foundTypeInfo), targetDecl(targetDecl), + targetDeclIsolationInfo(targetDeclIsolationInfo), + captureLoc(captureLoc), isAsyncLet(isAsyncLet) {} + + Expr *lookThroughArgExpr(Expr *expr) { + while (true) { + if (auto *memberRefExpr = dyn_cast(expr)) { + expr = memberRefExpr->getBase(); + continue; + } + + if (auto *cvt = dyn_cast(expr)) { + expr = cvt->getSubExpr(); + continue; + } + + if (auto *e = dyn_cast(expr)) { + expr = e->getSubExpr(); + continue; + } + + if (auto *t = dyn_cast(expr)) { + expr = t->getBase(); + continue; + } + + return expr; + } + } + + PreWalkResult walkToExprPre(Expr *expr) override { + if (auto *declRef = dyn_cast(expr)) { + // If this decl ref expr was not visited as part of a callExpr and is our + // target decl... emit a simple async let error. + // + // This occurs if we do: + // + // ``` + // let x = ... + // async let y = x + // ``` + if (declRef->getDecl() == targetDecl) { + foundTypeInfo.emitNamedAsyncLetCapture(captureLoc, + targetDecl->getBaseIdentifier(), + targetDeclIsolationInfo); + return Action::Continue(expr); + } + } + + return Action::Continue(expr); + } +}; + bool TransferNonTransferrableDiagnosticInferrer::run() { // We need to find the isolation info. auto *op = diagnosticEmitter.getOperand(); @@ -1570,6 +1662,26 @@ bool TransferNonTransferrableDiagnosticInferrer::run() { } } + // If we are failing due to an autoclosure... see if we can find the captured + // value that is causing the issue. + if (auto *autoClosureExpr = loc.getAsASTNode()) { + // To split up this work, we only do this for async let for now. + if (autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet) { + auto *i = op->getUser(); + auto pai = ApplySite::isa(i); + unsigned captureIndex = pai.getAppliedArgIndex(*op); + auto captureInfo = + autoClosureExpr->getCaptureInfo().getCaptures()[captureIndex]; + auto loc = RegularLocation(captureInfo.getLoc(), false /*implicit*/); + AutoClosureWalker walker( + diagnosticEmitter, captureInfo.getDecl(), + diagnosticEmitter.getIsolationRegionInfo().getIsolationInfo(), loc, + autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet); + autoClosureExpr->walk(walker); + return true; + } + } + diagnosticEmitter.emitUnknownUse(loc); return true; } diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 31dedb82ee604..ff564cc86fefb 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -741,6 +741,13 @@ isIsolationInferenceBoundaryClosure(const AbstractClosureExpr *closure, } } + // An autoclosure for an async let acts as a boundary. It is non-Sendable + // regardless of its context. + if (auto *autoclosure = dyn_cast(closure)) { + if (autoclosure->getThunkKind() == AutoClosureExpr::Kind::AsyncLet) + return true; + } + return isSendableClosure(closure, forActorIsolation); } diff --git a/test/Concurrency/global_actor_function_types.swift b/test/Concurrency/global_actor_function_types.swift index d4709407f9960..7da8d58d093a7 100644 --- a/test/Concurrency/global_actor_function_types.swift +++ b/test/Concurrency/global_actor_function_types.swift @@ -327,9 +327,12 @@ func stripActor(_ expr: @Sendable @autoclosure () -> (() -> ())) async { return await stripActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}} } -// NOTE: this warning is correct, but is only being caught by TypeCheckConcurrency's extra check. +// We used to not emit an error here with strict-concurrency enabled since we +// were inferring the async let to main actor isolated (which was incorrect). We +// now always treat async let as non-isolated, so we get the same error in all +// contexts. @MainActor func exampleWhereConstraintSolverHasWrongDeclContext_v2() async -> Int { - async let a: () = noActor(mainActorFn) // expected-without-transferring-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}} + async let a: () = noActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}} await a } diff --git a/test/Concurrency/issue-57376.swift b/test/Concurrency/issue-57376.swift index 15b7959a30934..fc4ec6a645c0f 100644 --- a/test/Concurrency/issue-57376.swift +++ b/test/Concurrency/issue-57376.swift @@ -26,47 +26,53 @@ func testAsyncSequence1Sendable(_ seq: Seq) async throws whe func testAsyncSequenceTypedPattern(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}} // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}} - async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}} - // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} - // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}} + // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}} + // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} let _ = try! await result } func testAsyncSequenceTypedPattern1(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}} // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}} - async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}} - // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} - // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}} + // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}} + // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} } func testAsyncSequence(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}} // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}} - async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}} - // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} - // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}} + // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}} + // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} let _ = try! await result } func testAsyncSequence1(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}} // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}} - async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}} - // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} - // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}} + // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}} + // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} } func testAsyncSequence3(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}} // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}} - async let result = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}} - // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} - // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + async let result = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}} + // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}} + // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} let _ = await result } func testAsyncSequence4(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}} // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}} - async let _ = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}} - // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} - // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + async let _ = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}} + // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}} + // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} + // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}} } func search(query: String, entities: [String]) async throws -> [String] { diff --git a/test/Concurrency/transfernonsendable_asynclet.swift b/test/Concurrency/transfernonsendable_asynclet.swift index 98674c2369acb..ccdf105f73aa7 100644 --- a/test/Concurrency/transfernonsendable_asynclet.swift +++ b/test/Concurrency/transfernonsendable_asynclet.swift @@ -18,7 +18,11 @@ class NonSendableKlass { class SendableKlass : @unchecked Sendable {} -actor Actor { +struct NonSendableStruct { // expected-note {{}} + var ns = NonSendableKlass() +} + +actor MyActor { var klass = NonSendableKlass() final var finalKlass = NonSendableKlass() @@ -28,7 +32,7 @@ actor Actor { func useNonSendableFunction(_: () -> Void) {} } -final actor FinalActor { +final actor FinalMyActor { var klass = NonSendableKlass() func useKlass(_ x: NonSendableKlass) {} } @@ -45,6 +49,13 @@ func useInOut(_ x: inout T) {} func useValue(_ x: T) -> T { x } func useValueWrapInOptional(_ x: T) -> T? { x } +func useValueNoReturnWithInstance(_ x: T, _ y: V) -> () { fatalError() } +func useValueAsyncNoReturnWithInstance(_ x: T, _ y: V) async -> () { fatalError() } +@MainActor +func useMainActorValueAsyncNoReturn(_ x: T) async -> () { fatalError() } +@MainActor +func useMainActorValueNoReturn(_ x: T) -> () { fatalError() } + @MainActor func returnValueFromMain() async -> T { fatalError() } @MainActor func transferToMain(_ t: T) async {} @MainActor func transferToMainInt(_ t: T) async -> Int { 5 } @@ -720,7 +731,7 @@ func asyncLetWithoutCapture() async { } func asyncLet_Let_ActorIsolated_Method() async { - let a = Actor() + let a = MyActor() let x = NonSendableKlass() async let y = a.useKlass(x) // expected-warning {{sending 'x' risks causing data races}} // expected-note @-1 {{sending 'x' to actor-isolated instance method 'useKlass' risks causing data races between actor-isolated and local nonisolated uses}} @@ -728,3 +739,42 @@ func asyncLet_Let_ActorIsolated_Method() async { useValue(x) // expected-note {{access can happen concurrently}} let _ = await y } + +extension NonSendableStruct { + func asyncLetInferAsNonIsolated( + isolation actor: isolated T + ) async throws { + async let subTask: Void = { + await useValueAsyncNoReturnWithInstance(self, actor) + // expected-warning @-1:47 {{sending 'self' risks causing data races}} + // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}} + }() + await subTask + + async let subTask2: () = await useValueAsyncNoReturnWithInstance(self, actor) + // expected-warning @-1 {{sending 'self' risks causing data races}} + // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}} + await subTask2 + + async let subTask3: () = useValueNoReturnWithInstance(self, actor) + // expected-warning @-1 {{sending 'self' risks causing data races}} + // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}} + await subTask3 + + async let subTask4: () = await useMainActorValueAsyncNoReturn(self) + // expected-warning @-1 {{sending 'self' risks causing data races}} + // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}} + await subTask4 + + async let subTask5: () = useMainActorValueNoReturn(self) + // expected-warning @-1 {{sending 'self' risks causing data races}} + // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}} + await subTask5 + + async let subTask6: NonSendableStruct = self + // expected-warning @-1 {{sending 'self' risks causing data races}} + // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}} + // expected-warning @-3 {{non-sendable type 'NonSendableStruct' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}} + _ = await subTask6 + } +} diff --git a/test/Concurrency/transfernonsendable_sending_results.swift b/test/Concurrency/transfernonsendable_sending_results.swift index 2067ba447f3b5..586be6b34abf6 100644 --- a/test/Concurrency/transfernonsendable_sending_results.swift +++ b/test/Concurrency/transfernonsendable_sending_results.swift @@ -7,7 +7,7 @@ // MARK: Declarations // //////////////////////// -class NonSendableKlass {} // expected-note {{}} +class NonSendableKlass {} // expected-note 2{{}} struct NonSendableStruct { var first = NonSendableKlass() @@ -17,6 +17,8 @@ struct NonSendableStruct { func getValue() -> T { fatalError() } func getValueAsync() async -> T { fatalError() } func getValueAsyncTransferring() async -> sending T { fatalError() } +@MainActor func getMainActorValueAsync() async -> T { fatalError() } +@MainActor func getMainActorValueAsyncTransferring() async -> sending T { fatalError() } func useValue(_ t: T) {} func getAny() -> Any { fatalError() } @@ -209,9 +211,22 @@ func asyncLetReabstractionThunkTest() async { let _ = await newValue2 } +func asyncLetReabstractionThunkTest2() async { + // We emit the error here since we are returning a main actor isolated value. + async let newValue: NonSendableKlass = await getMainActorValueAsync() + // expected-warning @-1 {{non-sendable type 'NonSendableKlass' returned by implicitly asynchronous call to main actor-isolated function cannot cross actor boundary}} + + let _ = await newValue + + // Without thunk. + async let newValue2: NonSendableKlass = await getMainActorValueAsyncTransferring() + let _ = await newValue2 +} + @MainActor func asyncLetReabstractionThunkTestGlobalActor() async { - // With thunk. We emit the sema error here. - async let newValue: NonSendableKlass = await getValueAsync() // expected-warning {{non-sendable type 'NonSendableKlass' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}} + // With thunk we do not emit an error since our async let is not main actor + // isolated despite being in an @MainActor function. + async let newValue: NonSendableKlass = await getValueAsync() let _ = await newValue // Without thunk. @@ -219,4 +234,14 @@ func asyncLetReabstractionThunkTest() async { let _ = await newValue2 } +@MainActor func asyncLetReabstractionThunkTestGlobalActor2() async { + // We emit the error here since we are returning a main actor isolated value. + async let newValue: NonSendableKlass = await getMainActorValueAsync() + // expected-warning @-1 {{non-sendable type 'NonSendableKlass' returned by implicitly asynchronous call to main actor-isolated function cannot cross actor boundary}} + + let _ = await newValue + // Without thunk. + async let newValue2: NonSendableKlass = await getMainActorValueAsyncTransferring() + let _ = await newValue2 +}