Skip to content

Commit 75e7a40

Browse files
committed
[region-isolation] Emit the correct error for closures that capture actor self.
rdar://122501400
1 parent da6ffaa commit 75e7a40

File tree

4 files changed

+64
-39
lines changed

4 files changed

+64
-39
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -603,33 +603,6 @@ static bool isTransferrableFunctionArgument(SILFunctionArgument *arg) {
603603
return arg->isTransferring();
604604
}
605605

606-
//===----------------------------------------------------------------------===//
607-
// MARK: SILIsolationInfo
608-
//===----------------------------------------------------------------------===//
609-
610-
void SILIsolationInfo::printForDiagnostics(llvm::raw_ostream &os) const {
611-
switch (Kind(*this)) {
612-
case Unknown:
613-
llvm::report_fatal_error("Printing unknown for diagnostics?!");
614-
return;
615-
case Disconnected:
616-
os << "disconnected";
617-
return;
618-
case Actor:
619-
if (SILValue instance = getActorInstance()) {
620-
if (auto name = VariableNameInferrer::inferName(instance)) {
621-
os << "'" << *name << "'-isolated";
622-
return;
623-
}
624-
}
625-
getActorIsolation().printForDiagnostics(os);
626-
return;
627-
case Task:
628-
os << "task-isolated";
629-
return;
630-
}
631-
}
632-
633606
//===----------------------------------------------------------------------===//
634607
// MARK: TrackableValue
635608
//===----------------------------------------------------------------------===//

lib/SILOptimizer/Utils/PartitionUtils.cpp

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "swift/SILOptimizer/Utils/PartitionUtils.h"
14+
1415
#include "swift/AST/Expr.h"
1516
#include "swift/SIL/ApplySite.h"
1617
#include "swift/SIL/InstructionUtils.h"
1718
#include "swift/SIL/PatternMatch.h"
1819
#include "swift/SIL/SILGlobalVariable.h"
20+
#include "swift/SILOptimizer/Utils/VariableNameUtils.h"
21+
1922
#include "llvm/Support/CommandLine.h"
2023

