Skip to content

Commit 3e6b369

Browse files
committed
Model "unsafe" global actor isolation.
With "unsafe" global actor isolation, we only enforce actor isolation when interacting with other explicitly-isolated code. This allows some code to be annotated with, e.g., `@MainActor(unsafe)` so that users who opt into concurrency get proper diagnostics, but existing code does not change.
1 parent b77dcf9 commit 3e6b369

File tree

9 files changed

+92
-28
lines changed

9 files changed

+92
-28
lines changed

include/swift/AST/ActorIsolation.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ class ActorIsolation {
5656
/// The declaration is isolated to a global actor. It can refer to other
5757
/// entities with the same global actor.
5858
GlobalActor,
59+
/// The declaration is isolated to a global actor but with the "unsafe"
60+
/// annotation, which means that we only enforce the isolation if we're
61+
/// coming from something with specific isolation.
62+
GlobalActorUnsafe,
5963
};
6064

6165
private:
@@ -93,8 +97,9 @@ class ActorIsolation {
9397
return ActorIsolation(ActorInstance, actor);
9498
}
9599

96-
static ActorIsolation forGlobalActor(Type globalActor) {
97-
return ActorIsolation(GlobalActor, globalActor);
100+
static ActorIsolation forGlobalActor(Type globalActor, bool unsafe) {
101+
return ActorIsolation(
102+
unsafe ? GlobalActorUnsafe : GlobalActor, globalActor);
98103
}
99104

100105
Kind getKind() const { return kind; }
@@ -108,8 +113,12 @@ class ActorIsolation {
108113
return actor;
109114
}
110115

116+
bool isGlobalActor() const {
117+
return getKind() == GlobalActor || getKind() == GlobalActorUnsafe;
118+
}
119+
111120
Type getGlobalActor() const {
112-
assert(getKind() == GlobalActor);
121+
assert(isGlobalActor());
113122
return globalActor;
114123
}
115124

@@ -135,6 +144,7 @@ class ActorIsolation {
135144
return lhs.actor == rhs.actor;
136145

137146
case GlobalActor:
147+
case GlobalActorUnsafe:
138148
return areTypesEqual(lhs.globalActor, rhs.globalActor);
139149
}
140150
}

lib/AST/Decl.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8085,7 +8085,8 @@ ActorIsolation swift::getActorIsolationOfContext(DeclContext *dc) {
80858085
return ActorIsolation::forIndependent(ActorIndependentKind::Safe);
80868086

80878087
case ClosureActorIsolation::GlobalActor: {
8088-
return ActorIsolation::forGlobalActor(isolation.getGlobalActor());
8088+
return ActorIsolation::forGlobalActor(
8089+
isolation.getGlobalActor(), /*unsafe=*/false);
80898090
}
80908091

80918092
case ClosureActorIsolation::ActorInstance: {

lib/AST/DiagnosticEngine.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,7 @@ static void formatDiagnosticArgument(StringRef Modifier,
690690
break;
691691

692692
case ActorIsolation::GlobalActor:
693+
case ActorIsolation::GlobalActorUnsafe:
693694
Out << "global actor " << FormatOpts.OpeningQuotationMark
694695
<< isolation.getGlobalActor().getString()
695696
<< FormatOpts.ClosingQuotationMark << "-isolated";

lib/AST/TypeCheckRequests.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1519,6 +1519,7 @@ bool ActorIsolation::requiresSubstitution() const {
15191519
return false;
15201520

15211521
case GlobalActor:
1522+
case GlobalActorUnsafe:
15221523
return getGlobalActor()->hasTypeParameter();
15231524
}
15241525
llvm_unreachable("unhandled actor isolation kind!");
@@ -1533,7 +1534,9 @@ ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const {
15331534
return *this;
15341535

15351536
case GlobalActor:
1536-
return forGlobalActor(getGlobalActor().subst(subs));
1537+
case GlobalActorUnsafe:
1538+
return forGlobalActor(
1539+
getGlobalActor().subst(subs), kind == GlobalActorUnsafe);
15371540
}
15381541
llvm_unreachable("unhandled actor isolation kind!");
15391542
}
@@ -1558,8 +1561,12 @@ void swift::simple_display(
15581561
break;
15591562

15601563
case ActorIsolation::GlobalActor:
1564+
case ActorIsolation::GlobalActorUnsafe:
15611565
out << "actor-isolated to global actor "
15621566
<< state.getGlobalActor().getString();
1567+
1568+
if (state == ActorIsolation::GlobalActorUnsafe)
1569+
out << "(unsafe)";
15631570
break;
15641571
}
15651572
}

lib/SILGen/SILGenApply.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4303,6 +4303,7 @@ Optional<SILValue> SILGenFunction::emitLoadActorExecutorForCallee(
43034303
}
43044304

43054305
case ActorIsolation::GlobalActor:
4306+
case ActorIsolation::GlobalActorUnsafe:
43064307
return emitLoadGlobalActorExecutor(actorIso.getGlobalActor());
43074308
}
43084309
}

