Skip to content

Commit 2f353b8

Browse files
committed
[region-isolation] Only make partial_apply actor derived if we capture an isolated parameter... not just any actor.
Before I couldn't do this since, @sil_isolated was not represented on partial_applies. Since in the previous commit, I added support to the compiler to represent this, I can now limit this query so now one can pass an actor instance outside of its method to a nonisolated non-Sendable partial apply. Since it is Sendable, it is always safe to do this since we are passing the actor. rdar://123881277
1 parent 2e4fad3 commit 2f353b8

File tree

3 files changed

+18
-24
lines changed

3 files changed

+18
-24
lines changed

include/swift/SIL/ApplySite.h

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,16 @@ class ApplySite {
571571
return getApplyOptions().contains(ApplyFlags::DoesNotAwait);
572572
}
573573

574+
/// Return the SILParameterInfo for this operand in the callee function.
575+
SILParameterInfo getArgumentParameterInfo(const Operand &oper) const {
576+
assert(!getArgumentConvention(oper).isIndirectOutParameter() &&
577+
"Can only be applied to non-out parameters");
578+
579+
// The ParameterInfo is going to be the parameter in the caller.
580+
unsigned calleeArgIndex = getCalleeArgIndex(oper);
581+
return getSubstCalleeConv().getParamInfoForSILArg(calleeArgIndex);
582+
}
583+
574584
static ApplySite getFromOpaqueValue(void *p) { return ApplySite(p); }
575585

576586
friend bool operator==(ApplySite lhs, ApplySite rhs) {
@@ -801,15 +811,6 @@ class FullApplySite : public ApplySite {
801811
}
802812
}
803813

804-
SILParameterInfo getArgumentParameterInfo(const Operand &oper) const {
805-
assert(!getArgumentConvention(oper).isIndirectOutParameter() &&
806-
"Can only be applied to non-out parameters");
807-
808-
// The ParameterInfo is going to be the parameter in the caller.
809-
unsigned calleeArgIndex = getCalleeArgIndex(oper);
810-
return getSubstCalleeConv().getParamInfoForSILArg(calleeArgIndex);
811-
}
812-
813814
static FullApplySite getFromOpaqueValue(void *p) { return FullApplySite(p); }
814815

815816
static bool classof(const SILInstruction *inst) {

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,18 +1693,19 @@ class PartitionOpTranslator {
16931693
}
16941694

16951695
SILMultiAssignOptions options;
1696-
for (auto arg : pai->getOperandValues()) {
1697-
if (auto value = tryToTrackValue(arg)) {
1696+
for (auto &op : pai->getAllOperands()) {
1697+
if (auto value = tryToTrackValue(op.get())) {
16981698
if (value->isActorDerived()) {
16991699
options |= SILMultiAssignFlags::PropagatesActorSelf;
17001700
}
17011701
} else {
1702-
// NOTE: One may think that only sendable things can enter
1703-
// here... but we treat things like function_ref/class_method which
1704-
// are non-Sendable as sendable for our purposes.
1705-
if (arg->getType().isActor()) {
1702+
// We only treat Sendable values as propagating actor self if the
1703+
// partial apply has operand as an sil_isolated parameter.
1704+
ApplySite applySite(pai);
1705+
if (applySite.isArgumentOperand(op) &&
1706+
ApplySite(pai).getArgumentParameterInfo(op).hasOption(
1707+
SILParameterInfo::Isolated))
17061708
options |= SILMultiAssignFlags::PropagatesActorSelf;
1707-
}
17081709
}
17091710
}
17101711

test/Concurrency/transfernonsendable.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,10 @@ func transferNonIsolatedNonAsyncClosureTwice() async {
195195
var actorCaptureClosure = { print(a) }
196196
await transferToMain(actorCaptureClosure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
197197
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
198-
// expected-tns-warning @-2 {{transferring 'actorCaptureClosure' may cause a race}}
199-
// expected-tns-note @-3 {{transferring nonisolated 'actorCaptureClosure' to main actor-isolated callee could cause races between main actor-isolated and nonisolated uses}}
200198

201199
actorCaptureClosure = { print(a) }
202200
await transferToMain(actorCaptureClosure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
203201
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
204-
// expected-tns-warning @-2 {{transferring 'actorCaptureClosure' may cause a race}}
205-
// expected-tns-note @-3 {{transferring nonisolated 'actorCaptureClosure' to main actor-isolated callee could cause races between main actor-isolated and nonisolated uses}}
206202
}
207203

208204
extension Actor {
@@ -403,8 +399,6 @@ func testSimpleLetClosureCaptureActor() async {
403399
let closure = { print(a) }
404400
await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
405401
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
406-
// expected-tns-warning @-2 {{transferring 'closure' may cause a race}}
407-
// expected-tns-note @-3 {{transferring nonisolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and nonisolated uses}}
408402
}
409403

410404
#if TYPECHECKER_ONLY
@@ -441,8 +435,6 @@ func testSimpleVarClosureCaptureActor() async {
441435
closure = { print(a) }
442436
await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
443437
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
444-
// expected-tns-warning @-2 {{transferring 'closure' may cause a race}}
445-
// expected-tns-note @-3 {{transferring nonisolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and nonisolated uses}}
446438
}
447439

448440
#if TYPECHECKER_ONLY

0 commit comments

Comments
 (0)