Skip to content

Commit e55eb6c

Browse files
authored
Merge pull request #82884 from DougGregor/isolated-conformances-region-based-diags
[SE-0470] Track the potential introduction of isolated conformances in regions
2 parents 57ef411 + 02c34bb commit e55eb6c

File tree

6 files changed

+132
-56
lines changed

6 files changed

+132
-56
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,10 @@ NOTE(regionbasedisolation_typed_sendneversendable_via_arg_callee, none,
10981098
"sending %0 value of non-Sendable type %1 to %2 %kind3 risks causing races in between %0 and %2 uses",
10991099
(StringRef, Type, StringRef, const ValueDecl *))
11001100

1101+
NOTE(regionbasedisolation_isolated_conformance_introduced, none,
1102+
"isolated conformance to %kind0 can be introduced here",
1103+
(const ValueDecl *))
1104+
11011105
// Error that is only used when the send non sendable emitter cannot discover any
11021106
// information to give a better diagnostic.
11031107
ERROR(regionbasedisolation_task_or_actor_isolated_sent, none,

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ class SILIsolationInfo {
190190
NonisolatedNonsendingTaskIsolated = 0x4,
191191

192192
/// The maximum number of bits used by a Flag.
193-
MaxNumBits = 2,
193+
MaxNumBits = 3,
194194
};
195195

196196
using Options = OptionSet<Flag>;
@@ -208,13 +208,19 @@ class SILIsolationInfo {
208208
/// derived isolatedValue from.
209209
ActorInstance actorInstance;
210210

211+
/// When the isolation is introduced due to a (potentially) isolated
212+
/// conformance, the protocol whose conformance might be isolated.
213+
ProtocolDecl *isolatedConformance = nullptr;
214+
211215
unsigned kind : 8;
212216
unsigned options : 8;
213217

214218
SILIsolationInfo(SILValue isolatedValue, SILValue actorInstance,
215-
ActorIsolation actorIsolation, Options options = Options())
219+
ActorIsolation actorIsolation, Options options = Options(),
220+
ProtocolDecl *isolatedConformance = nullptr)
216221
: actorIsolation(actorIsolation), isolatedValue(isolatedValue),
217-
actorInstance(ActorInstance::getForValue(actorInstance)), kind(Actor),
222+
actorInstance(ActorInstance::getForValue(actorInstance)),
223+
isolatedConformance(isolatedConformance), kind(Actor),
218224
options(options.toRaw()) {
219225
assert((!actorInstance ||
220226
(actorIsolation.getKind() == ActorIsolation::ActorInstance &&
@@ -226,15 +232,20 @@ class SILIsolationInfo {
226232
}
227233

228234
SILIsolationInfo(SILValue isolatedValue, ActorInstance actorInstance,
229-
ActorIsolation actorIsolation, Options options = Options())
235+
ActorIsolation actorIsolation, Options options = Options(),
236+
ProtocolDecl *isolatedConformance = nullptr)
230237
: actorIsolation(actorIsolation), isolatedValue(isolatedValue),
231-
actorInstance(actorInstance), kind(Actor), options(options.toRaw()) {
238+
actorInstance(actorInstance), isolatedConformance(isolatedConformance),
239+
kind(Actor), options(options.toRaw())
240+
{
232241
assert(actorInstance);
233242
assert(actorIsolation.getKind() == ActorIsolation::ActorInstance);
234243
}
235244

236-
SILIsolationInfo(Kind kind, SILValue isolatedValue)
237-
: actorIsolation(), isolatedValue(isolatedValue), kind(kind), options(0) {
245+
SILIsolationInfo(Kind kind, SILValue isolatedValue,
246+
ProtocolDecl *isolatedConformance = nullptr)
247+
: actorIsolation(), isolatedValue(isolatedValue),
248+
isolatedConformance(isolatedConformance), kind(kind), options(0) {
238249
}
239250

240251
SILIsolationInfo(Kind kind, Options options = Options())
@@ -261,6 +272,12 @@ class SILIsolationInfo {
261272
return getOptions().contains(Flag::UnsafeNonIsolated);
262273
}
263274

275+
// Retrieve the protocol to which there is (or could be) an isolated
276+
// conformance.
277+
ProtocolDecl *getIsolatedConformance() const {
278+
return isolatedConformance;
279+
}
280+
264281
SILIsolationInfo withUnsafeNonIsolated(bool newValue = true) const {
265282
assert(*this && "Cannot be unknown");
266283
auto self = *this;
@@ -292,6 +309,26 @@ class SILIsolationInfo {
292309
return self;
293310
}
294311

312+
/// Produce a new isolation info value that merges in the given isolated
313+
/// conformance value.
314+
///
315+
/// If both isolation infos have an isolation conformance, pick one
316+
/// arbitrarily. Otherwise, the result has no isolated conformance.
317+
SILIsolationInfo
318+
withMergedIsolatedConformance(ProtocolDecl *newIsolatedConformance) const {
319+
SILIsolationInfo result(*this);
320+
if (!isolatedConformance || !newIsolatedConformance) {
321+
result.isolatedConformance = nullptr;
322+
return result;
323+
}
324+
325+
result.isolatedConformance =
326+
ProtocolDecl::compare(isolatedConformance, newIsolatedConformance) <= 0
327+
? isolatedConformance
328+
: newIsolatedConformance;
329+
return result;
330+
}
331+
295332
/// Returns true if this actor isolation is derived from an unapplied
296333
/// isolation parameter. When merging, we allow for this to be merged with a
297334
/// more specific isolation kind.
@@ -458,10 +495,13 @@ class SILIsolationInfo {
458495
Flag::UnappliedIsolatedAnyParameter};
459496
}
460497

461-
static SILIsolationInfo getGlobalActorIsolated(SILValue value,
462-
Type globalActorType) {
498+
static SILIsolationInfo getGlobalActorIsolated(
499+
SILValue value,
500+
Type globalActorType,
501+
ProtocolDecl *isolatedConformance = nullptr) {
463502
return {value, SILValue() /*no actor instance*/,
464-
ActorIsolation::forGlobalActor(globalActorType)};
503+
ActorIsolation::forGlobalActor(globalActorType),
504+
Options(), isolatedConformance};
465505
}
466506

467507
static SILIsolationInfo getGlobalActorIsolated(SILValue value,
@@ -473,8 +513,9 @@ class SILIsolationInfo {
473513
isolation.getGlobalActor());
474514
}
475515

476-
static SILIsolationInfo getTaskIsolated(SILValue value) {
477-
return {Kind::Task, value};
516+
static SILIsolationInfo getTaskIsolated(
517+
SILValue value, ProtocolDecl *isolatedConformance = nullptr) {
518+
return {Kind::Task, value, isolatedConformance};
478519
}
479520

480521
/// Attempt to infer the isolation region info for \p inst.

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,9 @@ static SILIsolationInfo getIsolationForCastConformances(
297297

298298
const auto &destLayout = destType.getExistentialLayout();
299299
for (auto proto : destLayout.getProtocols()) {
300+
if (proto->isMarkerProtocol())
301+
continue;
302+
300303
// If the source type already conforms to the protocol, we won't be looking
301304
// it up dynamically.
302305
if (!lookupConformance(sourceType, proto, /*allowMissing=*/false).isInvalid())
@@ -313,12 +316,12 @@ static SILIsolationInfo getIsolationForCastConformances(
313316
// Otherwise, it's task-isolated.
314317
if (functionIsolation && functionIsolation->isGlobalActor()) {
315318
return SILIsolationInfo::getGlobalActorIsolated(
316-
value, functionIsolation->getGlobalActor());
319+
value, functionIsolation->getGlobalActor(), proto);
317320
}
318321

319322
// Consider the cast to be task-isolated, because the runtime could find
320323
// a conformance that is isolated to the current context.
321-
return SILIsolationInfo::getTaskIsolated(value);
324+
return SILIsolationInfo::getTaskIsolated(value, proto);
322325
}
323326

324327
return {};
@@ -4131,6 +4134,9 @@ SILIsolationInfo
41314134
PartitionOpTranslator::getIsolationFromConformances(
41324135
SILValue value, ArrayRef<ProtocolConformanceRef> conformances) {
41334136
for (auto conformance: conformances) {
4137+
if (conformance.getProtocol()->isMarkerProtocol())
4138+
continue;
4139+
41344140
// If the conformance is a pack, recurse.
41354141
if (conformance.isPack()) {
41364142
auto pack = conformance.getPack();
@@ -4149,7 +4155,7 @@ PartitionOpTranslator::getIsolationFromConformances(
41494155
auto isolation = conformance.getConcrete()->getIsolation();
41504156
if (isolation.isGlobalActor()) {
41514157
return SILIsolationInfo::getGlobalActorIsolated(
4152-
value, isolation.getGlobalActor());
4158+
value, isolation.getGlobalActor(), conformance.getProtocol());
41534159
}
41544160

41554161
continue;
@@ -4164,7 +4170,8 @@ PartitionOpTranslator::getIsolationFromConformances(
41644170
if (sendableMetatype &&
41654171
lookupConformance(conformance.getType(), sendableMetatype,
41664172
/*allowMissing=*/false).isInvalid()) {
4167-
return SILIsolationInfo::getTaskIsolated(value);
4173+
return SILIsolationInfo::getTaskIsolated(value,
4174+
conformance.getProtocol());
41684175
}
41694176
}
41704177
}
@@ -4175,7 +4182,7 @@ PartitionOpTranslator::getIsolationFromConformances(
41754182
TranslationSemantics
41764183
PartitionOpTranslator::visitInitExistentialAddrInst(InitExistentialAddrInst *ieai) {
41774184
auto conformanceIsolationInfo = getIsolationFromConformances(
4178-
ieai->getResult(0), ieai->getConformances());
4185+
ieai, ieai->getConformances());
41794186

41804187
translateSILMultiAssign(ieai->getResults(),
41814188
makeOperandRefRange(ieai->getAllOperands()),
@@ -4187,7 +4194,7 @@ PartitionOpTranslator::visitInitExistentialAddrInst(InitExistentialAddrInst *iea
41874194
TranslationSemantics
41884195
PartitionOpTranslator::visitInitExistentialRefInst(InitExistentialRefInst *ieri) {
41894196
auto conformanceIsolationInfo = getIsolationFromConformances(
4190-
ieri->getResult(0), ieri->getConformances());
4197+
ieri, ieri->getConformances());
41914198

41924199
translateSILMultiAssign(ieri->getResults(),
41934200
makeOperandRefRange(ieri->getAllOperands()),
@@ -4199,7 +4206,7 @@ PartitionOpTranslator::visitInitExistentialRefInst(InitExistentialRefInst *ieri)
41994206
TranslationSemantics
42004207
PartitionOpTranslator::visitInitExistentialValueInst(InitExistentialValueInst *ievi) {
42014208
auto conformanceIsolationInfo = getIsolationFromConformances(
4202-
ievi->getResult(0), ievi->getConformances());
4209+
ievi, ievi->getConformances());
42034210

42044211
translateSILMultiAssign(ievi->getResults(),
42054212
makeOperandRefRange(ievi->getAllOperands()),

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,6 +1312,18 @@ class SendNeverSentDiagnosticEmitter {
13121312
~SendNeverSentDiagnosticEmitter() {
13131313
if (!emittedErrorDiagnostic) {
13141314
emitUnknownPatternError();
1315+
} else if (auto proto = isolationRegionInfo.getIsolationInfo()
1316+
.getIsolatedConformance()) {
1317+
// If the diagnostic comes from a (potentially) isolated conformance,
1318+
// add a note saying so and indicating where the isolated conformance
1319+
// can come in.
1320+
if (auto value = isolationRegionInfo.getIsolationInfo().getIsolatedValue()) {
1321+
if (auto loc = value.getLoc()) {
1322+
diagnoseNote(
1323+
loc, diag::regionbasedisolation_isolated_conformance_introduced,
1324+
proto);
1325+
}
1326+
}
13151327
}
13161328
}
13171329

@@ -1326,6 +1338,13 @@ class SendNeverSentDiagnosticEmitter {
13261338
}
13271339

13281340
std::optional<DiagnosticBehavior> getBehaviorLimit() const {
1341+
// If the failure is due to an isolated conformance, downgrade the error
1342+
// to a warning prior to Swift 7.
1343+
if (isolationRegionInfo.getIsolationInfo().getIsolatedConformance() &&
1344+
!sendingOperand->get()->getType().getASTType()->getASTContext().LangOpts
1345+
.isSwiftVersionAtLeast(7))
1346+
return DiagnosticBehavior::Warning;
1347+
13291348
return sendingOperand->get()->getType().getConcurrencyDiagnosticBehavior(
13301349
getOperand()->getFunction());
13311350
}
@@ -1577,8 +1596,7 @@ class SendNeverSentDiagnosticEmitter {
15771596
getIsolationRegionInfo().printForDiagnostics(getFunction());
15781597

15791598
diagnoseNote(loc, diag::regionbasedisolation_named_send_nt_asynclet_capture,
1580-
name, descriptiveKindStr)
1581-
.limitBehaviorIf(getBehaviorLimit());
1599+
name, descriptiveKindStr);
15821600
}
15831601

15841602
void emitNamedIsolation(SILLocation loc, Identifier name,

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,6 +1070,10 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
10701070
}
10711071

10721072
void SILIsolationInfo::printOptions(llvm::raw_ostream &os) const {
1073+
if (isolatedConformance) {
1074+
os << "isolated-conformance-to(" << isolatedConformance->getName() << ")";
1075+
}
1076+
10731077
auto opts = getOptions();
10741078
if (!opts)
10751079
return;
@@ -1255,9 +1259,11 @@ void SILIsolationInfo::Profile(llvm::FoldingSetNodeID &id) const {
12551259
return;
12561260
case Task:
12571261
id.AddPointer(getIsolatedValue());
1262+
id.AddPointer(getIsolatedConformance());
12581263
return;
12591264
case Actor:
12601265
id.AddPointer(getIsolatedValue());
1266+
id.AddPointer(getIsolatedConformance());
12611267
getActorIsolation().Profile(id);
12621268
return;
12631269
}
@@ -1570,7 +1576,7 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
15701576
// If both innerInfo and other have the same isolation, we are obviously
15711577
// done. Just return innerInfo since we could return either.
15721578
if (innerInfo.hasSameIsolation(other))
1573-
return {innerInfo};
1579+
return {innerInfo.withMergedIsolatedConformance(other.getIsolatedConformance())};
15741580

15751581
// Ok, there is some difference in between innerInfo and other. Lets see if
15761582
// they are both actor instance isolated and if either are unapplied
@@ -1579,9 +1585,9 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
15791585
if (innerInfo.getActorIsolation().isActorInstanceIsolated() &&
15801586
other.getActorIsolation().isActorInstanceIsolated()) {
15811587
if (innerInfo.isUnappliedIsolatedAnyParameter())
1582-
return other;
1588+
return other.withMergedIsolatedConformance(innerInfo.getIsolatedConformance());
15831589
if (other.isUnappliedIsolatedAnyParameter())
1584-
return innerInfo;
1590+
return innerInfo.withMergedIsolatedConformance(other.getIsolatedConformance());
15851591
}
15861592

15871593
// Otherwise, they do not match... so return None to signal merge failure.
@@ -1610,7 +1616,8 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
16101616
if (innerInfo.isTaskIsolated() && other.isTaskIsolated()) {
16111617
if (innerInfo.isNonisolatedNonsendingTaskIsolated() ||
16121618
other.isNonisolatedNonsendingTaskIsolated())
1613-
return other.withNonisolatedNonsendingTaskIsolated(true);
1619+
return other.withNonisolatedNonsendingTaskIsolated(true)
1620+
.withMergedIsolatedConformance(innerInfo.getIsolatedConformance());
16141621
}
16151622

16161623
// Otherwise, just return other.

0 commit comments

Comments
 (0)