2124
using namespace swift;
@@ -101,8 +104,19 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
101104
if (auto *pai = dyn_cast<PartialApplyInst>(inst)) {
102105
if (auto *ace = pai->getLoc().getAsASTNode<AbstractClosureExpr>()) {
103106
auto actorIsolation = ace->getActorIsolation();
107+
SILValue actorInstance;
104108
if (actorIsolation.isActorIsolated()) {
105-
return SILIsolationInfo::getActorIsolated(pai, SILValue(),
109+
if (actorIsolation.getKind() == ActorIsolation::ActorInstance) {
110+
ApplySite as(pai);
111+
for (auto &op : as.getArgumentOperands()) {
112+
if (as.getArgumentParameterInfo(op).hasOption(
113+
SILParameterInfo::Isolated)) {
114+
actorInstance = pai->getAllOperands().back().get();
115+
break;
116+
}
117+
}
118+
}
119+
return SILIsolationInfo::getActorIsolated(pai, actorInstance,
106120
actorIsolation);
107121
}
108122
}
@@ -403,6 +417,37 @@ void SILIsolationInfo::Profile(llvm::FoldingSetNodeID &id) const {
403417
}
404418
}
405419

420+
void SILIsolationInfo::printForDiagnostics(llvm::raw_ostream &os) const {
421+
switch (Kind(*this)) {
422+
case Unknown:
423+
llvm::report_fatal_error("Printing unknown for diagnostics?!");
424+
return;
425+
case Disconnected:
426+
os << "disconnected";
427+
return;
428+
case Actor:
429+
if (SILValue instance = getActorInstance()) {
430+
if (auto name = VariableNameInferrer::inferName(instance)) {
431+
os << "'" << *name << "'-isolated";
432+
return;
433+
}
434+
}
435+
436+
if (getActorIsolation().getKind() == ActorIsolation::ActorInstance) {
437+
if (auto *vd = getActorIsolation().getActorInstance()) {
438+
os << "'" << vd->getBaseIdentifier() << "'-isolated";
439+
return;
440+
}
441+
}
442+
443+
getActorIsolation().printForDiagnostics(os);
444+
return;
445+
case Task:
446+
os << "task-isolated";
447+
return;
448+
}
449+
}
450+
406451
//===----------------------------------------------------------------------===//
407452
// MARK: PartitionOp
408453
//===----------------------------------------------------------------------===//

test/Concurrency/transfernonsendable.swift

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ extension Actor {
245245
await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
246246
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
247247
// expected-tns-warning @-2 {{transferring 'closure' may cause a data race}}
248-
// expected-tns-note @-3 {{transferring actor-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
248+
// expected-tns-note @-3 {{transferring 'self'-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
249249
}
250250

251251
func simpleClosureCaptureSelfAndTransferThroughTuple() async {
@@ -255,7 +255,7 @@ extension Actor {
255255
let x = (1, closure)
256256
await transferToMain(x) // expected-complete-warning {{passing argument of non-sendable type '(Int, () -> ())' into main actor-isolated context may introduce data races}}
257257
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
258-
// expected-tns-warning @-2 {{actor-isolated value of type '(Int, () -> ())' transferred to main actor-isolated context}}
258+
// expected-tns-warning @-2 {{'self'-isolated value of type '(Int, () -> ())' transferred to main actor-isolated context}}
259259
}
260260

261261
func simpleClosureCaptureSelfAndTransferThroughTupleBackwards() async {
@@ -264,7 +264,7 @@ extension Actor {
264264
}
265265

266266
let x = (closure, 1)
267-
await transferToMain(x) // expected-tns-warning {{actor-isolated value of type '(() -> (), Int)' transferred to main actor-isolated context}}
267+
await transferToMain(x) // expected-tns-warning {{'self'-isolated value of type '(() -> (), Int)' transferred to main actor-isolated context}}
268268
// expected-complete-warning @-1 {{passing argument of non-sendable type '(() -> (), Int)' into main actor-isolated context may introduce data races}}
269269
// expected-complete-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
270270
}
@@ -276,7 +276,7 @@ extension Actor {
276276
let x: Any? = (1, closure)
277277
await transferToMain(x) // expected-complete-warning {{passing argument of non-sendable type 'Any?' into main actor-isolated context may introduce data races}}
278278
// expected-tns-warning @-1 {{transferring 'x' may cause a data race}}
279-
// expected-tns-note @-2 {{transferring actor-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
279+
// expected-tns-note @-2 {{transferring 'self'-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
280280
}
281281

282282
func simpleClosureCaptureSelfAndTransferThroughOptionalBackwards() async {
@@ -289,7 +289,7 @@ extension Actor {
289289
let x: Any? = (closure, 1)
290290
await transferToMain(x) // expected-complete-warning {{passing argument of non-sendable type 'Any?' into main actor-isolated context may introduce data races}}
291291
// expected-tns-warning @-1 {{transferring 'x' may cause a data race}}
292-
// expected-tns-note @-2 {{transferring actor-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
292+
// expected-tns-note @-2 {{transferring 'self'-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
293293
}
294294

295295
func simpleClosureCaptureSelfWithReinit() async {
@@ -301,7 +301,7 @@ extension Actor {
301301
await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
302302
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
303303
// expected-tns-warning @-2 {{transferring 'closure' may cause a data race}}
304-
// expected-tns-note @-3 {{transferring actor-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
304+
// expected-tns-note @-3 {{transferring 'self'-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
305305

306306
closure = {}
307307

@@ -327,7 +327,7 @@ extension Actor {
327327
await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
328328
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
329329
// expected-tns-warning @-2 {{transferring 'closure' may cause a data race}}
330-
// expected-tns-note @-3 {{transferring actor-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
330+
// expected-tns-note @-3 {{transferring 'self'-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
331331
}
332332

333333
func simpleClosureCaptureSelfWithReinit3() async {
@@ -349,7 +349,7 @@ extension Actor {
349349
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
350350
// expected-tns-note @-2 {{use here could race}}
351351
// expected-tns-warning @-3 {{transferring 'closure' may cause a data race}}
352-
// expected-tns-note @-4 {{transferring actor-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
352+
// expected-tns-note @-4 {{transferring 'self'-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
353353
}
354354

355355
// In this case, we reinit along both paths, but only one has an actor derived
@@ -372,7 +372,7 @@ extension Actor {
372372
await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
373373
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
374374
// expected-tns-warning @-2 {{transferring 'closure' may cause a data race}}
375-
// expected-tns-note @-3 {{transferring actor-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
375+
// expected-tns-note @-3 {{transferring 'self'-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
376376
}
377377

378378
#if TYPECHECKER_ONLY
@@ -539,7 +539,7 @@ extension Actor {
539539
await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
540540
// expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
541541
// expected-tns-warning @-2 {{transferring 'closure' may cause a data race}}
542-
// expected-tns-note @-3 {{transferring actor-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
542+
// expected-tns-note @-3 {{transferring 'self'-isolated 'closure' to main actor-isolated callee could cause races between main actor-isolated and 'self'-isolated uses}}
543543

544544
// This doesnt since we re-assign
545545
closure = {}
@@ -1570,3 +1570,10 @@ func functionArgumentIntoClosure(_ x: @escaping () -> ()) async {
15701570
c = a.klassLet
15711571
}
15721572
}
1573+
1574+
func testGetActorName() async {
1575+
let a = Actor()
1576+
let x = NonSendableKlass()
1577+
await a.useKlass(x)
1578+
useValue(x)
1579+
}

test/Concurrency/transfernonsendable_region_based_sendability.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ actor A_Sendable {
442442
// support the ability to dynamically invoke the synchronous closure on
443443
// the specific actor.
444444
await a.foo(captures_self) // expected-tns-warning {{transferring 'captures_self' may cause a data race}}
445-
// expected-tns-note @-1 {{transferring actor-isolated 'captures_self' to actor-isolated callee could cause races between actor-isolated and actor-isolated uses}}
445+
// expected-tns-note @-1 {{transferring 'self'-isolated 'captures_self' to actor-isolated callee could cause races between actor-isolated and 'self'-isolated uses}}
446446
// expected-complete-warning @-2 {{passing argument of non-sendable type '() -> ()' into actor-isolated context may introduce data races}}
447447
// expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
448448
}

0 commit comments

Comments
 (0)