Skip to content

Commit 6637960

Browse files
committed
[SE-0470] Track the potential introduction of isolated conformances in regions
When we introduce isolation due to a (potential) isolated conformance, keep track of the protocol to which the conformance could be introduced. Use this information for two reasons: 1. Downgrade the error to a warning in Swift < 7, because we are newly diagnosing these 2. Add a note indicating where the isolated conformance could be introduced. (cherry picked from commit 02c34bb)
1 parent 900a4cc commit 6637960

File tree

6 files changed

+130
-55
lines changed

6 files changed

+130
-55
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,10 @@ NOTE(regionbasedisolation_typed_sendneversendable_via_arg_callee, none,
10811081
"sending %0 value of non-Sendable type %1 to %2 %3 %4 risks causing races in between %0 and %2 uses",
10821082
(StringRef, Type, ActorIsolation, DescriptiveDeclKind, DeclName))
10831083

1084+
NOTE(regionbasedisolation_isolated_conformance_introduced, none,
1085+
"isolated conformance to %kind0 can be introduced here",
1086+
(const ValueDecl *))
1087+
10841088
// Error that is only used when the send non sendable emitter cannot discover any
10851089
// information to give a better diagnostic.
10861090
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
@@ -186,7 +186,7 @@ class SILIsolationInfo {
186186
UnappliedIsolatedAnyParameter = 0x2,
187187

188188
/// The maximum number of bits used by a Flag.
189-
MaxNumBits = 2,
189+
MaxNumBits = 3,
190190
};
191191

