Skip to content

Commit 14634b6

Browse files
gottesmmgottesmm
committed
[rbi] Begin tracking if a function argument is from a nonisolated(nonsending) parameter and adjust printing as appropriate.
Specifically in terms of printing, if NonisolatedNonsendingByDefault is enabled, we print out things as nonisolated/task-isolated and @concurrent/@Concurrent task-isolated. If said feature is disabled, we print out things as nonisolated(nonsending)/nonisolated(nonsending) task-isolated and nonisolated/task-isolated. This ensures in the default case, diagnostics do not change and we always print out things to match the expected meaning of nonisolated depending on the mode. I also updated the tests as appropriate/added some more tests/added to the SendNonSendable education notes information about this.
1 parent 4433ab8 commit 14634b6

File tree

9 files changed

+209
-79
lines changed

9 files changed

+209
-79
lines changed

‎include/swift/SILOptimizer/Utils/SILIsolationInfo.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,10 @@ class SILIsolationInfo {
185185
/// parameter and should be allowed to merge into a self parameter.
186186
UnappliedIsolatedAnyParameter = 0x2,
187187

188+
/// If set, this was a TaskIsolated value from a nonisolated(nonsending)
189+
/// parameter.
190+
NonisolatedNonsendingTaskIsolated = 0x4,
191+
188192
/// The maximum number of bits used by a Flag.
189193
MaxNumBits = 2,
190194
};
@@ -269,6 +273,25 @@ class SILIsolationInfo {
269273
return self;
270274
}
271275

276+
bool isNonisolatedNonsendingTaskIsolated() const {
277+
return getOptions().contains(Flag::NonisolatedNonsendingTaskIsolated);
278+
}
279+
280+
SILIsolationInfo
281+
withNonisolatedNonsendingTaskIsolated(bool newValue = true) const {
282+
assert(*this && "Cannot be unknown");
283+
assert(isTaskIsolated() && "Can only be task isolated");
284+
auto self = *this;
285+
if (newValue) {
286+
self.options =
287+
(self.getOptions() | Flag::NonisolatedNonsendingTaskIsolated).toRaw();
288+
} else {
289+
self.options = self.getOptions().toRaw() &
290+
~Options(Flag::NonisolatedNonsendingTaskIsolated).toRaw();
291+
}
292+
return self;
293+
}
294+
272295
/// Returns true if this actor isolation is derived from an unapplied
273296
/// isolation parameter. When merging, we allow for this to be merged with a
274297
/// more specific isolation kind.

‎lib/SILOptimizer/Utils/SILIsolationInfo.cpp‎

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,12 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
933933
/// Consider non-Sendable metatypes to be task-isolated, so they cannot cross
934934
/// into another isolation domain.
935935
if (auto *mi = dyn_cast<MetatypeInst>(inst)) {
936+
if (auto funcIsolation = mi->getFunction()->getActorIsolation();
937+
funcIsolation && funcIsolation->isCallerIsolationInheriting()) {
938+
return SILIsolationInfo::getTaskIsolated(mi)
939+
.withNonisolatedNonsendingTaskIsolated(true);
940+
}
941+
936942
return SILIsolationInfo::getTaskIsolated(mi);
937943
}
938944

@@ -998,7 +1004,8 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
9981004
// task isolated.
9991005
if (auto funcIsolation = fArg->getFunction()->getActorIsolation();
10001006
funcIsolation && funcIsolation->isCallerIsolationInheriting()) {
1001-
return SILIsolationInfo::getTaskIsolated(fArg);
1007+
return SILIsolationInfo::getTaskIsolated(fArg)
1008+
.withNonisolatedNonsendingTaskIsolated(true);
10021009
}
10031010

