Skip to content

Commit 14acd6a

Browse files
committed
Reimplement conformance isolation checking using the actor reference logic.
Reimplement the final client of ActorIsolationRestriction, conformance isolation checking, to base it on the new "actor reference" logic. Centralize the diagnostics emission so we have a single place where we emit the primary diagnostic (which is heavily customized based on actor isolation/distributed/etc.) and any relevant notes to make adjustments to the witness and/or requirement, e.g., adding 'distributed', 'async', 'throws', etc. Improve the diagnostics slightly by providing Fix-Its when suggesting that we add "async" and/or "throws". With the last client of ActorIsolationRestriction gone, remove it entirely. (cherry picked from commit fdc6260)
1 parent 00cae5a commit 14acd6a

12 files changed

+311
-651
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4470,14 +4470,11 @@ NOTE(note_add_nonisolated_to_decl,none,
44704470
(DeclName, DescriptiveDeclKind))
44714471
NOTE(note_add_async_and_throws_to_decl,none,
44724472
"mark the protocol requirement %0 '%select{|async|throws|async throws}1' "
4473-
"in order witness it with 'distributed' function declared in distributed actor %2",
4474-
(DeclName, unsigned, DeclName))
4473+
"to allow actor-isolated conformances",
4474+
(DeclName, unsigned))
44754475
NOTE(note_add_distributed_to_decl,none,
4476-
"add 'distributed' to %0 to make this %1 witness the protocol requirement",
4476+
"add 'distributed' to %0 to make this %1 satisfy the protocol requirement",
44774477
(DeclName, DescriptiveDeclKind))
4478-
NOTE(note_distributed_requirement_defined_here,none,
4479-
"distributed instance method requirement %0 declared here",
4480-
(DeclName))
44814478
NOTE(note_add_globalactor_to_function,none,
44824479
"add '@%0' to make %1 %2 part of global actor %3",
44834480
(StringRef, DescriptiveDeclKind, DeclName, Type))
@@ -4675,19 +4672,9 @@ WARNING(shared_mutable_state_access,none,
46754672
"reference to %0 %1 is not concurrency-safe because it involves "
46764673
"shared mutable state", (DescriptiveDeclKind, DeclName))
46774674
ERROR(actor_isolated_witness,none,
4678-
"actor-isolated %0 %1 cannot be used to satisfy a protocol requirement",
4679-
(DescriptiveDeclKind, DeclName))
4680-
ERROR(distributed_actor_isolated_witness,none,
4681-
"distributed actor-isolated %0 %1 cannot be used to satisfy a protocol requirement",
4682-
(DescriptiveDeclKind, DeclName))
4683-
ERROR(global_actor_isolated_witness,none,
4684-
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
4685-
"requirement from protocol %3",
4686-
(DescriptiveDeclKind, DeclName, Type, Identifier))
4687-
ERROR(global_actor_isolated_requirement_witness_conflict,none,
4688-
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
4689-
"requirement from protocol %3 isolated to global actor %4",
4690-
(DescriptiveDeclKind, DeclName, Type, Identifier, Type))
4675+
"%select{|distributed }0%1 %2 %3 cannot be used to satisfy %4 protocol "
4676+
"requirement",
4677+
(bool, ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
46914678
ERROR(actor_cannot_conform_to_global_actor_protocol,none,
46924679
"actor %0 cannot conform to global actor isolated protocol %1",
46934680
(Type, Type))

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 44 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -447,138 +447,6 @@ bool swift::isLetAccessibleAnywhere(const ModuleDecl *fromModule,
447447
return varIsSafeAcrossActors(fromModule, let, isolation);
448448
}
449449

450-
/// Determine the isolation rules for a given declaration.
451-
ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
452-
ConcreteDeclRef declRef, const DeclContext *fromDC, bool fromExpression) {
453-
auto decl = declRef.getDecl();
454-
455-
switch (decl->getKind()) {
456-
case DeclKind::AssociatedType:
457-
case DeclKind::Class:
458-
case DeclKind::Enum:
459-
case DeclKind::Extension:
460-
case DeclKind::GenericTypeParam:
461-
case DeclKind::OpaqueType:
462-
case DeclKind::Protocol:
463-
case DeclKind::Struct:
464-
case DeclKind::TypeAlias:
465-
// Types are always available.
466-
return forUnrestricted();
467-
468-
case DeclKind::EnumCase:
469-
case DeclKind::EnumElement:
470-
// Type-level entities don't require isolation.
471-
return forUnrestricted();
472-
473-
case DeclKind::IfConfig:
474-
case DeclKind::Import:
475-
case DeclKind::InfixOperator:
476-
case DeclKind::MissingMember:
477-
case DeclKind::Module:
478-
case DeclKind::PatternBinding:
479-
case DeclKind::PostfixOperator:
480-
case DeclKind::PoundDiagnostic:
481-
case DeclKind::PrecedenceGroup:
482-
case DeclKind::PrefixOperator:
483-
case DeclKind::TopLevelCode:
484-
// Non-value entities don't require isolation.
485-
return forUnrestricted();
486-
487-
case DeclKind::Destructor:
488-
// Destructors don't require isolation.
489-
return forUnrestricted();
490-
491-
case DeclKind::Param:
492-
case DeclKind::Var:
493-
case DeclKind::Accessor:
494-
case DeclKind::Constructor:
495-
case DeclKind::Func:
496-
case DeclKind::Subscript: {
497-
// Local captures are checked separately.
498-
if (cast<ValueDecl>(decl)->isLocalCapture())
499-
return forUnrestricted();
500-
501-
auto isolation = getActorIsolation(cast<ValueDecl>(decl));
502-
503-
// 'let' declarations are immutable, so some of them can be accessed across
504-
// actors.
505-
bool isAccessibleAcrossActors = false;
506-
if (auto var = dyn_cast<VarDecl>(decl)) {
507-
if (varIsSafeAcrossActors(fromDC->getParentModule(), var, isolation))
508-
isAccessibleAcrossActors = true;
509-
}
510-
511-
// A function that provides an asynchronous context has no restrictions
512-
// on its access.
513-
//
514-
// FIXME: technically, synchronous functions are allowed to be cross-actor.
515-
// The call-sites are just conditionally async based on where they appear
516-
// (outside or inside the actor). This suggests that the implicitly-async
517-
// concept could be merged into the CrossActorSelf concept.
518-
if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
519-
if (func->isAsyncContext())
520-
isAccessibleAcrossActors = true;
521-
}
522-
523-
// Similarly, a computed property or subscript that has an 'async' getter
524-
// provides an asynchronous context, and has no restrictions.
525-
if (auto storageDecl = dyn_cast<AbstractStorageDecl>(decl)) {
526-
if (auto effectfulGetter = storageDecl->getEffectfulGetAccessor())
527-
if (effectfulGetter->hasAsync())
528-
isAccessibleAcrossActors = true;
529-
}
530-
531-
// Determine the actor isolation of the given declaration.
532-
switch (isolation) {
533-
case ActorIsolation::ActorInstance:
534-
// Protected actor instance members can only be accessed on 'self'.
535-
return forActorSelf(isolation.getActor(),
536-
isAccessibleAcrossActors || isa<ConstructorDecl>(decl));
537-
538-
case ActorIsolation::GlobalActorUnsafe:
539-
case ActorIsolation::GlobalActor: {
540-
// A global-actor-isolated function referenced within an expression
541-
// carries the global actor into its function type. The actual
542-
// reference to the function is therefore not restricted, because the
543-
// call to the function is.
544-
if (fromExpression && isa<AbstractFunctionDecl>(decl))
545-
return forUnrestricted();
546-
547-
Type actorType = isolation.getGlobalActor();
548-
if (auto subs = declRef.getSubstitutions())
549-
actorType = actorType.subst(subs);
550-
551-
return forGlobalActor(actorType, isAccessibleAcrossActors,
552-
isolation == ActorIsolation::GlobalActorUnsafe);
553-
}
554-
555-
case ActorIsolation::Independent:
556-
// While some synchronous actor inits are not isolated they still need
557-
// cross-actor restrictions (e.g., for Sendable) for safety.
558-
if (auto *ctor = dyn_cast<ConstructorDecl>(decl))
559-
if (auto *parent = ctor->getParent()->getSelfClassDecl())
560-
if (parent->isAnyActor())
561-
return forActorSelf(parent, /*isCrossActor=*/true);
562-
563-
// `nonisolated let` members are cross-actor as well.
564-
if (auto var = dyn_cast<VarDecl>(decl)) {
565-
if (var->isInstanceMember() && var->isLet()) {
566-
if (auto parent = var->getDeclContext()->getSelfClassDecl()) {
567-
if (parent->isActor() && !parent->isDistributedActor())
568-
return forActorSelf(parent, /*isCrossActor=*/true);
569-
}
570-
}
571-
}
572-
573-
return forUnrestricted();
574-
575-
case ActorIsolation::Unspecified:
576-
return isAccessibleAcrossActors ? forUnrestricted() : forUnsafe();
577-
}
578-
}
579-
}
580-
}
581-
582450
namespace {
583451
/// Describes the important parts of a partial apply thunk.
584452
struct PartialApplyThunkInfo {
@@ -1593,7 +1461,7 @@ static ActorIsolation getInnermostIsolatedContext(const DeclContext *dc) {
15931461
}
15941462

15951463
/// Determine whether this declaration is always accessed asynchronously.
1596-
static bool isAsyncDecl(ConcreteDeclRef declRef) {
1464+
bool swift::isAsyncDecl(ConcreteDeclRef declRef) {
15971465
auto decl = declRef.getDecl();
15981466

15991467
// An async function is asynchronously accessed.
@@ -4862,6 +4730,10 @@ static ActorIsolation getActorIsolationForReference(
48624730

48634731
// A constructor that is not explicitly 'nonisolated' is treated as
48644732
// isolated from the perspective of the referencer.
4733+
//
4734+
// FIXME: The current state is that even `nonisolated` initializers are
4735+
// externally treated as being on the actor, even though this model isn't
4736+
// consistent. We'll fix it later.
48654737
if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
48664738
// If the constructor is part of an actor, references to it are treated
48674739
// as needing to enter the actor.
@@ -4896,7 +4768,7 @@ static ActorIsolation getActorIsolationForReference(
48964768
}
48974769

48984770
/// Determine whether this declaration always throws.
4899-
static bool isThrowsDecl(ConcreteDeclRef declRef) {
4771+
bool swift::isThrowsDecl(ConcreteDeclRef declRef) {
49004772
auto decl = declRef.getDecl();
49014773

49024774
// An async function is asynchronously accessed.
@@ -4998,14 +4870,34 @@ ActorReferenceResult ActorReferenceResult::forReference(
49984870
ConcreteDeclRef declRef, SourceLoc declRefLoc, const DeclContext *fromDC,
49994871
Optional<VarRefUseEnv> useKind,
50004872
Optional<ReferencedActor> actorInstance) {
4873+
return forReference(declRef, declRefLoc, fromDC, useKind, actorInstance,
4874+
getInnermostIsolatedContext(fromDC));
4875+
}
4876+
4877+
// Determine if two actor isolation contexts are considered to be equivalent.
4878+
static bool equivalentIsolationContexts(
4879+
const ActorIsolation &lhs, const ActorIsolation &rhs) {
4880+
if (lhs == rhs)
4881+
return true;
4882+
4883+
if (lhs == ActorIsolation::ActorInstance &&
4884+
rhs == ActorIsolation::ActorInstance &&
4885+
lhs.isDistributedActor() == rhs.isDistributedActor())
4886+
return true;
4887+
4888+
return false;
4889+
}
4890+
4891+
ActorReferenceResult ActorReferenceResult::forReference(
4892+
ConcreteDeclRef declRef, SourceLoc declRefLoc, const DeclContext *fromDC,
4893+
Optional<VarRefUseEnv> useKind,
4894+
Optional<ReferencedActor> actorInstance,
4895+
ActorIsolation contextIsolation) {
50014896
// Compute the isolation of the declaration, adjusted for references.
50024897
auto declIsolation = getActorIsolationForReference(declRef.getDecl(), fromDC);
50034898
if (auto subs = declRef.getSubstitutions())
50044899
declIsolation = declIsolation.subst(subs);
50054900

5006-
// Compute the isolation of the context.
5007-
auto contextIsolation = getInnermostIsolatedContext(fromDC);
5008-
50094901
// When the declaration is not actor-isolated, it can always be accessed
50104902
// directly.
50114903
if (!declIsolation.isActorIsolated()) {
@@ -5025,7 +4917,7 @@ ActorReferenceResult ActorReferenceResult::forReference(
50254917
// If this instance is isolated, we're in the same concurrency domain.
50264918
if (actorInstance->isIsolated())
50274919
return forSameConcurrencyDomain(declIsolation);
5028-
} else if (contextIsolation == declIsolation) {
4920+
} else if (equivalentIsolationContexts(declIsolation, contextIsolation)) {
50294921
// The context isolation matches, so we are in the same concurrency
50304922
// domain.
50314923
return forSameConcurrencyDomain(declIsolation);
@@ -5078,13 +4970,21 @@ ActorReferenceResult ActorReferenceResult::forReference(
50784970
options |= Flags::AsyncPromotion;
50794971

50804972
// If the declaration is isolated to a distributed actor and we are not
5081-
// guaranteed to be on the same node, adjustments for a distributed call.
5082-
if (declIsolation.isDistributedActor() &&
5083-
!(actorInstance && actorInstance->isKnownToBeLocal())) {
5084-
options |= Flags::Distributed;
4973+
// guaranteed to be on the same node, make adjustments distributed
4974+
// access.
4975+
if (declIsolation.isDistributedActor()) {
4976+
bool needsDistributed;
4977+
if (actorInstance)
4978+
needsDistributed = !actorInstance->isKnownToBeLocal();
4979+
else
4980+
needsDistributed = !contextIsolation.isDistributedActor();
50854981

5086-
if (!isThrowsDecl(declRef))
5087-
options |= Flags::ThrowsPromotion;
4982+
if (needsDistributed) {
4983+
options |= Flags::Distributed;
4984+
4985+
if (!isThrowsDecl(declRef))
4986+
options |= Flags::ThrowsPromotion;
4987+
}
50884988
}
50894989

50904990
return forEntersActor(declIsolation, options);

0 commit comments

Comments
 (0)