Skip to content

Commit 2ee1242

Browse files
committed
[region-isolation] Give a proper named error for passing a never transferring values as a transferring argument
This eliminates a bunch of "task or actor isolated value transferred". I also deleted some dead code.
1 parent 4eeb8ce commit 2ee1242

File tree

4 files changed

+70
-34
lines changed

4 files changed

+70
-34
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -968,34 +968,21 @@ NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,
968968
ERROR(regionbasedisolation_named_transfer_yields_race, none,
969969
"transferring %0 may cause a race",
970970
(Identifier))
971-
ERROR(regionbasedisolation_stronglytransfer_assignment_yields_race_name, none,
972-
"assigning %0 to transferring parameter %1 may cause a race",
973-
(Identifier, Identifier))
974-
NOTE(regionbasedisolation_named_transfer_into_transferring_param, none,
975-
"%0 %1 is passed as a transferring parameter; Uses in callee may race with later %0 uses",
976-
(StringRef, Identifier))
977971

978972
NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
979973
"%0 is transferred from %1 caller to %2 callee. Later uses in caller could race with potential uses in callee",
980974
(Identifier, ActorIsolation, ActorIsolation))
981-
NOTE(regionbasedisolation_transfer_non_transferrable_named_note, none,
975+
NOTE(regionbasedisolation_named_transfer_non_transferrable, none,
982976
"transferring %1 %0 to %2 callee could cause races between %2 and %1 uses",
983977
(Identifier, ActorIsolation, ActorIsolation))
984-
NOTE(regionbasedisolation_named_info_isolated_capture, none,
985-
"%1 value %0 is captured by %2 closure. Later local uses could race",
986-
(Identifier, ActorIsolation, ActorIsolation))
987-
NOTE(regionbasedisolation_named_arg_info, none,
988-
"Transferring task-isolated function argument %0 could yield races with caller uses",
989-
(Identifier))
990-
NOTE(regionbasedisolation_named_stronglytransferred_binding, none,
991-
"Cannot access a transferring parameter after the parameter has been transferred", ())
992-
NOTE(regionbasedisolation_note_arg_passed_to_strongly_transferred_param, none,
993-
"%0 value of type %1 passed as a strongly transferred parameter; later accesses could race",
994-
(StringRef, Identifier, Type))
995-
ERROR(regionbasedisolation_named_arg_passed_to_strongly_transferred_param, none,
996-
"%0 %1 passed as a strongly transferred parameter; Uses in callee could race with later %0 uses",
997-
(StringRef, Identifier))
978+
NOTE(regionbasedisolation_named_transfer_into_transferring_param, none,
979+
"%0 %1 is passed as a transferring parameter; Uses in callee may race with later %0 uses",
980+
(StringRef, Identifier))
981+
NOTE(regionbasedisolation_named_notransfer_transfer_into_result, none,
982+
"%0 %1 cannot be a transferring result. %0 uses may race with caller uses",
983+
(StringRef, Identifier))
998984

985+
// Misc Error.
999986
ERROR(regionbasedisolation_task_or_actor_isolated_transferred, none,
1000987
"task or actor isolated value cannot be transferred", ())
1001988

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -985,15 +985,14 @@ class TransferNonTransferrableDiagnosticEmitter {
985985
void emitNamedIsolation(SILLocation loc, Identifier name,
986986
ApplyIsolationCrossing isolationCrossing) {
987987
emitNamedOnlyError(loc, name);
988-
diagnoseNote(
989-
loc, diag::regionbasedisolation_transfer_non_transferrable_named_note,
990-
name, isolationCrossing.getCallerIsolation(),
991-
isolationCrossing.getCalleeIsolation());
988+
diagnoseNote(loc,
989+
diag::regionbasedisolation_named_transfer_non_transferrable,
990+
name, isolationCrossing.getCallerIsolation(),
991+
isolationCrossing.getCalleeIsolation());
992992
}
993993

994-
void emitNamedFunctionArgumentApplyStronglyTransferred(
995-
SILLocation loc, Identifier varName,
996-
ValueIsolationRegionInfo isolationRegionInfo) {
994+
void emitNamedFunctionArgumentApplyStronglyTransferred(SILLocation loc,
995+
Identifier varName) {
997996
emitNamedOnlyError(loc, varName);
998997
SmallString<64> descriptiveKindStr;
999998
{
@@ -1005,6 +1004,18 @@ class TransferNonTransferrableDiagnosticEmitter {
10051004
diagnoseNote(loc, diag, descriptiveKindStr, varName);
10061005
}
10071006

1007+
void emitNamedTransferringReturn(SILLocation loc, Identifier varName) {
1008+
emitNamedOnlyError(loc, varName);
1009+
SmallString<64> descriptiveKindStr;
1010+
{
1011+
llvm::raw_svector_ostream os(descriptiveKindStr);
1012+
getIsolationRegionInfo().printForDiagnostics(os);
1013+
}
1014+
auto diag =
1015+
diag::regionbasedisolation_named_notransfer_transfer_into_result;
1016+
diagnoseNote(loc, diag, descriptiveKindStr, varName);
1017+
}
1018+
10081019
private:
10091020
ASTContext &getASTContext() const {
10101021
return getOperand()->getFunction()->getASTContext();
@@ -1106,7 +1117,7 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
11061117
SmallString<64> resultingName;
11071118
if (auto varName = inferNameFromValue(op->get())) {
11081119
diagnosticEmitter.emitNamedFunctionArgumentApplyStronglyTransferred(
1109-
loc, *varName, diagnosticEmitter.getIsolationRegionInfo());
1120+
loc, *varName);
11101121
return true;
11111122
}
11121123

@@ -1171,6 +1182,33 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
11711182
}
11721183
}
11731184

1185+
if (auto *ri = dyn_cast<ReturnInst>(op->getUser())) {
1186+
auto fType = ri->getFunction()->getLoweredFunctionType();
1187+
if (fType->getNumResults() &&
1188+
fType->getResults()[0].hasOption(SILResultInfo::IsTransferring)) {
1189+
assert(llvm::all_of(fType->getResults(),
1190+
[](SILResultInfo resultInfo) {
1191+
return resultInfo.hasOption(
1192+
SILResultInfo::IsTransferring);
1193+
}) &&
1194+
"All result info must be the same... if that changes... update "
1195+
"this code!");
1196+
SmallString<64> resultingName;
1197+
if (auto name = inferNameFromValue(op->get())) {
1198+
diagnosticEmitter.emitNamedTransferringReturn(loc, *name);
1199+
return true;
1200+
}
1201+
} else {
1202+
assert(llvm::none_of(fType->getResults(),
1203+
[](SILResultInfo resultInfo) {
1204+
return resultInfo.hasOption(
1205+
SILResultInfo::IsTransferring);
1206+
}) &&
1207+
"All result info must be the same... if that changes... update "
1208+
"this code!");
1209+
}
1210+
}
1211+
11741212
diagnosticEmitter.emitUnknownUse(loc);
11751213
return true;
11761214
}

test/Concurrency/transfernonsendable.sil

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,23 +222,26 @@ bb0(%0 : @guaranteed $NonSendableStruct):
222222
debug_value %0 : $NonSendableStruct, var, name "myname"
223223
%2 = struct_extract %0 : $NonSendableStruct, #NonSendableStruct.ns
224224
%3 = copy_value %2 : $NonSendableKlass
225-
return %3 : $NonSendableKlass // expected-warning {{task or actor isolated value cannot be transferred}}
225+
return %3 : $NonSendableKlass // expected-warning {{transferring 'myname.ns' may cause a race}}
226+
// expected-note @-1 {{task-isolated 'myname.ns' cannot be a transferring result. task-isolated uses may race with caller uses}}
226227
}
227228

228229
sil [ossa] @synchronous_returns_transferring_globalactor_struct_structextract : $@convention(method) (@guaranteed MainActorIsolatedStruct) -> @sil_transferring @owned NonSendableKlass {
229230
bb0(%0 : @guaranteed $MainActorIsolatedStruct):
230231
debug_value %0 : $MainActorIsolatedStruct, var, name "myname"
231232
%2 = struct_extract %0 : $MainActorIsolatedStruct, #MainActorIsolatedStruct.ns
232233
%3 = copy_value %2 : $NonSendableKlass
233-
return %3 : $NonSendableKlass // expected-warning {{task or actor isolated value cannot be transferred}}
234+
return %3 : $NonSendableKlass // expected-warning {{transferring 'myname.ns' may cause a race}}
235+
// expected-note @-1 {{main actor-isolated 'myname.ns' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
234236
}
235237

236238
sil [ossa] @synchronous_returns_transferring_globalactor_struct_structelementaddr : $@convention(method) (@in_guaranteed MainActorIsolatedStruct) -> @sil_transferring @owned NonSendableKlass {
237239
bb0(%0 : $*MainActorIsolatedStruct):
238240
debug_value %0 : $*MainActorIsolatedStruct, var, name "myname"
239241
%2 = struct_element_addr %0 : $*MainActorIsolatedStruct, #MainActorIsolatedStruct.ns
240242
%3 = load [copy] %2 : $*NonSendableKlass
241-
return %3 : $NonSendableKlass // expected-warning {{task or actor isolated value cannot be transferred}}
243+
return %3 : $NonSendableKlass // expected-warning {{transferring 'myname.ns' may cause a race}}
244+
// expected-note @-1 {{main actor-isolated 'myname.ns' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
242245
}
243246

244247
sil [ossa] @synchronous_returns_transferring_globalactor_enum_uncheckedenumdata : $@convention(method) (@guaranteed MainActorIsolatedEnum) -> @sil_transferring @owned NonSendableKlass {

test/Concurrency/transfernonsendable_strong_transferring_results.swift

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,14 @@ func transferInAndOut(_ x: transferring NonSendableKlass) -> transferring NonSen
110110

111111

112112
func transferReturnArg(_ x: NonSendableKlass) -> transferring NonSendableKlass {
113-
return x // expected-warning {{task or actor isolated value cannot be transferred}}
113+
return x // expected-warning {{transferring 'x' may cause a race}}
114+
// expected-note @-1 {{task-isolated 'x' cannot be a transferring result. task-isolated uses may race with caller uses}}
115+
}
116+
117+
// TODO: This will be fixed once I represent @MainActor on func types.
118+
@MainActor func transferReturnArgMainActor(_ x: NonSendableKlass) -> transferring NonSendableKlass {
119+
return x // expected-warning {{transferring 'x' may cause a race}}
120+
// expected-note @-1 {{task-isolated 'x' cannot be a transferring result. task-isolated uses may race with caller uses}}
114121
}
115122

116123
// This is safe since we are returning the whole tuple fresh. In contrast,
@@ -130,7 +137,8 @@ func useTransferredResult() async {
130137

131138
extension MainActorIsolatedStruct {
132139
func testNonSendableErrorReturnWithTransfer() -> transferring NonSendableKlass {
133-
return ns // expected-warning {{task or actor isolated value cannot be transferred}}
140+
return ns // expected-warning {{transferring 'self.ns' may cause a race}}
141+
// expected-note @-1 {{main actor-isolated 'self.ns' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
134142
}
135143
func testNonSendableErrorReturnNoTransfer() -> NonSendableKlass {
136144
return ns

0 commit comments

Comments
 (0)