Skip to content

Commit fdc6260

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.
1 parent 4c77d71 commit fdc6260

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
@@ -4457,14 +4457,11 @@ NOTE(note_add_nonisolated_to_decl,none,
44574457
(DeclName, DescriptiveDeclKind))
44584458
NOTE(note_add_async_and_throws_to_decl,none,
44594459
"mark the protocol requirement %0 '%select{|async|throws|async throws}1' "
4460-
"in order witness it with 'distributed' function declared in distributed actor %2",
4461-
(DeclName, unsigned, DeclName))
4460+
"to allow actor-isolated conformances",
4461+
(DeclName, unsigned))
44624462
NOTE(note_add_distributed_to_decl,none,
4463-
"add 'distributed' to %0 to make this %1 witness the protocol requirement",
4463+
"add 'distributed' to %0 to make this %1 satisfy the protocol requirement",
44644464
(DeclName, DescriptiveDeclKind))
4465-
NOTE(note_distributed_requirement_defined_here,none,
4466-
"distributed instance method requirement %0 declared here",
4467-
(DeclName))
44684465
NOTE(note_add_globalactor_to_function,none,
44694466
"add '@%0' to make %1 %2 part of global actor %3",
44704467
(StringRef, DescriptiveDeclKind, DeclName, Type))
@@ -4658,19 +4655,9 @@ WARNING(shared_mutable_state_access,none,
46584655
"reference to %0 %1 is not concurrency-safe because it involves "
46594656
"shared mutable state", (DescriptiveDeclKind, DeclName))
46604657
ERROR(actor_isolated_witness,none,
4661-
"actor-isolated %0 %1 cannot be used to satisfy a protocol requirement",
4662-
(DescriptiveDeclKind, DeclName))
4663-
ERROR(distributed_actor_isolated_witness,none,
4664-
"distributed actor-isolated %0 %1 cannot be used to satisfy a protocol requirement",
4665-
(DescriptiveDeclKind, DeclName))
4666-
ERROR(global_actor_isolated_witness,none,
4667-
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
4668-
"requirement from protocol %3",
4669-
(DescriptiveDeclKind, DeclName, Type, Identifier))
4670-
ERROR(global_actor_isolated_requirement_witness_conflict,none,
4671-
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
4672-
"requirement from protocol %3 isolated to global actor %4",
4673-
(DescriptiveDeclKind, DeclName, Type, Identifier, Type))
4658+
"%select{|distributed }0%1 %2 %3 cannot be used to satisfy %4 protocol "
4659+
"requirement",
4660+
(bool, ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
46744661
ERROR(actor_cannot_conform_to_global_actor_protocol,none,
46754662
"actor %0 cannot conform to global actor isolated protocol %1",
46764663
(Type, Type))

lib/Sema/TypeCheckConcurrency.cpp

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

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

15411409
/// Determine whether this declaration is always accessed asynchronously.
1542-
static bool isAsyncDecl(ConcreteDeclRef declRef) {
1410+
bool swift::isAsyncDecl(ConcreteDeclRef declRef) {
15431411
auto decl = declRef.getDecl();
15441412

15451413
// An async function is asynchronously accessed.
@@ -4792,6 +4660,10 @@ static ActorIsolation getActorIsolationForReference(
47924660

47934661
// A constructor that is not explicitly 'nonisolated' is treated as
47944662
// isolated from the perspective of the referencer.
4663+
//
4664+
// FIXME: The current state is that even `nonisolated` initializers are
4665+
// externally treated as being on the actor, even though this model isn't
4666+
// consistent. We'll fix it later.
47954667
if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
47964668
// If the constructor is part of an actor, references to it are treated
47974669
// as needing to enter the actor.
@@ -4826,7 +4698,7 @@ static ActorIsolation getActorIsolationForReference(
48264698
}
48274699

48284700
/// Determine whether this declaration always throws.
4829-
static bool isThrowsDecl(ConcreteDeclRef declRef) {
4701+
bool swift::isThrowsDecl(ConcreteDeclRef declRef) {
48304702
auto decl = declRef.getDecl();
48314703

48324704
// An async function is asynchronously accessed.
@@ -4928,14 +4800,34 @@ ActorReferenceResult ActorReferenceResult::forReference(
49284800
ConcreteDeclRef declRef, SourceLoc declRefLoc, const DeclContext *fromDC,
49294801
Optional<VarRefUseEnv> useKind,
49304802
Optional<ReferencedActor> actorInstance) {
4803+
return forReference(declRef, declRefLoc, fromDC, useKind, actorInstance,
4804+
getInnermostIsolatedContext(fromDC));
4805+
}
4806+
4807+
// Determine if two actor isolation contexts are considered to be equivalent.
4808+
static bool equivalentIsolationContexts(
4809+
const ActorIsolation &lhs, const ActorIsolation &rhs) {
4810+
if (lhs == rhs)
4811+
return true;
4812+
4813+
if (lhs == ActorIsolation::ActorInstance &&
4814+
rhs == ActorIsolation::ActorInstance &&
4815+
lhs.isDistributedActor() == rhs.isDistributedActor())
4816+
return true;
4817+
4818+
return false;
4819+
}
4820+
4821+
ActorReferenceResult ActorReferenceResult::forReference(
4822+
ConcreteDeclRef declRef, SourceLoc declRefLoc, const DeclContext *fromDC,
4823+
Optional<VarRefUseEnv> useKind,
4824+
Optional<ReferencedActor> actorInstance,
4825+
ActorIsolation contextIsolation) {
49314826
// Compute the isolation of the declaration, adjusted for references.
49324827
auto declIsolation = getActorIsolationForReference(declRef.getDecl(), fromDC);
49334828
if (auto subs = declRef.getSubstitutions())
49344829
declIsolation = declIsolation.subst(subs);
49354830

4936-
// Compute the isolation of the context.
4937-
auto contextIsolation = getInnermostIsolatedContext(fromDC);
4938-
49394831
// When the declaration is not actor-isolated, it can always be accessed
49404832
// directly.
49414833
if (!declIsolation.isActorIsolated()) {
@@ -4955,7 +4847,7 @@ ActorReferenceResult ActorReferenceResult::forReference(
49554847
// If this instance is isolated, we're in the same concurrency domain.
49564848
if (actorInstance->isIsolated())
49574849
return forSameConcurrencyDomain(declIsolation);
4958-
} else if (contextIsolation == declIsolation) {
4850+
} else if (equivalentIsolationContexts(declIsolation, contextIsolation)) {
49594851
// The context isolation matches, so we are in the same concurrency
49604852
// domain.
49614853
return forSameConcurrencyDomain(declIsolation);
@@ -5008,13 +4900,21 @@ ActorReferenceResult ActorReferenceResult::forReference(
50084900
options |= Flags::AsyncPromotion;
50094901

50104902
// If the declaration is isolated to a distributed actor and we are not
5011-
// guaranteed to be on the same node, adjustments for a distributed call.
5012-
if (declIsolation.isDistributedActor() &&
5013-
!(actorInstance && actorInstance->isKnownToBeLocal())) {
5014-
options |= Flags::Distributed;
4903+
// guaranteed to be on the same node, make adjustments distributed
4904+
// access.
4905+
if (declIsolation.isDistributedActor()) {
4906+
bool needsDistributed;
4907+
if (actorInstance)
4908+
needsDistributed = !actorInstance->isKnownToBeLocal();
4909+
else
4910+
needsDistributed = !contextIsolation.isDistributedActor();
50154911

5016-
if (!isThrowsDecl(declRef))
5017-
options |= Flags::ThrowsPromotion;
4912+
if (needsDistributed) {
4913+
options |= Flags::Distributed;
4914+
4915+
if (!isThrowsDecl(declRef))
4916+
options |= Flags::ThrowsPromotion;
4917+
}
50184918
}
50194919

50204920
return forEntersActor(declIsolation, options);

0 commit comments

Comments
 (0)