lib/SILGen/SILGenProlog.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
472472
case ActorIsolation::Unspecified:
473473
case ActorIsolation::Independent:
474474
case ActorIsolation::IndependentUnsafe:
475+
case ActorIsolation::GlobalActorUnsafe:
475476
break;
476477

477478
case ActorIsolation::ActorInstance: {

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,10 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
567567
// Actor-independent have no restrictions on their access.
568568
return forUnrestricted();
569569

570+
case ActorIsolation::GlobalActorUnsafe:
571+
// TODO: Capture GlobalActorUnsafe so we can do more checking.
572+
return forUnrestricted();
573+
570574
case ActorIsolation::Unspecified:
571575
return isAccessibleAcrossActors ? forUnrestricted() : forUnsafe();
572576
}
@@ -1361,8 +1365,10 @@ namespace {
13611365
return isolation;
13621366

13631367
case ActorIsolation::GlobalActor:
1368+
case ActorIsolation::GlobalActorUnsafe:
13641369
return ActorIsolation::forGlobalActor(
1365-
constDC->mapTypeIntoContext(isolation.getGlobalActor()));
1370+
constDC->mapTypeIntoContext(isolation.getGlobalActor()),
1371+
isolation == ActorIsolation::GlobalActorUnsafe);
13661372
}
13671373
};
13681374

