From bfb1f2effa993f773018bbf1a0bf41613519d7c3 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Tue, 13 Aug 2024 13:02:22 -0700 Subject: [PATCH] [region-isolation] Improve the error we emit for closure literals captured as a sending parameter. Specifically: I changed the main error message to focus on the closure and that the closure is being accessed concurrently. If we find that we captured a value that is the actual isolation source, we emit that the capture is actually actor isolated. If the captured value is in the same region as the isolated value but is not isolated, we instead say that the value is accessible from *-isolated code or code within the current task. If we find multiple captures and we do not which is the actual value that was in the same region before we formed the partial apply, we just emit a note on the captures saying that the closure captures the value. I changed the diagnostics from using the phrase "task-isolated" to use some variant of accessible to code in the current task. The idea is that in all situations we provide a breadcrumb that the user can start investigating rather than just saying that the closure is "task-isolated". From a preconcurrency perspective, I made it so that we apply the preconcurrency behavior of all of the captures. This means that if one of the captures is preconcurrency, we apply the preconcurrency restriction to the closure. This is one step towards making it so that preconcurrency applies at the region level... we just are not completely there yet. rdar://133798044 (cherry picked from commit 4bb2e4f3b1253444ee96dbe685f0b33ff3cfe37e) Conflicts: include/swift/AST/DiagnosticsSIL.def lib/SILOptimizer/Mandatory/TransferNonSendable.cpp test/Concurrency/concurrent_value_checking.swift test/Concurrency/isolated_parameters.swift test/Concurrency/sendable_preconcurrency.swift test/Concurrency/sendable_without_preconcurrency.swift test/Concurrency/sendable_without_preconcurrency_2.swift test/Concurrency/transfernonsendable.swift test/Concurrency/transfernonsendable_global_actor_sending.swift --- include/swift/AST/DiagnosticsSIL.def | 17 ++ .../swift/SILOptimizer/Utils/PartitionUtils.h | 6 + .../Mandatory/TransferNonSendable.cpp | 244 +++++++++++++++++- .../concurrent_value_checking.swift | 16 +- test/Concurrency/isolated_parameters.swift | 4 +- .../Concurrency/sendable_preconcurrency.swift | 9 +- .../sendable_without_preconcurrency.swift | 10 +- .../sendable_without_preconcurrency_2.swift | 4 +- ...sfernonsendable_global_actor_sending.swift | 20 +- ...ernonsendable_preconcurrency_sending.swift | 188 ++++++++++++++ .../transfernonsendable_sending_params.swift | 61 ++++- 11 files changed, 551 insertions(+), 28 deletions(-) diff --git a/include/swift/AST/DiagnosticsSIL.def b/include/swift/AST/DiagnosticsSIL.def index 2d7f7c29e3e87..72c26dffd61cc 100644 --- a/include/swift/AST/DiagnosticsSIL.def +++ b/include/swift/AST/DiagnosticsSIL.def @@ -1041,6 +1041,23 @@ 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)) +ERROR(regionbasedisolation_typed_tns_passed_sending_closure, none, + "passing closure as a 'sending' parameter risks causing data races between %0 and concurrent execution of the closure", + (StringRef)) +NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value, none, + "closure captures %0 %1", + (StringRef, DeclName)) +NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_task_isolated, none, + "closure captures %0 which is accessible to code in the current task", + (DeclName)) +NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_region, none, + "closure captures %1 which is accessible to %0 code", + (StringRef, DeclName)) + +NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_multiple_value, none, + "closure captures non-Sendable %0", + (DeclName)) + 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)) diff --git a/include/swift/SILOptimizer/Utils/PartitionUtils.h b/include/swift/SILOptimizer/Utils/PartitionUtils.h index 8cac507bc068c..fcdfe37965937 100644 --- a/include/swift/SILOptimizer/Utils/PartitionUtils.h +++ b/include/swift/SILOptimizer/Utils/PartitionUtils.h @@ -141,6 +141,12 @@ class RepresentativeValue { return value.get(); } + bool operator==(SILValue other) const { + if (hasRegionIntroducingInst()) + return false; + return getValue() == other; + } + SWIFT_DEBUG_DUMP { print(llvm::dbgs()); } private: diff --git a/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp b/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp index 6bca8beb3e022..1ab36f86849a5 100644 --- a/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp +++ b/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp @@ -58,6 +58,29 @@ using Region = PartitionPrimitives::Region; // MARK: Utilities //===----------------------------------------------------------------------===// +static SILValue stripFunctionConversions(SILValue val) { + while (true) { + if (auto ti = dyn_cast(val)) { + val = ti->getOperand(); + continue; + } + + if (auto cfi = dyn_cast(val)) { + val = cfi->getOperand(); + continue; + } + + if (auto cvt = dyn_cast(val)) { + val = cvt->getOperand(); + continue; + } + + break; + } + + return val; +} + static std::optional getDiagnosticBehaviorLimitForValue(SILValue value) { auto *nom = value->getType().getNominalOrBoundGenericNominal(); @@ -72,6 +95,38 @@ getDiagnosticBehaviorLimitForValue(SILValue value) { return getConcurrencyDiagnosticBehaviorLimit(nom, fromDC); } +static std::optional +getDiagnosticBehaviorLimitForCapturedValue(CapturedValue value) { + ValueDecl *decl = value.getDecl(); + auto *nom = decl->getInterfaceType()->getNominalOrBoundGenericNominal(); + if (!nom) + return {}; + + auto *fromDC = decl->getInnermostDeclContext(); + return getConcurrencyDiagnosticBehaviorLimit(nom, fromDC); +} + +/// Find the most conservative diagnostic behavior by taking the max over all +/// DiagnosticBehavior for the captured values. +static std::optional +getDiagnosticBehaviorLimitForCapturedValues( + ArrayRef capturedValues) { + using UnderlyingType = std::underlying_type::type; + + std::optional diagnosticBehavior; + for (auto value : capturedValues) { + auto lhs = UnderlyingType( + diagnosticBehavior.value_or(DiagnosticBehavior::Unspecified)); + auto rhs = UnderlyingType( + getDiagnosticBehaviorLimitForCapturedValue(value).value_or( + DiagnosticBehavior::Unspecified)); + auto result = DiagnosticBehavior(std::max(lhs, rhs)); + if (result != DiagnosticBehavior::Unspecified) + diagnosticBehavior = result; + } + return diagnosticBehavior; +} + static std::optional getDeclRefForCallee(SILInstruction *inst) { auto fas = FullApplySite::isa(inst); if (!fas) @@ -1318,6 +1373,98 @@ class TransferNonTransferrableDiagnosticEmitter { .limitBehaviorIf(getBehaviorLimit()); } + /// Only use if we were able to find the actual isolated value. + void emitTypedSendingNeverSendableToSendingClosureParamDirectlyIsolated( + SILLocation loc, CapturedValue capturedValue) { + SmallString<64> descriptiveKindStr; + { + llvm::raw_svector_ostream os(descriptiveKindStr); + if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) { + os << "code in the current task"; + } else { + getIsolationRegionInfo().printForDiagnostics(os); + os << " code"; + } + } + + diagnoseError(loc, + diag::regionbasedisolation_typed_tns_passed_sending_closure, + descriptiveKindStr) + .highlight(loc.getSourceRange()) + .limitBehaviorIf( + getDiagnosticBehaviorLimitForCapturedValue(capturedValue)); + + auto capturedLoc = RegularLocation(capturedValue.getLoc()); + if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) { + auto diag = diag:: + regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_task_isolated; + diagnoseNote(capturedLoc, diag, capturedValue.getDecl()->getName()); + return; + } + + descriptiveKindStr.clear(); + { + llvm::raw_svector_ostream os(descriptiveKindStr); + getIsolationRegionInfo().printForDiagnostics(os); + } + + auto diag = diag:: + regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value; + diagnoseNote(capturedLoc, diag, descriptiveKindStr, + capturedValue.getDecl()->getName()); + } + + void emitTypedSendingNeverSendableToSendingClosureParam( + SILLocation loc, ArrayRef capturedValues) { + SmallString<64> descriptiveKindStr; + { + llvm::raw_svector_ostream os(descriptiveKindStr); + if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) { + os << "code in the current task"; + } else { + getIsolationRegionInfo().printForDiagnostics(os); + os << " code"; + } + } + + auto behaviorLimit = + getDiagnosticBehaviorLimitForCapturedValues(capturedValues); + diagnoseError(loc, + diag::regionbasedisolation_typed_tns_passed_sending_closure, + descriptiveKindStr) + .highlight(loc.getSourceRange()) + .limitBehaviorIf(behaviorLimit); + + if (capturedValues.size() == 1) { + auto captured = capturedValues.front(); + auto capturedLoc = RegularLocation(captured.getLoc()); + if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) { + auto diag = diag:: + regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_task_isolated; + diagnoseNote(capturedLoc, diag, captured.getDecl()->getName()); + return; + } + + descriptiveKindStr.clear(); + { + llvm::raw_svector_ostream os(descriptiveKindStr); + getIsolationRegionInfo().printForDiagnostics(os); + } + auto diag = diag:: + regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_region; + diagnoseNote(capturedLoc, diag, descriptiveKindStr, + captured.getDecl()->getName()); + return; + } + + for (auto captured : capturedValues) { + auto capturedLoc = RegularLocation(captured.getLoc()); + auto diag = diag:: + regionbasedisolation_typed_tns_passed_to_sending_closure_helper_multiple_value; + diagnoseNote(capturedLoc, diag, captured.getDecl()->getName()); + } + } + void emitNamedOnlyError(SILLocation loc, Identifier name) { diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race, name) @@ -1455,12 +1602,13 @@ class TransferNonTransferrableDiagnosticEmitter { class TransferNonTransferrableDiagnosticInferrer { struct AutoClosureWalker; + RegionAnalysisValueMap &valueMap; TransferNonTransferrableDiagnosticEmitter diagnosticEmitter; public: TransferNonTransferrableDiagnosticInferrer( - TransferredNonTransferrableInfo info) - : diagnosticEmitter(info) {} + RegionAnalysisValueMap &valueMap, TransferredNonTransferrableInfo info) + : valueMap(valueMap), diagnosticEmitter(info) {} /// Gathers diagnostics. Returns false if we emitted a "I don't understand /// error". If we emit such an error, we should bail without emitting any @@ -1474,10 +1622,94 @@ class TransferNonTransferrableDiagnosticInferrer { bool initForIsolatedPartialApply( Operand *op, AbstractClosureExpr *ace, std::optional actualCallerIsolation = {}); + + bool initForSendingPartialApply(FullApplySite fas, Operand *pai); + + std::optional + getIsolatedValuePartialApplyIndex(PartialApplyInst *pai, + SILValue isolatedValue) { + for (auto &paiOp : ApplySite(pai).getArgumentOperands()) { + if (valueMap.getTrackableValue(paiOp.get()).getRepresentative() == + isolatedValue) { + // isolated_any causes all partial apply parameters to be shifted by 1 + // due to the implicit isolated any parameter. + unsigned isIsolatedAny = pai->getFunctionType()->getIsolation() == + SILFunctionTypeIsolation::Erased; + return ApplySite(pai).getAppliedArgIndex(paiOp) - isIsolatedAny; + } + } + + return {}; + } }; } // namespace +bool TransferNonTransferrableDiagnosticInferrer::initForSendingPartialApply( + FullApplySite fas, Operand *paiOp) { + auto *pai = + dyn_cast(stripFunctionConversions(paiOp->get())); + if (!pai) + return false; + + // For now we want this to be really narrow and to only apply to closure + // literals. + auto *ce = pai->getLoc().getAsASTNode(); + if (!ce) + return false; + + // Ok, we now know we have a partial apply and it is a closure literal. Lets + // see if we can find the exact thing that caused the closure literal to be + // actor isolated. + auto isolationInfo = diagnosticEmitter.getIsolationRegionInfo(); + if (isolationInfo->hasIsolatedValue()) { + // Now that we have the value, see if that value is one of our captured + // values. + auto isolatedValue = isolationInfo->getIsolatedValue(); + auto matchingElt = getIsolatedValuePartialApplyIndex(pai, isolatedValue); + if (matchingElt) { + // Ok, we found the matching element. Lets emit our diagnostic! + auto capture = ce->getCaptureInfo().getCaptures()[*matchingElt]; + diagnosticEmitter + .emitTypedSendingNeverSendableToSendingClosureParamDirectlyIsolated( + ce, capture); + return true; + } + } + + // Ok, we are not tracking an actual isolated value or we do not capture the + // isolated value directly... we need to be smarter here. First lets gather up + // all non-Sendable values captured by the closure. + SmallVector nonSendableCaptures; + for (auto capture : ce->getCaptureInfo().getCaptures()) { + auto *decl = capture.getDecl(); + auto type = decl->getInterfaceType()->getCanonicalType(); + auto silType = SILType::getPrimitiveObjectType(type); + if (!SILIsolationInfo::isNonSendableType(silType, pai->getFunction())) + continue; + + auto *fromDC = decl->getInnermostDeclContext(); + auto *nom = silType.getNominalOrBoundGenericNominal(); + if (nom && fromDC) { + if (auto diagnosticBehavior = + getConcurrencyDiagnosticBehaviorLimit(nom, fromDC)) { + if (*diagnosticBehavior == DiagnosticBehavior::Ignore) + continue; + } + } + nonSendableCaptures.push_back(capture); + } + + // If we do not have any non-Sendable captures... bail. + if (nonSendableCaptures.empty()) + return false; + + // Otherwise, emit the diagnostic. + diagnosticEmitter.emitTypedSendingNeverSendableToSendingClosureParam( + ce, nonSendableCaptures); + return true; +} + bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply( Operand *op, AbstractClosureExpr *ace, std::optional actualCallerIsolation) { @@ -1585,6 +1817,11 @@ bool TransferNonTransferrableDiagnosticInferrer::run() { if (auto fas = FullApplySite::isa(op->getUser())) { if (fas.getArgumentParameterInfo(*op).hasOption( SILParameterInfo::Sending)) { + // Before we do anything, lets see if we are passing a sendable closure + // literal. If we do, we want to emit a special error that states which + // captured value caused the actual error. + if (initForSendingPartialApply(fas, op)) + return true; // See if we can infer a name from the value. SmallString<64> resultingName; @@ -1724,7 +1961,8 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() { llvm::dbgs() << "Emitting transfer non transferrable diagnostics.\n"); for (auto info : transferredNonTransferrableInfoList) { - TransferNonTransferrableDiagnosticInferrer diagnosticInferrer(info); + TransferNonTransferrableDiagnosticInferrer diagnosticInferrer( + regionInfo->getValueMap(), info); diagnosticInferrer.run(); } } diff --git a/test/Concurrency/concurrent_value_checking.swift b/test/Concurrency/concurrent_value_checking.swift index 4c8b0c281a53c..37602571a580d 100644 --- a/test/Concurrency/concurrent_value_checking.swift +++ b/test/Concurrency/concurrent_value_checking.swift @@ -413,20 +413,20 @@ extension NotConcurrent { func f() { } func test() { - Task { // expected-tns-warning {{task-isolated value of type '() async -> ()' passed as a strongly transferred parameter}} - f() + Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + f() // expected-tns-note {{closure captures 'self' which is accessible to code in the current task}} } - Task { // expected-tns-warning {{task-isolated value of type '() async -> ()' passed as a strongly transferred parameter}} - self.f() + Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + self.f() // expected-tns-note {{closure captures 'self' which is accessible to code in the current task}} } - Task { [self] in // expected-tns-warning {{task-isolated value of type '() async -> ()' passed as a strongly transferred parameter}} - f() + Task { [self] in // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + f() // expected-tns-note {{closure captures 'self' which is accessible to code in the current task}} } - Task { [self] in // expected-tns-warning {{task-isolated value of type '() async -> ()' passed as a strongly transferred parameter}} - self.f() + Task { [self] in // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + self.f() // expected-tns-note {{closure captures 'self' which is accessible to code in the current task}} } } } diff --git a/test/Concurrency/isolated_parameters.swift b/test/Concurrency/isolated_parameters.swift index 799235a2ec8af..9fb0cd4313677 100644 --- a/test/Concurrency/isolated_parameters.swift +++ b/test/Concurrency/isolated_parameters.swift @@ -501,9 +501,9 @@ func testNonSendableCaptures(ns: NotSendable, a: isolated MyActor) { // FIXME: The `a` in the capture list and `isolated a` are the same, // but the actor isolation checker doesn't know that. - Task { [a] in // expected-tns-warning {{'a'-isolated value of type '() async -> ()' passed as a strongly transferred parameter}} + Task { [a] in // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between 'a'-isolated code and concurrent execution of the closure}} _ = a - _ = ns + _ = ns // expected-tns-note {{closure captures 'a'-isolated 'ns'}} } } diff --git a/test/Concurrency/sendable_preconcurrency.swift b/test/Concurrency/sendable_preconcurrency.swift index 9ecb0e7a27939..db8668cbd26dd 100644 --- a/test/Concurrency/sendable_preconcurrency.swift +++ b/test/Concurrency/sendable_preconcurrency.swift @@ -31,10 +31,13 @@ struct MyType3 { } func testA(ns: NS, mt: MyType, mt2: MyType2, mt3: MyType3, sc: StrictClass, nsc: NonStrictClass) async { - // This is task isolated since we are capturing function arguments. - Task { // expected-tns-warning {{task-isolated value of type '() async -> ()' passed as a strongly transferred parameter}} + // This is task isolated since we are capturing function arguments... but + // since we are merging NonStrictClass from a preconcurrency module, the whole + // error is squelched since we allow for preconcurrency to apply to the entire + // capture list. + Task { print(ns) - print(mt) // no warning: MyType is Sendable because we suppressed NonStrictClass's warning + print(mt) print(mt2) print(mt3) print(sc) diff --git a/test/Concurrency/sendable_without_preconcurrency.swift b/test/Concurrency/sendable_without_preconcurrency.swift index d3c34faed0b41..3c26a58919a8c 100644 --- a/test/Concurrency/sendable_without_preconcurrency.swift +++ b/test/Concurrency/sendable_without_preconcurrency.swift @@ -26,11 +26,11 @@ struct MyType2 { } func testA(ns: NS, mt: MyType, mt2: MyType2, sc: StrictClass, nsc: NonStrictClass) async { - Task { // expected-tns-warning {{task-isolated value of type '() async -> ()' passed as a strongly transferred parameter; later accesses could race}} - print(ns) - print(mt) // no warning by default: MyType is Sendable because we suppressed NonStrictClass's warning - print(mt2) - print(sc) + Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(ns) // expected-tns-note {{closure captures non-Sendable 'ns'}} + print(mt) // expected-tns-note {{closure captures non-Sendable 'mt'}} + print(mt2) // expected-tns-note {{closure captures non-Sendable 'mt2'}} + print(sc) // expected-tns-note {{closure captures non-Sendable 'sc'}} } } diff --git a/test/Concurrency/sendable_without_preconcurrency_2.swift b/test/Concurrency/sendable_without_preconcurrency_2.swift index ca6770aa20615..3126e1c70f3ed 100644 --- a/test/Concurrency/sendable_without_preconcurrency_2.swift +++ b/test/Concurrency/sendable_without_preconcurrency_2.swift @@ -29,12 +29,12 @@ struct MyType2: Sendable { } func testA(ns: NS, mt: MyType, mt2: MyType2, sc: StrictClass, nsc: NonStrictClass) async { - Task { // expected-tns-warning {{task-isolated value of type '() async -> ()' passed as a strongly transferred parameter}} + Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} print(ns) print(mt) // no warning with targeted: MyType is Sendable because we suppressed NonStrictClass's warning print(mt2) print(sc) - print(nsc) + print(nsc) // expected-tns-note {{closure captures 'nsc' which is accessible to code in the current task}} } } diff --git a/test/Concurrency/transfernonsendable_global_actor_sending.swift b/test/Concurrency/transfernonsendable_global_actor_sending.swift index 6fc9edf0701ac..6654f0cc5c025 100644 --- a/test/Concurrency/transfernonsendable_global_actor_sending.swift +++ b/test/Concurrency/transfernonsendable_global_actor_sending.swift @@ -14,6 +14,11 @@ extension Task where Failure == Never { public static func fakeInit( @_implicitSelfCapture operation: sending @escaping () async -> Success ) {} + + // This matches the current impl + public static func fakeInit2( + @_implicitSelfCapture @_inheritActorContext operation: sending @escaping @isolated(any) () async -> Success + ) {} } func useValue(_ t: T) {} @@ -25,8 +30,19 @@ func useValue(_ t: T) {} @MainActor func testGlobalFakeInit() { let ns = NonSendableKlass() - // Will be resolved once @MainActor is @Sendable - Task.fakeInit { @MainActor in // expected-error {{main actor-isolated value of type '@MainActor @Sendable () async -> ()' passed as a strongly transferred parameter}} + // Will be resolved once @MainActor is @Sendable. + Task.fakeInit { @MainActor in // expected-error {{passing closure as a 'sending' parameter risks causing data races between main actor-isolated code and concurrent execution of the closure}} + print(ns) // expected-note {{closure captures 'ns' which is accessible to main actor-isolated code}} + } + + useValue(ns) +} + +@MainActor func testGlobalFakeInit2() { + let ns = NonSendableKlass() + + // We shouldn't error here. + Task.fakeInit2 { @MainActor in print(ns) } diff --git a/test/Concurrency/transfernonsendable_preconcurrency_sending.swift b/test/Concurrency/transfernonsendable_preconcurrency_sending.swift index 93451181b1e05..ebf65063061f4 100644 --- a/test/Concurrency/transfernonsendable_preconcurrency_sending.swift +++ b/test/Concurrency/transfernonsendable_preconcurrency_sending.swift @@ -46,6 +46,10 @@ func useValueAsync(_ t: T) async {} func transferArg(_ t: sending T) {} +actor MyActor {} +func takeClosure(_ x: sending () -> ()) {} +func takeClosureAndParam(_ x: T, _ y: sending () -> ()) {} + //////////////////////////////////// // MARK: Use After Transfer Tests // //////////////////////////////////// @@ -149,4 +153,188 @@ func testNeverTransferInexactMatchExplicit(_ x: (PreCUncheckedExplicitlyNonSenda // expected-swift-6-note @-4 {{task-isolated 'x' is passed as a 'sending' parameter; Uses in callee may race with later task-isolated uses}} } +//////////////////////////////////////// +// MARK: Never Sendable Closure Tests // +//////////////////////////////////////// + +// In this case, if anything captures PreCUncheckedNonSendableKlass directly, we +// squelch the error due to preconcurrency. If we do not capture directly, we +// emit the error... unfortunately. +func taskIsolatedCaptureInSendingClosureLiteral(_ x: PreCUncheckedNonSendableKlass) { + Task { + print(x) + } + + takeClosure { + print(x) + } + + takeClosureAndParam(PreCUncheckedNonSendableKlass()) { + print(x) + } + + // This isn't an exact match so we emit the error. + let y = (x, x) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to code in the current task}} + } + + let z = (x, y) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y, z) // expected-note @:11 {{closure captures non-Sendable 'y'}} + // expected-note @-1:14 {{closure captures non-Sendable 'z'}} + } +} + +extension MyActor { + func actorIsolatedCaptureInSendingClosureLiteral(_ x: PreCUncheckedNonSendableKlass) { + Task { + print(x) + } + + takeClosure { + print(x) + } + + takeClosureAndParam(PreCUncheckedNonSendableKlass()) { + print(x) + } + + let y = (x, x) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to 'self'-isolated code}} + } + + let z = (x, y) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y, z) // expected-note @:13 {{closure captures non-Sendable 'y'}} + // expected-note @-1:16 {{closure captures non-Sendable 'z'}} + } + } +} + +// Since this is unchecked by marking as explicitly non-Sendable, we respect +// what the importing user wanted and change swift 6 to warn and swift 5 to +// warn. +func taskIsolatedCaptureInSendingClosureLiteral(_ x: PreCUncheckedExplicitlyNonSendableKlass) { + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + takeClosure { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + takeClosureAndParam(PreCUncheckedExplicitlyNonSendableKlass()) { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + let y = (x, x) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to code in the current task}} + } + + let z = (x, y) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y, z) // expected-note @:11 {{closure captures non-Sendable 'y'}} + // expected-note @-1:14 {{closure captures non-Sendable 'z'}} + } +} + +extension MyActor { + func actorIsolatedCaptureInSendingClosureLiteral(_ x: PreCUncheckedExplicitlyNonSendableKlass) { + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + takeClosure { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + takeClosureAndParam(PreCUncheckedExplicitlyNonSendableKlass()) { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + let y = (x, x) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to 'self'-isolated code}} + } + + let z = (x, y) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y, z) // expected-note @:13 {{closure captures non-Sendable 'y'}} + // expected-note @-1:16 {{closure captures non-Sendable 'z'}} + } + } +} + +// Since this is a swift 6 class, we emit the full error. +func taskIsolatedCaptureInSendingClosureLiteral(_ x: PostCUncheckedNonSendableKlass) { + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + takeClosure { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + takeClosureAndParam(PostCUncheckedNonSendableKlass()) { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + let y = (x, x) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to code in the current task}} + } + + let z = (x, y) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y, z) // expected-note @:11 {{closure captures non-Sendable 'y'}} + // expected-note @-1:14 {{closure captures non-Sendable 'z'}} + } +} + +extension MyActor { + func actorIsolatedCaptureInSendingClosureLiteral(_ x: PostCUncheckedNonSendableKlass) { + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + takeClosure { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + takeClosureAndParam(PostCUncheckedNonSendableKlass()) { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + let y = (x, x) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to 'self'-isolated code}} + } + + let z = (x, y) + Task { // expected-swift-6-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + // expected-swift-5-warning @-1 {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y, z) // expected-note @:13 {{closure captures non-Sendable 'y'}} + // expected-note @-1:16 {{closure captures non-Sendable 'z'}} + } + } +} diff --git a/test/Concurrency/transfernonsendable_sending_params.swift b/test/Concurrency/transfernonsendable_sending_params.swift index c09ca174d81fe..80ee66a2f4024 100644 --- a/test/Concurrency/transfernonsendable_sending_params.swift +++ b/test/Concurrency/transfernonsendable_sending_params.swift @@ -42,9 +42,9 @@ actor Custom { @globalActor struct CustomActor { - static var shared: Custom { - return Custom() - } + static var shared: Custom { + return Custom() + } } @MainActor func transferToMain(_ t: T) {} @@ -67,6 +67,9 @@ func twoTransferArg(_ x: sending NonSendableKlass, _ y: sending NonSendableKlass struct MyError : Error {} +func takeClosure(_ x: sending () -> ()) {} +func takeClosureAndParam(_ x: NonSendableKlass, _ y: sending () -> ()) {} + ///////////////// // MARK: Tests // ///////////////// @@ -503,3 +506,55 @@ func testWrongIsolationGlobalIsolation(_ x: inout sending NonSendableKlass) { x = globalKlass } // expected-warning {{'inout sending' parameter 'x' cannot be main actor-isolated at end of function}} // expected-note @-1 {{main actor-isolated 'x' risks causing races in between main actor-isolated uses and caller uses since caller assumes value is not actor isolated}} + +func taskIsolatedCaptureInSendingClosureLiteral(_ x: NonSendableKlass) { + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + takeClosure { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + takeClosureAndParam(NonSendableKlass()) { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} + } + + let y = (x, x) + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to code in the current task}} + } + + let z = (x, y) + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + print(y, z) // expected-note @:11 {{closure captures non-Sendable 'y'}} + // expected-note @-1:14 {{closure captures non-Sendable 'z'}} + } +} + +extension MyActor { + func actorIsolatedCaptureInSendingClosureLiteral(_ x: NonSendableKlass) { + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + takeClosure { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + takeClosureAndParam(NonSendableKlass()) { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(x) // expected-note {{closure captures 'self'-isolated 'x'}} + } + + let y = (x, x) + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y) // expected-note {{closure captures 'y' which is accessible to 'self'-isolated code}} + } + + let z = (x, y) + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}} + print(y, z) // expected-note @:13 {{closure captures non-Sendable 'y'}} + // expected-note @-1:16 {{closure captures non-Sendable 'z'}} + } + } +}