192192
using Options = OptionSet<Flag>;
@@ -204,13 +204,19 @@ class SILIsolationInfo {
204204
/// derived isolatedValue from.
205205
ActorInstance actorInstance;
206206

207+
/// When the isolation is introduced due to a (potentially) isolated
208+
/// conformance, the protocol whose conformance might be isolated.
209+
ProtocolDecl *isolatedConformance = nullptr;
210+
207211
unsigned kind : 8;
208212
unsigned options : 8;
209213

210214
SILIsolationInfo(SILValue isolatedValue, SILValue actorInstance,
211-
ActorIsolation actorIsolation, Options options = Options())
215+
ActorIsolation actorIsolation, Options options = Options(),
216+
ProtocolDecl *isolatedConformance = nullptr)
212217
: actorIsolation(actorIsolation), isolatedValue(isolatedValue),
213-
actorInstance(ActorInstance::getForValue(actorInstance)), kind(Actor),
218+
actorInstance(ActorInstance::getForValue(actorInstance)),
219+
isolatedConformance(isolatedConformance), kind(Actor),
214220
options(options.toRaw()) {
215221
assert((!actorInstance ||
216222
(actorIsolation.getKind() == ActorIsolation::ActorInstance &&
@@ -222,15 +228,20 @@ class SILIsolationInfo {
222228
}
223229

224230
SILIsolationInfo(SILValue isolatedValue, ActorInstance actorInstance,
225-
ActorIsolation actorIsolation, Options options = Options())
231+
ActorIsolation actorIsolation, Options options = Options(),
232+
ProtocolDecl *isolatedConformance = nullptr)
226233
: actorIsolation(actorIsolation), isolatedValue(isolatedValue),
227-
actorInstance(actorInstance), kind(Actor), options(options.toRaw()) {
234+
actorInstance(actorInstance), isolatedConformance(isolatedConformance),
235+
kind(Actor), options(options.toRaw())
236+
{
228237
assert(actorInstance);
229238
assert(actorIsolation.getKind() == ActorIsolation::ActorInstance);
230239
}
231240

232-
SILIsolationInfo(Kind kind, SILValue isolatedValue)
233-
: actorIsolation(), isolatedValue(isolatedValue), kind(kind), options(0) {
241+
SILIsolationInfo(Kind kind, SILValue isolatedValue,
242+
ProtocolDecl *isolatedConformance = nullptr)
243+
: actorIsolation(), isolatedValue(isolatedValue),
244+
isolatedConformance(isolatedConformance), kind(kind), options(0) {
234245
}
235246

236247
SILIsolationInfo(Kind kind, Options options = Options())
@@ -257,6 +268,12 @@ class SILIsolationInfo {
257268
return getOptions().contains(Flag::UnsafeNonIsolated);
258269
}
259270

271+
// Retrieve the protocol to which there is (or could be) an isolated
272+
// conformance.
273+
ProtocolDecl *getIsolatedConformance() const {
274+
return isolatedConformance;
275+
}
276+
260277
SILIsolationInfo withUnsafeNonIsolated(bool newValue = true) const {
261278
assert(*this && "Cannot be unknown");
262279
auto self = *this;
@@ -269,6 +286,26 @@ class SILIsolationInfo {
269286
return self;
270287
}
271288

289+
/// Produce a new isolation info value that merges in the given isolated
290+
/// conformance value.
291+
///
292+
/// If both isolation infos have an isolation conformance, pick one
293+
/// arbitrarily. Otherwise, the result has no isolated conformance.
294+
SILIsolationInfo
295+
withMergedIsolatedConformance(ProtocolDecl *newIsolatedConformance) const {
296+
SILIsolationInfo result(*this);
297+
if (!isolatedConformance || !newIsolatedConformance) {
298+
result.isolatedConformance = nullptr;
299+
return result;
300+
}
301+
302+
result.isolatedConformance =
303+
ProtocolDecl::compare(isolatedConformance, newIsolatedConformance) <= 0
304+
? isolatedConformance
305+
: newIsolatedConformance;
306+
return result;
307+
}
308+
272309
/// Returns true if this actor isolation is derived from an unapplied
273310
/// isolation parameter. When merging, we allow for this to be merged with a
274311
/// more specific isolation kind.
@@ -427,10 +464,13 @@ class SILIsolationInfo {
427464
Flag::UnappliedIsolatedAnyParameter};
428465
}
429466

430-
static SILIsolationInfo getGlobalActorIsolated(SILValue value,
431-
Type globalActorType) {
467+
static SILIsolationInfo getGlobalActorIsolated(
468+
SILValue value,
469+
Type globalActorType,
470+
ProtocolDecl *isolatedConformance = nullptr) {
432471
return {value, SILValue() /*no actor instance*/,
433-
ActorIsolation::forGlobalActor(globalActorType)};
472+
ActorIsolation::forGlobalActor(globalActorType),
473+
Options(), isolatedConformance};
434474
}
435475

436476
static SILIsolationInfo getGlobalActorIsolated(SILValue value,
@@ -442,8 +482,9 @@ class SILIsolationInfo {
442482
isolation.getGlobalActor());
443483
}
444484

445-
static SILIsolationInfo getTaskIsolated(SILValue value) {
446-
return {Kind::Task, value};
485+
static SILIsolationInfo getTaskIsolated(
486+
SILValue value, ProtocolDecl *isolatedConformance = nullptr) {
487+
return {Kind::Task, value, isolatedConformance};
447488
}
448489

449490
/// 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
@@ -288,6 +288,9 @@ static SILIsolationInfo getIsolationForCastConformances(
288288

289289
const auto &destLayout = destType.getExistentialLayout();
290290
for (auto proto : destLayout.getProtocols()) {
291+
if (proto->isMarkerProtocol())
292+
continue;
293+
291294
// If the source type already conforms to the protocol, we won't be looking
292295
// it up dynamically.
293296
if (!lookupConformance(sourceType, proto, /*allowMissing=*/false).isInvalid())
@@ -304,12 +307,12 @@ static SILIsolationInfo getIsolationForCastConformances(
304307
// Otherwise, it's task-isolated.
305308
if (functionIsolation && functionIsolation->isGlobalActor()) {
306309
return SILIsolationInfo::getGlobalActorIsolated(
307-
value, functionIsolation->getGlobalActor());
310+
value, functionIsolation->getGlobalActor(), proto);
308311
}
309312

310313
// Consider the cast to be task-isolated, because the runtime could find
311314
// a conformance that is isolated to the current context.
312-
return SILIsolationInfo::getTaskIsolated(value);
315+
return SILIsolationInfo::getTaskIsolated(value, proto);
313316
}
314317

315318
return {};
@@ -4087,6 +4090,9 @@ SILIsolationInfo
40874090
PartitionOpTranslator::getIsolationFromConformances(
40884091
SILValue value, ArrayRef<ProtocolConformanceRef> conformances) {
40894092
for (auto conformance: conformances) {
4093+
if (conformance.getProtocol()->isMarkerProtocol())
4094+
continue;
4095+
40904096
// If the conformance is a pack, recurse.
40914097
if (conformance.isPack()) {
40924098
auto pack = conformance.getPack();
@@ -4105,7 +4111,7 @@ PartitionOpTranslator::getIsolationFromConformances(
41054111
auto isolation = conformance.getConcrete()->getIsolation();
41064112
if (isolation.isGlobalActor()) {
41074113
return SILIsolationInfo::getGlobalActorIsolated(
4108-
value, isolation.getGlobalActor());
4114+
value, isolation.getGlobalActor(), conformance.getProtocol());
41094115
}
41104116

41114117
continue;
@@ -4120,7 +4126,8 @@ PartitionOpTranslator::getIsolationFromConformances(
41204126
if (sendableMetatype &&
41214127
lookupConformance(conformance.getType(), sendableMetatype,
41224128
/*allowMissing=*/false).isInvalid()) {
4123-
return SILIsolationInfo::getTaskIsolated(value);
4129+
return SILIsolationInfo::getTaskIsolated(value,
4130+
conformance.getProtocol());
41244131
}
41254132
}
41264133
}
@@ -4131,7 +4138,7 @@ PartitionOpTranslator::getIsolationFromConformances(
41314138
TranslationSemantics
41324139
PartitionOpTranslator::visitInitExistentialAddrInst(InitExistentialAddrInst *ieai) {
41334140
auto conformanceIsolationInfo = getIsolationFromConformances(
4134-
ieai->getResult(0), ieai->getConformances());
4141+
ieai, ieai->getConformances());
41354142

41364143
translateSILMultiAssign(ieai->getResults(),
41374144
makeOperandRefRange(ieai->getAllOperands()),
@@ -4143,7 +4150,7 @@ PartitionOpTranslator::visitInitExistentialAddrInst(InitExistentialAddrInst *iea
41434150
TranslationSemantics
41444151
PartitionOpTranslator::visitInitExistentialRefInst(InitExistentialRefInst *ieri) {
41454152
auto conformanceIsolationInfo = getIsolationFromConformances(
4146-
ieri->getResult(0), ieri->getConformances());
4153+
ieri, ieri->getConformances());
41474154

41484155
translateSILMultiAssign(ieri->getResults(),
41494156
makeOperandRefRange(ieri->getAllOperands()),
@@ -4155,7 +4162,7 @@ PartitionOpTranslator::visitInitExistentialRefInst(InitExistentialRefInst *ieri)
41554162
TranslationSemantics
41564163
PartitionOpTranslator::visitInitExistentialValueInst(InitExistentialValueInst *ievi) {
41574164
auto conformanceIsolationInfo = getIsolationFromConformances(
4158-
ievi->getResult(0), ievi->getConformances());
4165+
ievi, ievi->getConformances());
41594166

41604167
translateSILMultiAssign(ievi->getResults(),
41614168
makeOperandRefRange(ievi->getAllOperands()),

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,18 @@ class SendNeverSentDiagnosticEmitter {
13041304
~SendNeverSentDiagnosticEmitter() {
13051305
if (!emittedErrorDiagnostic) {
13061306
emitUnknownPatternError();
1307+
} else if (auto proto = isolationRegionInfo.getIsolationInfo()
1308+
.getIsolatedConformance()) {
1309+
// If the diagnostic comes from a (potentially) isolated conformance,
1310+
// add a note saying so and indicating where the isolated conformance
1311+
// can come in.
1312+
if (auto value = isolationRegionInfo.getIsolationInfo().getIsolatedValue()) {
1313+
if (auto loc = value.getLoc()) {
1314+
diagnoseNote(
1315+
loc, diag::regionbasedisolation_isolated_conformance_introduced,
1316+
proto);
1317+
}
1318+
}
13071319
}
13081320
}
13091321

@@ -1318,6 +1330,13 @@ class SendNeverSentDiagnosticEmitter {
13181330
}
13191331

13201332
std::optional<DiagnosticBehavior> getBehaviorLimit() const {
1333+
// If the failure is due to an isolated conformance, downgrade the error
1334+
// to a warning prior to Swift 7.
1335+
if (isolationRegionInfo.getIsolationInfo().getIsolatedConformance() &&
1336+
!sendingOperand->get()->getType().getASTType()->getASTContext().LangOpts
1337+
.isSwiftVersionAtLeast(7))
1338+
return DiagnosticBehavior::Warning;
1339+
13211340
return sendingOperand->get()->getType().getConcurrencyDiagnosticBehavior(
13221341
getOperand()->getFunction());
13231342
}
@@ -1595,8 +1614,7 @@ class SendNeverSentDiagnosticEmitter {
15951614
}
15961615

15971616
diagnoseNote(loc, diag::regionbasedisolation_named_send_nt_asynclet_capture,
1598-
name, descriptiveKindStr)
1599-
.limitBehaviorIf(getBehaviorLimit());
1617+
name, descriptiveKindStr);
16001618
}
16011619

16021620
void emitNamedIsolation(SILLocation loc, Identifier name,

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,10 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
961961
}
962962

963963
void SILIsolationInfo::printOptions(llvm::raw_ostream &os) const {
964+
if (isolatedConformance) {
965+
os << "isolated-conformance-to(" << isolatedConformance->getName() << ")";
966+
}
967+
964968
auto opts = getOptions();
965969
if (!opts)
966970
return;
@@ -1111,9 +1115,11 @@ void SILIsolationInfo::Profile(llvm::FoldingSetNodeID &id) const {
11111115
return;
11121116
case Task:
11131117
id.AddPointer(getIsolatedValue());
1118+
id.AddPointer(getIsolatedConformance());
11141119
return;
11151120
case Actor:
11161121
id.AddPointer(getIsolatedValue());
1122+
id.AddPointer(getIsolatedConformance());
11171123
getActorIsolation().Profile(id);
11181124
return;
11191125
}
@@ -1383,7 +1389,7 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
13831389
// If both innerInfo and other have the same isolation, we are obviously
13841390
// done. Just return innerInfo since we could return either.
13851391
if (innerInfo.hasSameIsolation(other))
1386-
return {innerInfo};
1392+
return {innerInfo.withMergedIsolatedConformance(other.getIsolatedConformance())};
13871393

13881394
// Ok, there is some difference in between innerInfo and other. Lets see if
13891395
// they are both actor instance isolated and if either are unapplied
@@ -1392,9 +1398,9 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
13921398
if (innerInfo.getActorIsolation().isActorInstanceIsolated() &&
13931399
other.getActorIsolation().isActorInstanceIsolated()) {
13941400
if (innerInfo.isUnappliedIsolatedAnyParameter())
1395-
return other;
1401+
return other.withMergedIsolatedConformance(innerInfo.getIsolatedConformance());
13961402
if (other.isUnappliedIsolatedAnyParameter())
1397-
return innerInfo;
1403+
return innerInfo.withMergedIsolatedConformance(other.getIsolatedConformance());
13981404
}
13991405

14001406
// Otherwise, they do not match... so return None to signal merge failure.

0 commit comments

Comments
 (0)