@@ -1467,7 +1473,8 @@ namespace {
14671473
// Check whether we are within the same isolation context, in which
14681474
// case there is nothing further to check,
14691475
auto contextIsolation = getInnermostIsolatedContext(declContext);
1470-
if (contextIsolation == ActorIsolation::forGlobalActor(globalActor)) {
1476+
if (contextIsolation.isGlobalActor() &&
1477+
contextIsolation.getGlobalActor()->isEqual(globalActor)) {
14711478
return false;
14721479
}
14731480

@@ -1490,7 +1497,8 @@ namespace {
14901497
noteIsolatedActorMember(value);
14911498
return true;
14921499

1493-
case ActorIsolation::GlobalActor: {
1500+
case ActorIsolation::GlobalActor:
1501+
case ActorIsolation::GlobalActorUnsafe: {
14941502
// Check if this decl reference is the callee of the enclosing Apply,
14951503
// making it OK as an implicitly async call.
14961504
if (inspectForImplicitlyAsync())
@@ -1829,6 +1837,7 @@ namespace {
18291837
}
18301838

18311839
case ActorIsolation::GlobalActor:
1840+
case ActorIsolation::GlobalActorUnsafe:
18321841
// Check for implicit async.
18331842
if (auto result = checkImplicitlyAsync())
18341843
return *result;
@@ -1880,7 +1889,8 @@ namespace {
18801889
case ActorIsolation::Unspecified:
18811890
return ClosureActorIsolation::forIndependent();
18821891

1883-
case ActorIsolation::GlobalActor: {
1892+
case ActorIsolation::GlobalActor:
1893+
case ActorIsolation::GlobalActorUnsafe: {
18841894
Type globalActorType = closure->mapTypeIntoContext(
18851895
parentIsolation.getGlobalActor()->mapTypeOutOfContext());
18861896
return ClosureActorIsolation::forGlobalActor(globalActorType);
@@ -2036,14 +2046,8 @@ static Optional<ActorIsolation> getIsolationFromAttributes(
20362046
diag::global_actor_non_unsafe_init, globalActorType);
20372047
}
20382048

2039-
// TODO: Model as unsafe from the actor-isolation perspective, which
2040-
// disables all checking. We probably want to model this as a separate kind
2041-
// of actor isolation to emit warnings.
2042-
if (isUnsafe)
2043-
return ActorIsolation::forIndependent(ActorIndependentKind::Unsafe);
2044-
20452049
return ActorIsolation::forGlobalActor(
2046-
globalActorType->mapTypeOutOfContext());
2050+
globalActorType->mapTypeOutOfContext(), isUnsafe);
20472051
}
20482052

20492053
llvm_unreachable("Forgot about an attribute?");
@@ -2113,7 +2117,8 @@ static Optional<ActorIsolation> getIsolationFromWitnessedRequirements(
21132117
case ActorIsolation::Unspecified:
21142118
return true;
21152119

2116-
case ActorIsolation::GlobalActor: {
2120+
case ActorIsolation::GlobalActor:
2121+
case ActorIsolation::GlobalActorUnsafe: {
21172122
// Substitute into the global actor type.
21182123
auto conformance = std::get<0>(isolated);
21192124
auto requirementSubs = SubstitutionMap::getProtocolSubstitutions(
@@ -2124,7 +2129,8 @@ static Optional<ActorIsolation> getIsolationFromWitnessedRequirements(
21242129
return true;
21252130

21262131
// Update the global actor type, now that we've done this substitution.
2127-
std::get<1>(isolated) = ActorIsolation::forGlobalActor(globalActor);
2132+
std::get<1>(isolated) = ActorIsolation::forGlobalActor(
2133+
globalActor, isolation == ActorIsolation::GlobalActorUnsafe);
21282134
return false;
21292135
}
21302136
}
@@ -2188,10 +2194,13 @@ ActorIsolation ActorIsolationRequest::evaluate(
21882194
ActorIndependentKind::Safe, /*IsImplicit=*/true));
21892195
break;
21902196

2191-
case ActorIsolation::GlobalActor: {
2197+
case ActorIsolation::GlobalActor:
2198+
case ActorIsolation::GlobalActorUnsafe: {
21922199
auto typeExpr = TypeExpr::createImplicit(inferred.getGlobalActor(), ctx);
21932200
auto attr = CustomAttr::create(
21942201
ctx, SourceLoc(), typeExpr, /*implicit=*/true);
2202+
if (inferred == ActorIsolation::GlobalActorUnsafe)
2203+
attr->setArgIsUnsafe(true);
21952204
value->getAttrs().add(attr);
21962205
break;
21972206
}
@@ -2315,15 +2324,40 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
23152324
// If the overridden declaration is @actorIndependent(unsafe) and the
23162325
// overriding declaration has been placed in a global actor, allow it.
23172326
if (overriddenIsolation.getKind() == ActorIsolation::IndependentUnsafe &&
2318-
isolation.getKind() == ActorIsolation::GlobalActor)
2327+
isolation.isGlobalActor())
23192328
return;
23202329

23212330
// If the overridden declaration is from Objective-C with no actor annotation,
23222331
// and the overriding declaration has been placed in a global actor, allow it.
23232332
if (overridden->hasClangNode() && !overriddenIsolation &&
2324-
isolation.getKind() == ActorIsolation::GlobalActor)
2333+
isolation.isGlobalActor())
23252334
return;
23262335

2336+
// If the overridden declaration uses an unsafe global actor, we can do
2337+
// anything except be actor-isolated or have a different global actor.
2338+
if (overriddenIsolation == ActorIsolation::GlobalActorUnsafe) {
2339+
switch (isolation) {
2340+
case ActorIsolation::Independent:
2341+
case ActorIsolation::IndependentUnsafe:
2342+
case ActorIsolation::Unspecified:
2343+
return;
2344+
2345+
case ActorIsolation::ActorInstance:
2346+
// Diagnose below.
2347+
break;
2348+
2349+
case ActorIsolation::GlobalActor:
2350+
case ActorIsolation::GlobalActorUnsafe:
2351+
// The global actors don't match; diagnose it.
2352+
if (overriddenIsolation.getGlobalActor()->isEqual(
2353+
isolation.getGlobalActor()))
2354+
return;
2355+
2356+
// Diagnose below.
2357+
break;
2358+
}
2359+
}
2360+
23272361
// Isolation mismatch. Diagnose it.
23282362
value->diagnose(
23292363
diag::actor_isolation_override_mismatch, isolation,

lib/Sema/TypeCheckConcurrency.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,22 +74,24 @@ class ActorIsolationRestriction {
7474
/// There is no restriction on references to the given declaration.
7575
Unrestricted,
7676

77-
/// Access to the declaration is unsafe in a concurrent context.
77+
/// Access to the declaration is unsafe in any concurrent context.
7878
Unsafe,
7979

8080
/// References to this entity are allowed from anywhere, but doing so
81-
/// may cross an actor boundary if it is not on \c self.
81+
/// may cross an actor boundary if it is not from within the same actor's
82+
/// isolation domain.
8283
CrossActorSelf,
8384

84-
/// References to this member of an actor are only permitted on 'self'.
85+
/// References to this member of an actor are only permitted from within
86+
/// the actor's isolation domain.
8587
ActorSelf,
8688

8789
/// References to a declaration that is part of a global actor are only
8890
/// permitted from other declarations with that same global actor.
8991
GlobalActor,
9092

91-
/// Referneces to this entity are allowed from anywhere, but doing so may
92-
/// cross an actor bounder if it is not from the same global actor.
93+
/// References to this entity are allowed from anywhere, but doing so may
94+
/// cross an actor boundary if it is not from the same global actor.
9395
CrossGlobalActor,
9496
};
9597

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2785,10 +2785,15 @@ bool ConformanceChecker::checkActorIsolation(
27852785

27862786
// Check whether the requirement requires some particular actor isolation.
27872787
Type requirementGlobalActor;
2788+
bool requirementIsUnsafe = false;
27882789
switch (auto requirementIsolation = getActorIsolation(requirement)) {
27892790
case ActorIsolation::ActorInstance:
27902791
llvm_unreachable("There are not actor protocols");
27912792

2793+
case ActorIsolation::GlobalActorUnsafe:
2794+
requirementIsUnsafe = true;
2795+
LLVM_FALLTHROUGH;
2796+
27922797
case ActorIsolation::GlobalActor: {
27932798
auto requirementSubs = SubstitutionMap::getProtocolSubstitutions(
27942799
Proto, Adoptee, ProtocolConformanceRef(Conformance));
@@ -2846,9 +2851,11 @@ bool ConformanceChecker::checkActorIsolation(
28462851

28472852
// If the requirement has a global actor but the witness does not, we have
28482853
// an isolation error.
2849-
//
2850-
// FIXME: Within a module, this will be an inference rule.
28512854
if (requirementGlobalActor && !witnessGlobalActor) {
2855+
// If the requirement's global actor was "unsafe", we allow this.
2856+
if (requirementIsUnsafe)
2857+
return false;
2858+
28522859
if (isCrossActor) {
28532860
return diagnoseNonConcurrentTypesInReference(
28542861
witness, DC, witness->getLoc(), ConcurrentReferenceKind::CrossActor);

0 commit comments

Comments
 (0)