diff --git a/include/swift/SILOptimizer/Utils/PartitionUtils.h b/include/swift/SILOptimizer/Utils/PartitionUtils.h index 7c33581619216..d971d5d5af974 100644 --- a/include/swift/SILOptimizer/Utils/PartitionUtils.h +++ b/include/swift/SILOptimizer/Utils/PartitionUtils.h @@ -1226,11 +1226,17 @@ struct PartitionOpEvaluator { } // Next see if we are disconnected and have the same isolation. In such a - // case, we do not transfer since the disconnected value is allowed to be - // resued after we return. - if (transferredRegionIsolation.isDisconnected() && calleeIsolationInfo && - transferredRegionIsolation.hasSameIsolation(calleeIsolationInfo)) - return; + // case, if we are not marked explicitly as sending, we do not transfer + // since the disconnected value is allowed to be resued after we + // return. If we are passed as a sending parameter, we cannot do this. + if (auto *sourceInst = Impl::getSourceInst(op)) { + if (auto fas = FullApplySite::isa(sourceInst); + (!fas || !fas.isSending(*op.getSourceOp())) && + transferredRegionIsolation.isDisconnected() && + calleeIsolationInfo && + transferredRegionIsolation.hasSameIsolation(calleeIsolationInfo)) + return; + } // Mark op.getOpArgs()[0] as transferred. TransferringOperandState &state = operandToStateMap.get(op.getSourceOp()); @@ -1578,6 +1584,9 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator { static SILLocation getLoc(SILInstruction *inst) { return inst->getLoc(); } static SILLocation getLoc(Operand *op) { return op->getUser()->getLoc(); } + static SILInstruction *getSourceInst(const PartitionOp &partitionOp) { + return partitionOp.getSourceInst(); + } static SILIsolationInfo getIsolationInfo(const PartitionOp &partitionOp) { return SILIsolationInfo::get(partitionOp.getSourceInst()); } diff --git a/test/Concurrency/transfernonsendable_sending_params.swift b/test/Concurrency/transfernonsendable_sending_params.swift index 4c9d719065a28..d5e295b7ead68 100644 --- a/test/Concurrency/transfernonsendable_sending_params.swift +++ b/test/Concurrency/transfernonsendable_sending_params.swift @@ -7,7 +7,9 @@ // MARK: Declarations // //////////////////////// -class NonSendableKlass {} +class NonSendableKlass { + func use() {} +} struct NonSendableStruct { var first = NonSendableKlass() @@ -55,6 +57,9 @@ func throwingFunction() throws { fatalError() } func transferArg(_ x: sending NonSendableKlass) { } +func transferArgAsync(_ x: sending NonSendableKlass) async { +} + func transferArgWithOtherParam(_ x: sending NonSendableKlass, _ y: NonSendableKlass) { } @@ -559,3 +564,56 @@ extension MyActor { } } } + +// We would normally not error here since transferArg is nonisolated and c is +// disconnected. Since c is passed as sending, we shouldn't squelch this. +func disconnectedPassedSendingToNonIsolatedCallee( +) async -> Void { + let c = NonSendableKlass() + transferArg(c) // expected-warning {{sending 'c' risks causing data races}} + // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}} + c.use() // expected-note {{access can happen concurrently}} +} + +// We would normally not error here since transferArg is nonisolated and c is +// disconnected. Since c is passed as sending, we shouldn't squelch this. +func disconnectedPassedSendingToAsyncNonIsolatedCallee( +) async -> Void { + let c = NonSendableKlass() + await transferArgAsync(c) // expected-warning {{sending 'c' risks causing data races}} + // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}} + c.use() // expected-note {{access can happen concurrently}} +} + +// We would normally not error here since transferArg is nonisolated and c is +// disconnected. Since c is passed as sending, we shouldn't squelch this. +func disconnectedPassedSendingToNonIsolatedCalleeIsolatedParam2( + isolation: isolated (any Actor)? = nil +) async -> Void { + let c = NonSendableKlass() + transferArg(c) // expected-warning {{sending 'c' risks causing data races}} + // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}} + c.use() // expected-note {{access can happen concurrently}} +} + +// We would normally not error here since transferArg is nonisolated and c is +// disconnected. Since c is passed as sending, we shouldn't squelch this. +func disconnectedPassedSendingToAsyncNonIsolatedCalleeIsolatedParam2( + isolation: isolated (any Actor)? = nil +) async -> Void { + let c = NonSendableKlass() + await transferArgAsync(c) // expected-warning {{sending 'c' risks causing data races}} + // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}} + c.use() // expected-note {{access can happen concurrently}} +} + +// We would normally not error here since transferArg is nonisolated and c is +// disconnected. Since c is passed as sending, we shouldn't squelch this. +func disconnectedPassedSendingToNonIsolatedCalleeIsolatedParam3( + isolation: isolated (any Actor)? = nil +) -> Void { + let c = NonSendableKlass() + transferArg(c) // expected-warning {{sending 'c' risks causing data races}} + // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}} + c.use() // expected-note {{access can happen concurrently}} +} diff --git a/unittests/SILOptimizer/PartitionUtilsTest.cpp b/unittests/SILOptimizer/PartitionUtilsTest.cpp index 8e712b3328163..ae89832ee3749 100644 --- a/unittests/SILOptimizer/PartitionUtilsTest.cpp +++ b/unittests/SILOptimizer/PartitionUtilsTest.cpp @@ -62,6 +62,10 @@ struct MockedPartitionOpEvaluator final return {}; } + static SILInstruction *getSourceInst(const PartitionOp &partitionOp) { + return nullptr; + } + static bool doesFunctionHaveSendingResult(const PartitionOp &partitionOp) { return false; } @@ -107,6 +111,10 @@ struct MockedPartitionOpEvaluatorWithFailureCallback final static SILIsolationInfo getIsolationInfo(const PartitionOp &partitionOp) { return {}; } + + static SILInstruction *getSourceInst(const PartitionOp &partitionOp) { + return nullptr; + } }; } // namespace