Skip to content

Commit 2d8f073

Browse files
committed
[Concurrency] Implement global actor isolation checking for conformances.
Witnesses and requirements need to agree on their global actor annotations. However, this is not true for 'async' or '@asyncHandler' witnesses, for which it does not matter what the actor annotation is because part of the contract is that the function will execute on the appropriate actor.
1 parent 3b807a4 commit 2d8f073

File tree

4 files changed

+185
-32
lines changed

4 files changed

+185
-32
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4194,6 +4194,18 @@ ERROR(actor_isolated_witness_could_be_async_handler,none,
41944194
"actor-isolated %0 %1 cannot be used to satisfy a protocol requirement; "
41954195
"did you mean to make it an asychronous handler?",
41964196
(DescriptiveDeclKind, DeclName))
4197+
ERROR(global_actor_isolated_requirement,none,
4198+
"%0 %1 must be isolated to the global actor %2 to satisfy corresponding "
4199+
"requirement from protocol %3",
4200+
(DescriptiveDeclKind, DeclName, Type, Identifier))
4201+
ERROR(global_actor_isolated_witness,none,
4202+
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
4203+
"requirement from protocol %3",
4204+
(DescriptiveDeclKind, DeclName, Type, Identifier))
4205+
ERROR(global_actor_isolated_requirement_witness_conflict,none,
4206+
"%0 %1 isolated to global actor %2 can not satisfy corresponding "
4207+
"requirement from protocol %3 isolated to global actor %4",
4208+
(DescriptiveDeclKind, DeclName, Type, Identifier, Type))
41974209

