Skip to content

Commit 50712a1

Browse files
committed
[region-isolation] Emit a standard short warning + long note when emitting a strongly transferring a structurally isolated value warning.
Specifically, instead of what I call an IsolationRegionInfo + Type error, e.x.: > task-isolated value of type 'NonSendableType' passed as a strongly transferred parameter; later accesses could race we use instead the standard warning of: ```swift func transfer(_ x: transferring Type) {} func test(_ x: Type) { transfer(x) // expected-warning {{transferring 'x' may cause a race}} // expected-note @-1 {{task-isolated 'x' is passed as a transferring parameter; Uses in callee may race with later task-isolated uses}} } ``` Implementation wise, I left in the old type diagnostic as a backup for now. With time, I am going to want to eliminate it completely.
1 parent c0c24c1 commit 50712a1

File tree

4 files changed

+87
-41
lines changed

4 files changed

+87
-41
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -957,8 +957,8 @@ ERROR(regionbasedisolation_arg_transferred, none,
957957
"%0 value of type %1 transferred to %2 context; later accesses to value could race",
958958
(StringRef, Type, ActorIsolation))
959959
ERROR(regionbasedisolation_arg_passed_to_strongly_transferred_param, none,
960-
"task-isolated value of type %0 passed as a strongly transferred parameter; later accesses could race",
961-
(Type))
960+
"%0 value of type %1 passed as a strongly transferred parameter; later accesses could race",
961+
(StringRef, Type))
962962
NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,
963963
"value is %0 since it is in the same region as %1",
964964
(StringRef, DeclBaseName))
@@ -973,9 +973,9 @@ ERROR(regionbasedisolation_named_transfer_yields_race, none,
973973
ERROR(regionbasedisolation_stronglytransfer_assignment_yields_race_name, none,
974974
"assigning %0 to transferring parameter %1 may cause a race",
975975
(Identifier, Identifier))
976-
NOTE(regionbasedisolation_stronglytransfer_taskisolated_assign_note, none,
977-
"%0 is a task-isolated value that is assigned into transferring parameter %1. Transferred uses of %1 may race with caller uses of %0",
978-
(Identifier, Identifier))
976+
NOTE(regionbasedisolation_named_transfer_into_transferring_param, none,
977+
"%0 %1 is passed as a transferring parameter; Uses in callee may race with later %0 uses",
978+
(StringRef, Identifier))
979979

980980
NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
981981
"%0 is transferred from %1 caller to %2 callee. Later uses in caller could race with potential uses in callee",
@@ -991,6 +991,12 @@ NOTE(regionbasedisolation_named_arg_info, none,
991991
(Identifier))
992992
NOTE(regionbasedisolation_named_stronglytransferred_binding, none,
993993
"Cannot access a transferring parameter after the parameter has been transferred", ())
994+
NOTE(regionbasedisolation_note_arg_passed_to_strongly_transferred_param, none,
995+
"%0 value of type %1 passed as a strongly transferred parameter; later accesses could race",
996+
(StringRef, Identifier, Type))
997+
ERROR(regionbasedisolation_named_arg_passed_to_strongly_transferred_param, none,
998+
"%0 %1 passed as a strongly transferred parameter; Uses in callee could race with later %0 uses",
999+
(StringRef, Identifier))
9941000

9951001
// TODO: print the name of the nominal type
9961002
ERROR(deinit_not_visible, none,

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 57 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -917,19 +917,18 @@ class TransferNonTransferrableDiagnosticEmitter {
917917
}
918918

919919
void emitFunctionArgumentApply(SILLocation loc, Type type,
920-
ValueIsolationRegionInfo regionInfo,
921920
ApplyIsolationCrossing crossing) {
922921
SmallString<64> descriptiveKindStr;
923922
{
924923
llvm::raw_svector_ostream os(descriptiveKindStr);
925-
regionInfo.printForDiagnostics(os);
924+
getIsolationRegionInfo().printForDiagnostics(os);
926925
}
927926
diagnoseError(loc, diag::regionbasedisolation_arg_transferred,
928927
StringRef(descriptiveKindStr), type,
929928
crossing.getCalleeIsolation())
930929
.highlight(getOperand()->getUser()->getLoc().getSourceRange());
931930

932-
if (regionInfo.isTaskIsolated()) {
931+
if (getIsolationRegionInfo().isTaskIsolated()) {
933932
auto *fArg =
934933
cast<SILFunctionArgument>(info.nonTransferrable.get<SILValue>());
935934
if (fArg->getDecl()) {
@@ -964,23 +963,46 @@ class TransferNonTransferrableDiagnosticEmitter {
964963

965964
void emitFunctionArgumentApplyStronglyTransferred(SILLocation loc,
966965
Type type) {
967-
diagnoseError(
968-
loc,
969-
diag::regionbasedisolation_arg_passed_to_strongly_transferred_param,
970-
type)
966+
SmallString<64> descriptiveKindStr;
967+
{
968+
llvm::raw_svector_ostream os(descriptiveKindStr);
969+
getIsolationRegionInfo().printForDiagnostics(os);
970+
}
971+
auto diag =
972+
diag::regionbasedisolation_arg_passed_to_strongly_transferred_param;
973+
diagnoseError(loc, diag, descriptiveKindStr, type)
974+
.highlight(getOperand()->getUser()->getLoc().getSourceRange());
975+
}
976+
977+
void emitNamedOnlyError(SILLocation loc, Identifier name) {
978+
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
979+
name)
971980
.highlight(getOperand()->getUser()->getLoc().getSourceRange());
972981
}
973982

974983
void emitNamedIsolation(SILLocation loc, Identifier name,
975984
ApplyIsolationCrossing isolationCrossing) {
976-
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
977-
name);
985+
emitNamedOnlyError(loc, name);
978986
diagnoseNote(
979987
loc, diag::regionbasedisolation_transfer_non_transferrable_named_note,
980988
name, isolationCrossing.getCallerIsolation(),
981989
isolationCrossing.getCalleeIsolation());
982990
}
983991

992+
void emitNamedFunctionArgumentApplyStronglyTransferred(
993+
SILLocation loc, Identifier varName,
994+
ValueIsolationRegionInfo isolationRegionInfo) {
995+
emitNamedOnlyError(loc, varName);
996+
SmallString<64> descriptiveKindStr;
997+
{
998+
llvm::raw_svector_ostream os(descriptiveKindStr);
999+
getIsolationRegionInfo().printForDiagnostics(os);
1000+
}
1001+
auto diag =
1002+
diag::regionbasedisolation_named_transfer_into_transferring_param;
1003+
diagnoseNote(loc, diag, descriptiveKindStr, varName);
1004+
}
1005+
9841006
private:
9851007
ASTContext &getASTContext() const {
9861008
return getOperand()->getFunction()->getASTContext();
@@ -1073,29 +1095,35 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
10731095
auto loc = op->getUser()->getLoc();
10741096

10751097
if (auto *sourceApply = loc.getAsASTNode<ApplyExpr>()) {
1076-
std::optional<ApplyIsolationCrossing> isolation = {};
1098+
// First see if we have a transferring argument.
1099+
if (auto fas = FullApplySite::isa(op->getUser())) {
1100+
if (fas.getArgumentParameterInfo(*op).hasOption(
1101+
SILParameterInfo::Transferring)) {
1102+
1103+
// See if we can infer a name from the value.
1104+
SmallString<64> resultingName;
1105+
if (auto varName = inferNameFromValue(op->get())) {
1106+
diagnosticEmitter.emitNamedFunctionArgumentApplyStronglyTransferred(
1107+
loc, *varName, diagnosticEmitter.getIsolationRegionInfo());
1108+
return true;
1109+
}
1110+
1111+
Type type = op->get()->getType().getASTType();
1112+
if (auto *inferredArgExpr =
1113+
inferArgumentExprFromApplyExpr(sourceApply, fas, op)) {
1114+
type = inferredArgExpr->findOriginalType();
1115+
}
1116+
diagnosticEmitter.emitFunctionArgumentApplyStronglyTransferred(loc,
1117+
type);
1118+
return true;
1119+
}
1120+
}
10771121

10781122
// First try to get the apply from the isolation crossing.
1079-
if (auto value = sourceApply->getIsolationCrossing())
1080-
isolation = value;
1123+
auto isolation = sourceApply->getIsolationCrossing();
10811124

10821125
// If we could not infer an isolation...
10831126
if (!isolation) {
1084-
// First see if we have a transferring argument.
1085-
if (auto fas = FullApplySite::isa(op->getUser())) {
1086-
if (fas.getArgumentParameterInfo(*op).hasOption(
1087-
SILParameterInfo::Transferring)) {
1088-
Type type = op->get()->getType().getASTType();
1089-
if (auto *inferredArgExpr =
1090-
inferArgumentExprFromApplyExpr(sourceApply, fas, op)) {
1091-
type = inferredArgExpr->findOriginalType();
1092-
}
1093-
diagnosticEmitter.emitFunctionArgumentApplyStronglyTransferred(loc,
1094-
type);
1095-
return true;
1096-
}
1097-
}
1098-
10991127
// Otherwise, emit a "we don't know error" that tells the user to file a
11001128
// bug.
11011129
diagnoseError(op->getUser(), diag::regionbasedisolation_unknown_pattern);
@@ -1120,9 +1148,7 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
11201148
}
11211149
}
11221150

1123-
auto isolationRegionInfo = diagnosticEmitter.getIsolationRegionInfo();
1124-
diagnosticEmitter.emitFunctionArgumentApply(loc, type, isolationRegionInfo,
1125-
*isolation);
1151+
diagnosticEmitter.emitFunctionArgumentApply(loc, type, *isolation);
11261152
return true;
11271153
}
11281154

@@ -1137,10 +1163,8 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
11371163
// See if we are in SIL and have an apply site specified isolation.
11381164
if (auto fas = FullApplySite::isa(op->getUser())) {
11391165
if (auto isolation = fas.getIsolationCrossing()) {
1140-
auto isolationRegionInfo = diagnosticEmitter.getIsolationRegionInfo();
11411166
diagnosticEmitter.emitFunctionArgumentApply(
1142-
loc, op->get()->getType().getASTType(), isolationRegionInfo,
1143-
*isolation);
1167+
loc, op->get()->getType().getASTType(), *isolation);
11441168
return true;
11451169
}
11461170
}

test/Concurrency/transfernonsendable_strong_transferring_params.swift

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,10 +333,19 @@ func testTransferOtherParamTuple(_ x: transferring Klass, y: (Klass, Klass)) asy
333333
x = y.0
334334
}
335335

336-
func useSugaredTypeNameWhenEmittingTaskIsolationError(_ x: @escaping @MainActor () async -> ()) {
336+
func taskIsolatedError(_ x: @escaping @MainActor () async -> ()) {
337337
func fakeInit(operation: transferring @escaping () async -> ()) {}
338338

339-
fakeInit(operation: x) // expected-warning {{task-isolated value of type '@MainActor () async -> ()' passed as a strongly transferred parameter}}
339+
fakeInit(operation: x) // expected-warning {{transferring 'x' may cause a race}}
340+
// expected-note @-1 {{task-isolated 'x' is passed as a transferring parameter; Uses in callee may race with later task-isolated uses}}
341+
}
342+
343+
@MainActor func actorIsolatedError(_ x: @escaping @MainActor () async -> ()) {
344+
func fakeInit(operation: transferring @escaping () async -> ()) {}
345+
346+
// TODO: This needs to say actor-isolated.
347+
fakeInit(operation: x) // expected-warning {{transferring 'x' may cause a race}}
348+
// expected-note @-1 {{task-isolated 'x' is passed as a transferring parameter; Uses in callee may race with later task-isolated uses}}
340349
}
341350

342351
// Make sure we error here on only the second since x by being assigned a part
@@ -348,3 +357,9 @@ func testMergeWithTaskIsolated(_ x: transferring Klass, y: Klass) async {
348357
await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
349358
// expected-note @-1 {{transferring nonisolated 'x' to main actor-isolated callee could cause races between main actor-isolated and nonisolated uses}}
350359
}
360+
361+
@MainActor func testMergeWithActorIsolated(_ x: transferring Klass, y: Klass) async {
362+
x = y
363+
await transferToCustom(x) // expected-warning {{transferring 'x' may cause a race}}
364+
// expected-note @-1 {{transferring main actor-isolated 'x' to global actor 'CustomActor'-isolated callee could cause races between global actor 'CustomActor'-isolated and main actor-isolated uses}}
365+
}

test/Concurrency/transfernonsendable_warning_until_swift6.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ func testTransferArgumentError(_ x: NonSendableType) async { // expected-note {{
2929
}
3030

3131
func testPassArgumentAsTransferringParameter(_ x: NonSendableType) async {
32-
transferValue(x) // expected-error {{task-isolated value of type 'NonSendableType' passed as a strongly transferred parameter; later accesses could race}}
32+
transferValue(x) // expected-error {{transferring 'x' may cause a race}}
33+
// expected-note @-1 {{task-isolated 'x' is passed as a transferring parameter; Uses in callee may race with later task-isolated uses}}
3334
}
3435

3536
func testAssignmentIntoTransferringParameter(_ x: transferring NonSendableType) async {

0 commit comments

Comments
 (0)