Skip to content

Commit 85fafa5

Browse files
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. (cherry picked from commit 14634b6)
1 parent ca8cdc1 commit 85fafa5

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 = 3,
190194
};
@@ -317,6 +321,25 @@ class SILIsolationInfo {
317321
return result;
318322
}
319323

324+
bool isNonisolatedNonsendingTaskIsolated() const {
325+
return getOptions().contains(Flag::NonisolatedNonsendingTaskIsolated);
326+
}
327+
328+
SILIsolationInfo
329+
withNonisolatedNonsendingTaskIsolated(bool newValue = true) const {
330+
assert(*this && "Cannot be unknown");
331+
assert(isTaskIsolated() && "Can only be task isolated");
332+
auto self = *this;
333+
if (newValue) {
334+
self.options =
335+
(self.getOptions() | Flag::NonisolatedNonsendingTaskIsolated).toRaw();
336+
} else {
337+
self.options = self.getOptions().toRaw() &
338+
~Options(Flag::NonisolatedNonsendingTaskIsolated).toRaw();
339+
}
340+
return self;
341+
}
342+
320343
/// Returns true if this actor isolation is derived from an unapplied
321344
/// isolation parameter. When merging, we allow for this to be merged with a
322345
/// more specific isolation kind.

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,12 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
938938
/// Consider non-Sendable metatypes to be task-isolated, so they cannot cross
939939
/// into another isolation domain.
940940
if (auto *mi = dyn_cast<MetatypeInst>(inst)) {
941+
if (auto funcIsolation = mi->getFunction()->getActorIsolation();
942+
funcIsolation && funcIsolation->isCallerIsolationInheriting()) {
943+
return SILIsolationInfo::getTaskIsolated(mi)
944+
.withNonisolatedNonsendingTaskIsolated(true);
945+
}
946+
941947
return SILIsolationInfo::getTaskIsolated(mi);
942948
}
943949

@@ -1003,7 +1009,8 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
10031009
// task isolated.
10041010
if (auto funcIsolation = fArg->getFunction()->getActorIsolation();
10051011
funcIsolation && funcIsolation->isCallerIsolationInheriting()) {
1006-
return SILIsolationInfo::getTaskIsolated(fArg);
1012+
return SILIsolationInfo::getTaskIsolated(fArg)
1013+
.withNonisolatedNonsendingTaskIsolated(true);
10071014
}
10081015

10091016
auto astType = isolatedArg->getType().getASTType();
@@ -1240,6 +1247,21 @@ void SILIsolationInfo::printOptions(llvm::raw_ostream &os) const {
12401247
void SILIsolationInfo::printActorIsolationForDiagnostics(
12411248
SILFunction *fn, ActorIsolation iso, llvm::raw_ostream &os,
12421249
StringRef openingQuotationMark, bool asNoun) {
1250+
// If we have NonisolatedNonsendingByDefault enabled, we need to return
1251+
// @concurrent for nonisolated and nonisolated for caller isolation inherited.
1252+
if (fn->isAsync() && fn->getASTContext().LangOpts.hasFeature(
1253+
Feature::NonisolatedNonsendingByDefault)) {
1254+
if (iso.isCallerIsolationInheriting()) {
1255+
os << "nonisolated";
1256+
return;
1257+
}
1258+
1259+
if (iso.isNonisolated()) {
1260+
os << "@concurrent";
1261+
return;
1262+
}
1263+
}
1264+
12431265
return iso.printForDiagnostics(os, openingQuotationMark, asNoun);
12441266
}
12451267

@@ -1361,6 +1383,7 @@ bool SILIsolationInfo::isEqual(const SILIsolationInfo &other) const {
13611383

13621384
void SILIsolationInfo::Profile(llvm::FoldingSetNodeID &id) const {
13631385
id.AddInteger(getKind());
1386+
id.AddInteger(getOptions().toRaw());
13641387
switch (getKind()) {
13651388
case Unknown:
13661389
case Disconnected:
@@ -1416,6 +1439,21 @@ void SILIsolationInfo::printForDiagnostics(SILFunction *fn,
14161439
printActorIsolationForDiagnostics(fn, getActorIsolation(), os);
14171440
return;
14181441
case Task:
1442+
if (fn->isAsync() && fn->getASTContext().LangOpts.hasFeature(
1443+
Feature::NonisolatedNonsendingByDefault)) {
1444+
if (isNonisolatedNonsendingTaskIsolated()) {
1445+
os << "task-isolated";
1446+
return;
1447+
}
1448+
os << "@concurrent task-isolated";
1449+
return;
1450+
}
1451+
1452+
if (isNonisolatedNonsendingTaskIsolated()) {
1453+
os << "nonisolated(nonsending) task-isolated";
1454+
return;
1455+
}
1456+
14191457
os << "task-isolated";
14201458
return;
14211459
}
@@ -1636,8 +1674,13 @@ std::optional<SILDynamicMergedIsolationInfo>
16361674
SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
16371675
// If we are greater than the other kind, then we are further along the
16381676
// lattice. We ignore the change.
1639-
if (unsigned(innerInfo.getKind() > unsigned(other.getKind())))
1677+
//
1678+
// NOTE: If we are further along, then we both cannot be task isolated. In
1679+
// such a case, we are the only potential thing that can be
1680+
// nonisolated(unsafe)... so we do not need to try to propagate.
1681+
if (unsigned(innerInfo.getKind() > unsigned(other.getKind()))) {
16401682
return {*this};
1683+
}
16411684

16421685
// If we are both actor isolated...
16431686
if (innerInfo.isActorIsolated() && other.isActorIsolated()) {
@@ -1672,6 +1715,21 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
16721715
return {other.withUnsafeNonIsolated(false)};
16731716
}
16741717

1718+
// We know that we are either the same as other or other is further along. If
1719+
// other is further along, it is the only thing that can propagate the task
1720+
// isolated bit. So we do not need to do anything. If we are equal though, we
1721+
// may need to propagate the bit. This ensures that when we emit a diagnostic
1722+
// we appropriately say potentially actor isolated code instead of code in the
1723+
// current task.
1724+
//
1725+
// TODO: We should really represent this as a separate isolation info
1726+
// kind... but that would be a larger change than we want for 6.2.
1727+
if (innerInfo.isTaskIsolated() && other.isTaskIsolated()) {
1728+
if (innerInfo.isNonisolatedNonsendingTaskIsolated() ||
1729+
other.isNonisolatedNonsendingTaskIsolated())
1730+
return other.withNonisolatedNonsendingTaskIsolated(true);
1731+
}
1732+
16751733
// Otherwise, just return other.
16761734
return {other};
16771735
}

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

@@ -2074,3 +2075,10 @@ func inferLocationOfCapturedTaskIsolatedSelfCorrectly() {
20742075
}
20752076
}
20762077

2078+
nonisolated(nonsending) func testCallNonisolatedNonsending(_ x: NonSendableKlass) async {
2079+
await useValueAsync(x) // expected-tns-ni-warning {{sending 'x' risks causing data races}}
2080+
// 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}}
2081+
await useValueAsyncConcurrent(x) // expected-tns-warning {{sending 'x' risks causing data races}}
2082+
// 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}}
2083+
// 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}}
2084+
}

0 commit comments

Comments
 (0)