10041011
auto astType = isolatedArg->getType().getASTType();
@@ -1092,6 +1099,21 @@ void SILIsolationInfo::printOptions(llvm::raw_ostream &os) const {
10921099
void SILIsolationInfo::printActorIsolationForDiagnostics(
10931100
SILFunction *fn, ActorIsolation iso, llvm::raw_ostream &os,
10941101
StringRef openingQuotationMark, bool asNoun) {
1102+
// If we have NonisolatedNonsendingByDefault enabled, we need to return
1103+
// @concurrent for nonisolated and nonisolated for caller isolation inherited.
1104+
if (fn->isAsync() && fn->getASTContext().LangOpts.hasFeature(
1105+
Feature::NonisolatedNonsendingByDefault)) {
1106+
if (iso.isCallerIsolationInheriting()) {
1107+
os << "nonisolated";
1108+
return;
1109+
}
1110+
1111+
if (iso.isNonisolated()) {
1112+
os << "@concurrent";
1113+
return;
1114+
}
1115+
}
1116+
10951117
return iso.printForDiagnostics(os, openingQuotationMark, asNoun);
10961118
}
10971119

@@ -1213,6 +1235,7 @@ bool SILIsolationInfo::isEqual(const SILIsolationInfo &other) const {
12131235

12141236
void SILIsolationInfo::Profile(llvm::FoldingSetNodeID &id) const {
12151237
id.AddInteger(getKind());
1238+
id.AddInteger(getOptions().toRaw());
12161239
switch (getKind()) {
12171240
case Unknown:
12181241
case Disconnected:
@@ -1266,6 +1289,21 @@ void SILIsolationInfo::printForDiagnostics(SILFunction *fn,
12661289
printActorIsolationForDiagnostics(fn, getActorIsolation(), os);
12671290
return;
12681291
case Task:
1292+
if (fn->isAsync() && fn->getASTContext().LangOpts.hasFeature(
1293+
Feature::NonisolatedNonsendingByDefault)) {
1294+
if (isNonisolatedNonsendingTaskIsolated()) {
1295+
os << "task-isolated";
1296+
return;
1297+
}
1298+
os << "@concurrent task-isolated";
1299+
return;
1300+
}
1301+
1302+
if (isNonisolatedNonsendingTaskIsolated()) {
1303+
os << "nonisolated(nonsending) task-isolated";
1304+
return;
1305+
}
1306+
12691307
os << "task-isolated";
12701308
return;
12711309
}
@@ -1486,8 +1524,13 @@ std::optional<SILDynamicMergedIsolationInfo>
14861524
SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
14871525
// If we are greater than the other kind, then we are further along the
14881526
// lattice. We ignore the change.
1489-
if (unsigned(innerInfo.getKind() > unsigned(other.getKind())))
1527+
//
1528+
// NOTE: If we are further along, then we both cannot be task isolated. In
1529+
// such a case, we are the only potential thing that can be
1530+
// nonisolated(unsafe)... so we do not need to try to propagate.
1531+
if (unsigned(innerInfo.getKind() > unsigned(other.getKind()))) {
14901532
return {*this};
1533+
}
14911534

14921535
// If we are both actor isolated...
14931536
if (innerInfo.isActorIsolated() && other.isActorIsolated()) {
@@ -1522,6 +1565,21 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
15221565
return {other.withUnsafeNonIsolated(false)};
15231566
}
15241567

1568+
// We know that we are either the same as other or other is further along. If
1569+
// other is further along, it is the only thing that can propagate the task
1570+
// isolated bit. So we do not need to do anything. If we are equal though, we
1571+
// may need to propagate the bit. This ensures that when we emit a diagnostic
1572+
// we appropriately say potentially actor isolated code instead of code in the
1573+
// current task.
1574+
//
1575+
// TODO: We should really represent this as a separate isolation info
1576+
// kind... but that would be a larger change than we want for 6.2.
1577+
if (innerInfo.isTaskIsolated() && other.isTaskIsolated()) {
1578+
if (innerInfo.isNonisolatedNonsendingTaskIsolated() ||
1579+
other.isNonisolatedNonsendingTaskIsolated())
1580+
return other.withNonisolatedNonsendingTaskIsolated(true);
1581+
}
1582+
15251583
// Otherwise, just return other.
15261584
return {other};
15271585
}

‎test/ClangImporter/regionbasedisolation.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,15 @@ nonisolated(nonsending) func useValueNonIsolatedNonSending<T>(_ t: T) async {}
134134
let x = ObjCObject()
135135
await x.useValue(y)
136136
await useValueConcurrently(x) // expected-error {{sending 'x' risks causing data races}}
137-
// expected-note @-1 {{sending main actor-isolated 'x' to nonisolated global function 'useValueConcurrently' risks causing data races between nonisolated and main actor-isolated uses}}
137+
// expected-ni-note @-1 {{sending main actor-isolated 'x' to nonisolated global function 'useValueConcurrently' risks causing data races between nonisolated and main actor-isolated uses}}
138+
// expected-ni-ns-note @-2 {{sending main actor-isolated 'x' to @concurrent global function 'useValueConcurrently' risks causing data races between @concurrent and main actor-isolated uses}}
138139
}
139140

140141
func testTaskLocal(_ y: NSObject) async {
141142
let x = ObjCObject()
142143
await x.useValue(y)
143144
await useValueConcurrently(x) // expected-ni-ns-error {{sending 'x' risks causing data races}}
144-
// expected-ni-ns-note @-1 {{sending task-isolated 'x' to nonisolated global function 'useValueConcurrently' risks causing data races between nonisolated and task-isolated uses}}
145+
// expected-ni-ns-note @-1 {{sending task-isolated 'x' to @concurrent global function 'useValueConcurrently' risks causing data races between @concurrent and task-isolated uses}}
145146

146147
// This is not safe since we merge x into y's region making x task
147148
// isolated. We then try to send it to a main actor function.

‎test/Concurrency/nonisolated_inherits_isolation.swift‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -289,45 +289,47 @@ func unspecifiedCallingVariousNonisolated(_ x: NonSendableKlass) async {
289289
await x.nonisolatedCaller()
290290
await x.nonisolatedNonSendingCaller()
291291
await x.concurrentCaller() // expected-enabled-error {{sending 'x' risks causing data races}}
292-
// expected-enabled-note @-1 {{sending task-isolated 'x' to nonisolated instance method 'concurrentCaller()' risks causing data races between nonisolated and task-isolated uses}}
292+
// expected-enabled-note @-1 {{sending task-isolated 'x' to @concurrent instance method 'concurrentCaller()' risks causing data races between @concurrent and task-isolated uses}}
293293

294294
await unspecifiedAsyncUse(x)
295295
await nonisolatedAsyncUse(x)
296296
await nonisolatedNonSendingAsyncUse(x)
297297
await concurrentAsyncUse(x) // expected-enabled-error {{sending 'x' risks causing data races}}
298-
// expected-enabled-note @-1 {{sending task-isolated 'x' to nonisolated global function 'concurrentAsyncUse' risks causing data races between nonisolated and task-isolated uses}}
298+
// expected-enabled-note @-1 {{sending task-isolated 'x' to @concurrent global function 'concurrentAsyncUse' risks causing data races between @concurrent and task-isolated uses}}
299299
}
300300

301301
nonisolated func nonisolatedCallingVariousNonisolated(_ x: NonSendableKlass) async {
302302
await x.unspecifiedCaller()
303303
await x.nonisolatedCaller()
304304
await x.nonisolatedNonSendingCaller()
305305
await x.concurrentCaller() // expected-enabled-error {{sending 'x' risks causing data races}}
306-
// expected-enabled-note @-1 {{sending task-isolated 'x' to nonisolated instance method 'concurrentCaller()' risks causing data races between nonisolated and task-isolated uses}}
306+
// expected-enabled-note @-1 {{sending task-isolated 'x' to @concurrent instance method 'concurrentCaller()' risks causing data races between @concurrent and task-isolated uses}}
307307

308308
await unspecifiedAsyncUse(x)
309309
await nonisolatedAsyncUse(x)
310310
await nonisolatedNonSendingAsyncUse(x)
311311
await concurrentAsyncUse(x) // expected-enabled-error {{sending 'x' risks causing data races}}
312-
// expected-enabled-note @-1 {{sending task-isolated 'x' to nonisolated global function 'concurrentAsyncUse' risks causing data races between nonisolated and task-isolated uses}}
312+
// expected-enabled-note @-1 {{sending task-isolated 'x' to @concurrent global function 'concurrentAsyncUse' risks causing data races between @concurrent and task-isolated uses}}
313313
}
314314

315315
nonisolated(nonsending) func nonisolatedNonSendingCallingVariousNonisolated(_ x: NonSendableKlass) async {
316316
await x.unspecifiedCaller() // expected-disabled-error {{sending 'x' risks causing data races}}
317-
// expected-disabled-note @-1 {{sending task-isolated 'x' to nonisolated instance method 'unspecifiedCaller()' risks causing data races between nonisolated and task-isolated uses}}
317+
// expected-disabled-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated instance method 'unspecifiedCaller()' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
318318
await x.nonisolatedCaller() // expected-disabled-error {{sending 'x' risks causing data races}}
319-
// expected-disabled-note @-1 {{sending task-isolated 'x' to nonisolated instance method 'nonisolatedCaller()' risks causing data races between nonisolated and task-isolated uses}}
319+
// expected-disabled-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated instance method 'nonisolatedCaller()' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
320320
await x.nonisolatedNonSendingCaller()
321321
await x.concurrentCaller() // expected-error {{sending 'x' risks causing data races}}
322-
// expected-note @-1 {{sending task-isolated 'x' to nonisolated instance method 'concurrentCaller()' risks causing data races between nonisolated and task-isolated uses}}
322+
// expected-disabled-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated instance method 'concurrentCaller()' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
323+
// expected-enabled-note @-2 {{sending task-isolated 'x' to @concurrent instance method 'concurrentCaller()' risks causing data races between @concurrent and task-isolated uses}}
323324

324325
await unspecifiedAsyncUse(x) // expected-disabled-error {{sending 'x' risks causing data races}}
325-
// expected-disabled-note @-1 {{sending task-isolated 'x' to nonisolated global function 'unspecifiedAsyncUse' risks causing data races between nonisolated and task-isolated uses}}
326+
// expected-disabled-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated global function 'unspecifiedAsyncUse' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
326327
await nonisolatedAsyncUse(x) // expected-disabled-error {{sending 'x' risks causing data races}}
327-
// expected-disabled-note @-1 {{sending task-isolated 'x' to nonisolated global function 'nonisolatedAsyncUse' risks causing data races between nonisolated and task-isolated uses}}
328+
// expected-disabled-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated global function 'nonisolatedAsyncUse' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
328329
await nonisolatedNonSendingAsyncUse(x)
329330
await concurrentAsyncUse(x) // expected-error {{sending 'x' risks causing data races}}
330-
// expected-note @-1 {{sending task-isolated 'x' to nonisolated global function 'concurrentAsyncUse' risks causing data races between nonisolated and task-isolated uses}}
331+
// expected-disabled-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated global function 'concurrentAsyncUse' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
332+
// expected-enabled-note @-2 {{sending task-isolated 'x' to @concurrent global function 'concurrentAsyncUse' risks causing data races between @concurrent and task-isolated uses}}
331333
}
332334

333335
@concurrent func concurrentCallingVariousNonisolated(_ x: NonSendableKlass) async {

‎test/Concurrency/transfernonsendable.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ final class FinalMainActorIsolatedKlass {
7171
func useInOut<T>(_ x: inout T) {}
7272
func useValue<T>(_ x: T) {}
7373
func useValueAsync<T>(_ x: T) async {}
74+
@concurrent func useValueAsyncConcurrent<T>(_ x: T) async {}
7475

7576
@MainActor func transferToMain<T>(_ t: T) async {}
7677

@@ -2101,3 +2102,10 @@ func inferLocationOfCapturedTaskIsolatedSelfCorrectly() {
21012102
}
21022103
}
21032104

2105+
nonisolated(nonsending) func testCallNonisolatedNonsending(_ x: NonSendableKlass) async {
2106+
await useValueAsync(x) // expected-tns-ni-warning {{sending 'x' risks causing data races}}
2107+
// expected-tns-ni-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
2108+
await useValueAsyncConcurrent(x) // expected-tns-warning {{sending 'x' risks causing data races}}
2109+
// expected-tns-ni-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated global function 'useValueAsyncConcurrent' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
2110+
// expected-tns-ni-ns-note @-2 {{sending task-isolated 'x' to @concurrent global function 'useValueAsyncConcurrent' risks causing data races between @concurrent and task-isolated uses}}
2111+
}

0 commit comments

Comments
 (0)