Skip to content

[Concurrency] Implement global actor isolation checking for conformances #34285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4194,6 +4194,18 @@ ERROR(actor_isolated_witness_could_be_async_handler,none,
"actor-isolated %0 %1 cannot be used to satisfy a protocol requirement; "
"did you mean to make it an asychronous handler?",
(DescriptiveDeclKind, DeclName))
ERROR(global_actor_isolated_requirement,none,
"%0 %1 must be isolated to the global actor %2 to satisfy corresponding "
"requirement from protocol %3",
(DescriptiveDeclKind, DeclName, Type, Identifier))
ERROR(global_actor_isolated_witness,none,
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
"requirement from protocol %3",
(DescriptiveDeclKind, DeclName, Type, Identifier))
ERROR(global_actor_isolated_requirement_witness_conflict,none,
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
"requirement from protocol %3 isolated to global actor %4",
(DescriptiveDeclKind, DeclName, Type, Identifier, Type))

ERROR(actorisolated_let,none,
"'@actorIsolated' is meaningless on 'let' declarations because "
Expand Down
141 changes: 109 additions & 32 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2656,6 +2656,114 @@ static void emitDeclaredHereIfNeeded(DiagnosticEngine &diags,
diags.diagnose(value, diag::decl_declared_here, value->getName());
}

bool ConformanceChecker::checkActorIsolation(
ValueDecl *requirement, ValueDecl *witness) {
// Ensure that the witness is not actor-isolated in a manner that makes it
// unsuitable as a witness.
Type witnessGlobalActor;
switch (auto witnessRestriction =
ActorIsolationRestriction::forDeclaration(witness)) {
case ActorIsolationRestriction::ActorSelf: {
// Actor-isolated witnesses cannot conform to protocol requirements.
bool canBeAsyncHandler = false;
if (auto witnessFunc = dyn_cast<FuncDecl>(witness)) {
canBeAsyncHandler = !witnessFunc->isAsyncHandler() &&
witnessFunc->canBeAsyncHandler();
}
auto diag = witness->diagnose(
canBeAsyncHandler
? diag::actor_isolated_witness_could_be_async_handler
: diag::actor_isolated_witness,
witness->getDescriptiveKind(), witness->getName());

if (canBeAsyncHandler) {
diag.fixItInsert(
witness->getAttributeInsertionLoc(false), "@asyncHandler ");
}

return true;
}

case ActorIsolationRestriction::GlobalActor: {
// Hang on to the global actor that's used for the witness. It will need
// to match that of the requirement.
witnessGlobalActor = witness->getDeclContext()->mapTypeIntoContext(
witnessRestriction.getGlobalActor());
break;
}

case ActorIsolationRestriction::Unsafe:
case ActorIsolationRestriction::LocalCapture:
break;

case ActorIsolationRestriction::Unrestricted:
// The witness is completely unrestricted, so ignore any annotations on
// the requirement.
return false;
}

// Check whether the requirement requires some particular actor isolation.
Type requirementGlobalActor;
switch (auto requirementIsolation = getActorIsolation(requirement)) {
case ActorIsolation::ActorInstance:
llvm_unreachable("There are not actor protocols");

case ActorIsolation::GlobalActor: {
auto requirementSubs = SubstitutionMap::getProtocolSubstitutions(
Proto, Adoptee, ProtocolConformanceRef(Conformance));
requirementGlobalActor = requirementIsolation.getGlobalActor()
.subst(requirementSubs);
break;
}

case ActorIsolation::Independent:
case ActorIsolation::Unspecified:
break;
}

// If neither has a global actor, we're done.
if (!witnessGlobalActor && !requirementGlobalActor)
return false;

// If the witness has a global actor but the requirement does not, we have
// an isolation error.
if (witnessGlobalActor && !requirementGlobalActor) {
witness->diagnose(
diag::global_actor_isolated_witness, witness->getDescriptiveKind(),
witness->getName(), witnessGlobalActor, Proto->getName());
requirement->diagnose(diag::decl_declared_here, requirement->getName());
return true;
}

// If the requirement has a global actor but the witness does not, we have
// an isolation error.
//
// FIXME: Within a module, this will be an inference rule.
if (requirementGlobalActor && !witnessGlobalActor) {
witness->diagnose(
diag::global_actor_isolated_requirement, witness->getDescriptiveKind(),
witness->getName(), requirementGlobalActor, Proto->getName())
.fixItInsert(
witness->getAttributeInsertionLoc(/*forModifier=*/false),
"@" + requirementGlobalActor.getString());
requirement->diagnose(diag::decl_declared_here, requirement->getName());
return true;
}

// If both have global actors but they differ, this is an isolation error.
if (!witnessGlobalActor->isEqual(requirementGlobalActor)) {
witness->diagnose(
diag::global_actor_isolated_requirement_witness_conflict,
witness->getDescriptiveKind(), witness->getName(), witnessGlobalActor,
Proto->getName(), requirementGlobalActor);
requirement->diagnose(diag::decl_declared_here, requirement->getName());
return true;
}

// Everything is okay.
return false;
}

bool ConformanceChecker::checkObjCTypeErasedGenerics(
AssociatedTypeDecl *assocType,
Type type,
Expand Down Expand Up @@ -4337,39 +4445,8 @@ void ConformanceChecker::resolveValueWitnesses() {
return;
}

// Check for actor-isolation consistency.
switch (auto restriction =
ActorIsolationRestriction::forDeclaration(witness)) {
case ActorIsolationRestriction::ActorSelf: {
// Actor-isolated witnesses cannot conform to protocol requirements.
bool canBeAsyncHandler = false;
if (auto witnessFunc = dyn_cast<FuncDecl>(witness)) {
canBeAsyncHandler = !witnessFunc->isAsyncHandler() &&
witnessFunc->canBeAsyncHandler();
}
auto diag = witness->diagnose(
canBeAsyncHandler
? diag::actor_isolated_witness_could_be_async_handler
: diag::actor_isolated_witness,
witness->getDescriptiveKind(), witness->getName());

if (canBeAsyncHandler) {
diag.fixItInsert(
witness->getAttributeInsertionLoc(false), "@asyncHandler ");
}
if (checkActorIsolation(requirement, witness))
return;
}

case ActorIsolationRestriction::GlobalActor: {
// FIXME: Check against the requirement. This needs serious refactoring.
break;
}

case ActorIsolationRestriction::Unrestricted:
case ActorIsolationRestriction::Unsafe:
case ActorIsolationRestriction::LocalCapture:
break;
}

// Objective-C checking for @objc requirements.
if (requirement->isObjC() &&
Expand Down
5 changes: 5 additions & 0 deletions lib/Sema/TypeCheckProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,11 @@ class ConformanceChecker : public WitnessChecker {
Type type,
TypeDecl *typeDecl);

/// Check that the witness and requirement have compatible actor contexts.
///
/// \returns true if an error occurred, false otherwise.
bool checkActorIsolation(ValueDecl *requirement, ValueDecl *witness);

/// Record a type witness.
///
/// \param assocType The associated type whose witness is being recorded.
Expand Down
59 changes: 59 additions & 0 deletions test/decl/class/actor/global_actor_conformance.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
// REQUIRES: concurrency

import _Concurrency

actor class SomeActor { }

@globalActor
struct GlobalActor {
static var shared: SomeActor { SomeActor() }
}

@globalActor
struct GenericGlobalActor<T> {
static var shared: SomeActor { SomeActor() }
}

protocol P1 {
associatedtype Assoc

@GlobalActor func method1() // expected-note{{declared here}}
@GenericGlobalActor<Int> func method2() // expected-note{{declared here}}
@GenericGlobalActor<Assoc> func method3()
func method4() // expected-note{{declared here}}
}

protocol P2 {
@GlobalActor func asyncMethod1() async
@GenericGlobalActor<Int> func asyncMethod2() async
func asyncMethod3() async
}

class C1 : P1, P2 {
typealias Assoc = String

// FIXME: This will be inferred
func method1() { } // expected-error{{instance method 'method1()' must be isolated to the global actor 'GlobalActor' to satisfy corresponding requirement from protocol 'P1'}}{{3-3=@GlobalActor}}

@GenericGlobalActor<String> func method2() { } // expected-error{{instance method 'method2()' isolated to global actor 'GenericGlobalActor<String>' can not satisfy corresponding requirement from protocol 'P1' isolated to global actor 'GenericGlobalActor<Int>'}}
@GenericGlobalActor<String >func method3() { }
@GlobalActor func method4() { } // expected-error{{instance method 'method4()' isolated to global actor 'GlobalActor' can not satisfy corresponding requirement from protocol 'P1'}}

// Okay: we can ignore the mismatch in global actor types for 'async' methods.
func asyncMethod1() async { }
@GenericGlobalActor<String> func asyncMethod2() async { }
@GlobalActor func asyncMethod3() async { }
}


class C2: P1 {
typealias Assoc = Int

// Okay: we can ignore the mismatch in global actor types for 'asyncHandler'
// methods.
@asyncHandler func method1() { }
@asyncHandler func method2() { }
@asyncHandler func method3() { }
@asyncHandler func method4() { }
}