Skip to content

Commit 5580bb4

Browse files
committed
[region-isolation] Add a named variant of the use after pass via strongly transfering parameter.
Specifically, I added a named version of the diagnostic: func testSimpleTransferLet() { let k = Klass() - transferArg(k) // expected-warning {{binding of non-Sendable type 'Klass' accessed after being transferred; later accesses could race}} + transferArg(k) // expected-warning {{transferring 'k' may cause a race}} + // expected-note @-1 {{'k' used after being passed as a transferring parameter}} useValue(k) // expected-note {{use here could race}} } and I also cleaned up the typed version of the diagnostic that is used if we fail to find a name: func testSimpleTransferLet() { let k = Klass() - transferArg(k) // expected-warning {{binding of non-Sendable type 'Klass' accessed after being transferred; later accesses could race}} - transferArg(k) // expected-warning {{value of non-Sendable type 'Klass' accessed after being transferred; later accesses could race}} useValue(k) // expected-note {{use here could race}} } This is the 2nd to the last part of a larger effort to rework all of the region based diagnostics to first try and use names and only go back to the old typed diagnostics when we fail to look up a name (which should be pretty rare, but is always possible). At some point if I really feel confident enough with the name lookup code, I am most likely just going to get rid of the typed diagnostic code and just emit a compiler doesnt understand error. The user will still not be able to ship the code but would also be told to file a bug so that we can fix the name inference.
1 parent f46e58a commit 5580bb4

File tree

3 files changed

+43
-23
lines changed

3 files changed

+43
-23
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -949,7 +949,7 @@ ERROR(regionbasedisolation_isolated_capture_yields_race, none,
949949
"%1 closure captures value of non-Sendable type %0 from %2 context; later accesses to value could race",
950950
(Type, ActorIsolation, ActorIsolation))
951951
ERROR(regionbasedisolation_transfer_yields_race_stronglytransferred_binding, none,
952-
"binding of non-Sendable type %0 accessed after being transferred; later accesses could race",
952+
"value of non-Sendable type %0 accessed after being transferred; later accesses could race",
953953
(Type))
954954
ERROR(regionbasedisolation_arg_transferred, none,
955955
"%0 value of type %1 transferred to %2 context; later accesses to value could race",
@@ -981,6 +981,9 @@ NOTE(regionbasedisolation_named_transfer_into_transferring_param, none,
981981
NOTE(regionbasedisolation_named_notransfer_transfer_into_result, none,
982982
"%0 %1 cannot be a transferring result. %0 uses may race with caller uses",
983983
(StringRef, Identifier))
984+
NOTE(regionbasedisolation_named_stronglytransferred_binding, none,
985+
"%0 used after being passed as a transferring parameter; Later uses could race",
986+
(Identifier))
984987

985988
// Misc Error.
986989
ERROR(regionbasedisolation_task_or_actor_isolated_transferred, none,

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -493,8 +493,7 @@ class UseAfterTransferDiagnosticEmitter {
493493

494494
void emitNamedIsolationCrossingError(SILLocation loc, Identifier name,
495495
SILIsolationInfo namesIsolationInfo,
496-
ApplyIsolationCrossing isolationCrossing,
497-
SILLocation variableDefinedLoc) {
496+
ApplyIsolationCrossing isolationCrossing) {
498497
// Emit the short error.
499498
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
500499
name)
@@ -523,7 +522,24 @@ class UseAfterTransferDiagnosticEmitter {
523522
emitRequireInstDiagnostics();
524523
}
525524

526-
void emitUseOfStronglyTransferredValue(SILLocation loc, Type inferredType) {
525+
void emitNamedUseOfStronglyTransferredValue(SILLocation loc,
526+
Identifier name) {
527+
// Emit the short error.
528+
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
529+
name)
530+
.highlight(loc.getSourceRange());
531+
532+
// Then emit the note with greater context.
533+
diagnoseNote(
534+
loc, diag::regionbasedisolation_named_stronglytransferred_binding, name)
535+
.highlight(loc.getSourceRange());
536+
537+
// Finally the require points.
538+
emitRequireInstDiagnostics();
539+
}
540+
541+
void emitTypedUseOfStronglyTransferredValue(SILLocation loc,
542+
Type inferredType) {
527543
diagnoseError(
528544
loc,
529545
diag::
@@ -783,7 +799,16 @@ void UseAfterTransferDiagnosticInferrer::infer() {
783799
"We should never transfer an indirect out parameter");
784800
if (fas.getArgumentParameterInfo(*transferOp->getOperand())
785801
.hasOption(SILParameterInfo::Transferring)) {
786-
return diagnosticEmitter.emitUseOfStronglyTransferredValue(
802+
803+
// First try to do the named diagnostic if we can find a name.
804+
if (auto rootValueAndName =
805+
inferNameAndRootFromValue(transferOp->get())) {
806+
return diagnosticEmitter.emitNamedUseOfStronglyTransferredValue(
807+
baseLoc, rootValueAndName->first);
808+
}
809+
810+
// Otherwise, emit the typed diagnostic.
811+
return diagnosticEmitter.emitTypedUseOfStronglyTransferredValue(
787812
baseLoc, baseInferredType);
788813
}
789814
}
@@ -804,20 +829,9 @@ void UseAfterTransferDiagnosticInferrer::infer() {
804829
// Before we do anything further, see if we can find a name and emit a name
805830
// error.
806831
if (auto rootValueAndName = inferNameAndRootFromValue(transferOp->get())) {
807-
if (auto *svi =
808-
dyn_cast<SingleValueInstruction>(rootValueAndName->second)) {
809-
return diagnosticEmitter.emitNamedIsolationCrossingError(
810-
baseLoc, rootValueAndName->first, transferOp->getIsolationInfo(),
811-
*sourceApply->getIsolationCrossing(), svi->getLoc());
812-
}
813-
814-
if (auto *fArg =
815-
dyn_cast<SILFunctionArgument>(rootValueAndName->second)) {
816-
return diagnosticEmitter.emitNamedIsolationCrossingError(
817-
baseLoc, rootValueAndName->first, transferOp->getIsolationInfo(),
818-
*sourceApply->getIsolationCrossing(),
819-
RegularLocation(fArg->getDecl()->getLoc()));
820-
}
832+
return diagnosticEmitter.emitNamedIsolationCrossingError(
833+
baseLoc, rootValueAndName->first, transferOp->getIsolationInfo(),
834+
*sourceApply->getIsolationCrossing());
821835
}
822836

823837
// Otherwise, try to infer from the ApplyExpr.

test/Concurrency/transfernonsendable_strong_transferring_params.swift

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,16 @@ func twoTransferArg(_ x: transferring Klass, _ y: transferring Klass) {}
6868

6969
func testSimpleTransferLet() {
7070
let k = Klass()
71-
transferArg(k) // expected-warning {{binding of non-Sendable type 'Klass' accessed after being transferred; later accesses could race}}
71+
transferArg(k) // expected-warning {{transferring 'k' may cause a race}}
72+
// expected-note @-1 {{'k' used after being passed as a transferring parameter}}
7273
useValue(k) // expected-note {{use here could race}}
7374
}
7475

7576
func testSimpleTransferVar() {
7677
var k = Klass()
7778
k = Klass()
78-
transferArg(k) // expected-warning {{binding of non-Sendable type 'Klass' accessed after being transferred; later accesses could race}}
79+
transferArg(k) // expected-warning {{transferring 'k' may cause a race}}
80+
// expected-note @-1 {{'k' used after being passed as a transferring parameter}}
7981
useValue(k) // expected-note {{use here could race}}
8082
}
8183

@@ -311,8 +313,9 @@ func mergeDoesNotEliminateEarlierTransfer2(_ x: transferring NonSendableStruct)
311313

312314
func doubleArgument() async {
313315
let x = Klass()
314-
twoTransferArg(x, x) // expected-warning {{binding of non-Sendable type 'Klass' accessed after being transferred}}
315-
// expected-note @-1 {{use here could race}}
316+
twoTransferArg(x, x) // expected-warning {{transferring 'x' may cause a race}}
317+
// expected-note @-1 {{'x' used after being passed as a transferring parameter}}
318+
// expected-note @-2 {{use here could race}}
316319
}
317320

318321
func testTransferSrc(_ x: transferring Klass) async {

0 commit comments

Comments
 (0)