41984210
ERROR(actorisolated_let,none,
41994211
"'@actorIsolated' is meaningless on 'let' declarations because "

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 109 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2656,6 +2656,114 @@ static void emitDeclaredHereIfNeeded(DiagnosticEngine &diags,
26562656
diags.diagnose(value, diag::decl_declared_here, value->getName());
26572657
}
26582658

2659+
bool ConformanceChecker::checkActorIsolation(
2660+
ValueDecl *requirement, ValueDecl *witness) {
2661+
// Ensure that the witness is not actor-isolated in a manner that makes it
2662+
// unsuitable as a witness.
2663+
Type witnessGlobalActor;
2664+
switch (auto witnessRestriction =
2665+
ActorIsolationRestriction::forDeclaration(witness)) {
2666+
case ActorIsolationRestriction::ActorSelf: {
2667+
// Actor-isolated witnesses cannot conform to protocol requirements.
2668+
bool canBeAsyncHandler = false;
2669+
if (auto witnessFunc = dyn_cast<FuncDecl>(witness)) {
2670+
canBeAsyncHandler = !witnessFunc->isAsyncHandler() &&
2671+
witnessFunc->canBeAsyncHandler();
2672+
}
2673+
auto diag = witness->diagnose(
2674+
canBeAsyncHandler
2675+
? diag::actor_isolated_witness_could_be_async_handler
2676+
: diag::actor_isolated_witness,
2677+
witness->getDescriptiveKind(), witness->getName());
2678+
2679+
if (canBeAsyncHandler) {
2680+
diag.fixItInsert(
2681+
witness->getAttributeInsertionLoc(false), "@asyncHandler ");
2682+
}
2683+
2684+
return true;
2685+
}
2686+
2687+
case ActorIsolationRestriction::GlobalActor: {
2688+
// Hang on to the global actor that's used for the witness. It will need
2689+
// to match that of the requirement.
2690+
witnessGlobalActor = witness->getDeclContext()->mapTypeIntoContext(
2691+
witnessRestriction.getGlobalActor());
2692+
break;
2693+
}
2694+
2695+
case ActorIsolationRestriction::Unsafe:
2696+
case ActorIsolationRestriction::LocalCapture:
2697+
break;
2698+
2699+
case ActorIsolationRestriction::Unrestricted:
2700+
// The witness is completely unrestricted, so ignore any annotations on
2701+
// the requirement.
2702+
return false;
2703+
}
2704+
2705+
// Check whether the requirement requires some particular actor isolation.
2706+
Type requirementGlobalActor;
2707+
switch (auto requirementIsolation = getActorIsolation(requirement)) {
2708+
case ActorIsolation::ActorInstance:
2709+
llvm_unreachable("There are not actor protocols");
2710+
2711+
case ActorIsolation::GlobalActor: {
2712+
auto requirementSubs = SubstitutionMap::getProtocolSubstitutions(
2713+
Proto, Adoptee, ProtocolConformanceRef(Conformance));
2714+
requirementGlobalActor = requirementIsolation.getGlobalActor()
2715+
.subst(requirementSubs);
2716+
break;
2717+
}
2718+
2719+
case ActorIsolation::Independent:
2720+
case ActorIsolation::Unspecified:
2721+
break;
2722+
}
2723+
2724+
// If neither has a global actor, we're done.
2725+
if (!witnessGlobalActor && !requirementGlobalActor)
2726+
return false;
2727+
2728+
// If the witness has a global actor but the requirement does not, we have
2729+
// an isolation error.
2730+
if (witnessGlobalActor && !requirementGlobalActor) {
2731+
witness->diagnose(
2732+
diag::global_actor_isolated_witness, witness->getDescriptiveKind(),
2733+
witness->getName(), witnessGlobalActor, Proto->getName());
2734+
requirement->diagnose(diag::decl_declared_here, requirement->getName());
2735+
return true;
2736+
}
2737+
2738+
// If the requirement has a global actor but the witness does not, we have
2739+
// an isolation error.
2740+
//
2741+
// FIXME: Within a module, this will be an inference rule.
2742+
if (requirementGlobalActor && !witnessGlobalActor) {
2743+
witness->diagnose(
2744+
diag::global_actor_isolated_requirement, witness->getDescriptiveKind(),
2745+
witness->getName(), requirementGlobalActor, Proto->getName())
2746+
.fixItInsert(
2747+
witness->getAttributeInsertionLoc(/*forModifier=*/false),
2748+
"@" + requirementGlobalActor.getString());
2749+
requirement->diagnose(diag::decl_declared_here, requirement->getName());
2750+
return true;
2751+
}
2752+
2753+
// If both have global actors but they differ, this is an isolation error.
2754+
if (!witnessGlobalActor->isEqual(requirementGlobalActor)) {
2755+
witness->diagnose(
2756+
diag::global_actor_isolated_requirement_witness_conflict,
2757+
witness->getDescriptiveKind(), witness->getName(), witnessGlobalActor,
2758+
Proto->getName(), requirementGlobalActor);
2759+
requirement->diagnose(diag::decl_declared_here, requirement->getName());
2760+
return true;
2761+
}
2762+
2763+
// Everything is okay.
2764+
return false;
2765+
}
2766+
26592767
bool ConformanceChecker::checkObjCTypeErasedGenerics(
26602768
AssociatedTypeDecl *assocType,
26612769
Type type,
@@ -4337,39 +4445,8 @@ void ConformanceChecker::resolveValueWitnesses() {
43374445
return;
43384446
}
43394447

4340-
// Check for actor-isolation consistency.
4341-
switch (auto restriction =
4342-
ActorIsolationRestriction::forDeclaration(witness)) {
4343-
case ActorIsolationRestriction::ActorSelf: {
4344-
// Actor-isolated witnesses cannot conform to protocol requirements.
4345-
bool canBeAsyncHandler = false;
4346-
if (auto witnessFunc = dyn_cast<FuncDecl>(witness)) {
4347-
canBeAsyncHandler = !witnessFunc->isAsyncHandler() &&
4348-
witnessFunc->canBeAsyncHandler();
4349-
}
4350-
auto diag = witness->diagnose(
4351-
canBeAsyncHandler
4352-
? diag::actor_isolated_witness_could_be_async_handler
4353-
: diag::actor_isolated_witness,
4354-
witness->getDescriptiveKind(), witness->getName());
4355-
4356-
if (canBeAsyncHandler) {
4357-
diag.fixItInsert(
4358-
witness->getAttributeInsertionLoc(false), "@asyncHandler ");
4359-
}
4448+
if (checkActorIsolation(requirement, witness))
43604449
return;
4361-
}
4362-
4363-
case ActorIsolationRestriction::GlobalActor: {
4364-
// FIXME: Check against the requirement. This needs serious refactoring.
4365-
break;
4366-
}
4367-
4368-
case ActorIsolationRestriction::Unrestricted:
4369-
case ActorIsolationRestriction::Unsafe:
4370-
case ActorIsolationRestriction::LocalCapture:
4371-
break;
4372-
}
43734450

43744451
// Objective-C checking for @objc requirements.
43754452
if (requirement->isObjC() &&

lib/Sema/TypeCheckProtocol.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,11 @@ class ConformanceChecker : public WitnessChecker {
739739
Type type,
740740
TypeDecl *typeDecl);
741741

742+
/// Check that the witness and requirement have compatible actor contexts.
743+
///
744+
/// \returns true if an error occurred, false otherwise.
745+
bool checkActorIsolation(ValueDecl *requirement, ValueDecl *witness);
746+
742747
/// Record a type witness.
743748
///
744749
/// \param assocType The associated type whose witness is being recorded.
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
2+
// REQUIRES: concurrency
3+
4+
import _Concurrency
5+
6+
actor class SomeActor { }
7+
8+
@globalActor
9+
struct GlobalActor {
10+
static var shared: SomeActor { SomeActor() }
11+
}
12+
13+
@globalActor
14+
struct GenericGlobalActor<T> {
15+
static var shared: SomeActor { SomeActor() }
16+
}
17+
18+
protocol P1 {
19+
associatedtype Assoc
20+
21+
@GlobalActor func method1() // expected-note{{declared here}}
22+
@GenericGlobalActor<Int> func method2() // expected-note{{declared here}}
23+
@GenericGlobalActor<Assoc> func method3()
24+
func method4() // expected-note{{declared here}}
25+
}
26+
27+
protocol P2 {
28+
@GlobalActor func asyncMethod1() async
29+
@GenericGlobalActor<Int> func asyncMethod2() async
30+
func asyncMethod3() async
31+
}
32+
33+
class C1 : P1, P2 {
34+
typealias Assoc = String
35+
36+
// FIXME: This will be inferred
37+
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}}
38+
39+
@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>'}}
40+
@GenericGlobalActor<String >func method3() { }
41+
@GlobalActor func method4() { } // expected-error{{instance method 'method4()' isolated to global actor 'GlobalActor' can not satisfy corresponding requirement from protocol 'P1'}}
42+
43+
// Okay: we can ignore the mismatch in global actor types for 'async' methods.
44+
func asyncMethod1() async { }
45+
@GenericGlobalActor<String> func asyncMethod2() async { }
46+
@GlobalActor func asyncMethod3() async { }
47+
}
48+
49+
50+
class C2: P1 {
51+
typealias Assoc = Int
52+
53+
// Okay: we can ignore the mismatch in global actor types for 'asyncHandler'
54+
// methods.
55+
@asyncHandler func method1() { }
56+
@asyncHandler func method2() { }
57+
@asyncHandler func method3() { }
58+
@asyncHandler func method4() { }
59+
}

0 commit comments

Comments
 (0)