Skip to content

Commit 4dcee71

Browse files
committed
[send-non-sendable] Warn if a non-final field of an actor is transferred.
rdar://115132118
1 parent 0579b35 commit 4dcee71

File tree

2 files changed

+40
-12
lines changed

2 files changed

+40
-12
lines changed

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -462,10 +462,15 @@ class PartitionOpTranslator {
462462
// Get the vector of IDs that cannot be legally consumed at any point in
463463
// this function. Since we place all args and self in a single region right
464464
// now, it is only necessary to choose a single representative of the set.
465-
ArrayRef<TrackableValueID> getNeverConsumedValues() {
465+
ArrayRef<TrackableValueID> getNeverConsumedValues() const {
466466
return llvm::makeArrayRef(neverConsumedValueIDs);
467467
}
468468

469+
void sortUniqueNeverConsumedValues() {
470+
// TODO: Make a FrozenSetVector.
471+
sortUnique(neverConsumedValueIDs);
472+
}
473+
469474
// get the results of an apply instruction. This is the single result value
470475
// for most apply instructions, but for try apply it is the two arguments
471476
// to each succ block
@@ -488,8 +493,9 @@ class PartitionOpTranslator {
488493
// resulting region to all non-sendable targets, or assign non-sendable
489494
// targets to a fresh region if there are no non-sendable sources
490495
template <typename TargetRange, typename SourceRange>
491-
std::vector<PartitionOp> translateSILMultiAssign(const TargetRange &tgts,
492-
const SourceRange &srcs) {
496+
std::vector<PartitionOp>
497+
translateSILMultiAssign(const TargetRange &tgts, const SourceRange &srcs,
498+
bool resultsActorIsolated = false) {
493499

494500
std::vector<SILValue> nonSendableSrcs;
495501
std::vector<SILValue> nonSendableTgts;
@@ -521,17 +527,28 @@ class PartitionOpTranslator {
521527
if (nonSendableTgts.empty()) return translated;
522528

523529
if (nonSendableSrcs.empty()) {
524-
// if no non-sendable srcs, non-sendable tgts get a fresh region
530+
// If no non-sendable srcs, non-sendable tgts get a fresh region.
525531
add_to_translation(AssignFresh(nonSendableTgts.front()));
526532
} else {
527533
add_to_translation(Assign(nonSendableTgts.front(),
528534
nonSendableSrcs.front()));
529535
}
530536

531-
// assign all targets to the target region
537+
// If our results are actor isolated (and thus cannot be consumed)... add
538+
// them to the neverConsumedValueID array.
539+
if (resultsActorIsolated) {
540+
neverConsumedValueIDs.push_back(lookupValueID(nonSendableTgts.front()));
541+
}
542+
543+
// Assign all targets to the target region.
532544
for (unsigned i = 1; i < nonSendableTgts.size(); i++) {
533545
add_to_translation(Assign(nonSendableTgts.at(i),
534546
nonSendableTgts.front()));
547+
548+
// If our results are actor isolated (and thus cannot be consumed)... add
549+
// them to the neverConsumedValueID array.
550+
if (resultsActorIsolated)
551+
neverConsumedValueIDs.push_back(lookupValueID(nonSendableTgts.at(i)));
535552
}
536553

537554
return translated;
@@ -540,9 +557,20 @@ class PartitionOpTranslator {
540557
std::vector<PartitionOp> translateSILApply(SILInstruction *applyInst) {
541558
// if this apply does not cross isolation domains, it has normal,
542559
// non-consuming multi-assignment semantics
543-
if (!SILApplyCrossesIsolation(applyInst))
560+
if (!SILApplyCrossesIsolation(applyInst)) {
561+
// TODO: How do we handle partial_apply here.
562+
bool hasActorSelf = false;
563+
if (auto fas = FullApplySite::isa(applyInst)) {
564+
if (fas.hasSelfArgument()) {
565+
if (auto self = fas.getSelfArgument()) {
566+
hasActorSelf = self->getType().getASTType()->isActorType();
567+
}
568+
}
569+
}
544570
return translateSILMultiAssign(getApplyResults(applyInst),
545-
applyInst->getOperandValues());
571+
applyInst->getOperandValues(),
572+
hasActorSelf);
573+
}
546574

547575
if (auto cast = dyn_cast<ApplyInst>(applyInst))
548576
return translateIsolationCrossingSILApply(cast);
@@ -1609,6 +1637,9 @@ class PartitionAnalysis {
16091637
}
16101638
}
16111639
}
1640+
1641+
// Now that we have finished processing, sort/unique our non consumed array.
1642+
translator.sortUniqueNeverConsumedValues();
16121643
}
16131644

16141645
// track the AST exprs that have already had diagnostics emitted about

test/Concurrency/sendnonsendable_basic.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,8 @@ func useValue<T>(_ x: T) {}
3333
////////////////////////////
3434

3535
extension Actor {
36-
func noWarningIfCallingGetter() async {
37-
// We should emit an error here since self is an actor which is non
38-
// sendable, so we could be accessing isolated information from a part of
39-
// the actor.
40-
await self.klass.asyncCall()
36+
func warningIfCallingGetter() async {
37+
await self.klass.asyncCall() // expected-sns-warning {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
4138
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' outside of actor-isolated context may introduce data races}}
4239
}
4340

0 commit comments

Comments
 (0)