Skip to content

Commit

Permalink
[Concurrency] Start diagnosing use of global actor isolated propertie…
Browse files Browse the repository at this point in the history
…s in key paths in strict mode

If key path is formed in a concurrency domain different from the
one where the member is, diagnose that as a warning in a strict mode
and error in swift 6 mode with strict concurrency enabled.
  • Loading branch information
xedin committed Dec 8, 2023
1 parent c9d8622 commit b9d9174
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 21 deletions.
4 changes: 2 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5234,8 +5234,8 @@ ERROR(concurrent_access_local,none,
"use of local %kind0 in concurrently-executing code",
(const ValueDecl *))
ERROR(actor_isolated_keypath_component,none,
"cannot form key path to%select{| distributed}0 actor-isolated %kind1",
(bool, const ValueDecl *))
"cannot form key path to %0 %kind1",
(ActorIsolation, const ValueDecl *))
ERROR(effectful_keypath_component,none,
"cannot form key path to %0 with 'throws' or 'async'",
(DescriptiveDeclKind))
Expand Down
37 changes: 26 additions & 11 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3319,8 +3319,8 @@ namespace {
for (const auto &component : keyPath->getComponents()) {
// The decl referred to by the path component cannot be within an actor.
if (component.hasDeclRef()) {
auto concDecl = component.getDeclRef();
auto decl = concDecl.getDecl();
auto declRef = component.getDeclRef();
auto decl = declRef.getDecl();
auto isolation = getActorIsolationForReference(
decl, getDeclContext());
switch (isolation) {
Expand All @@ -3330,13 +3330,22 @@ namespace {
break;

case ActorIsolation::GlobalActor:
case ActorIsolation::GlobalActorUnsafe:
// Disable global actor checking for now.
if (isolation.isGlobalActor() &&
!ctx.LangOpts.isSwiftVersionAtLeast(6))
case ActorIsolation::GlobalActorUnsafe: {
// Perform the check only in `complete` mode or
// stricter.
if (ctx.LangOpts.StrictConcurrencyLevel <
StrictConcurrency::Complete)
break;

auto result = ActorReferenceResult::forReference(
declRef, component.getLoc(), getDeclContext(),
kindOfUsage(decl, keyPath));

if (result == ActorReferenceResult::SameConcurrencyDomain)
break;

LLVM_FALLTHROUGH;
}

case ActorIsolation::ActorInstance:
// If this entity is always accessible across actors, just check
Expand All @@ -3352,11 +3361,17 @@ namespace {
break;
}

ctx.Diags.diagnose(component.getLoc(),
diag::actor_isolated_keypath_component,
isolation.isDistributedActor(),
decl);
diagnosed = true;
{
auto diagnostic = ctx.Diags.diagnose(
component.getLoc(), diag::actor_isolated_keypath_component,
isolation, decl);

if (isolation == ActorIsolation::ActorInstance)
diagnosed = true;
else
diagnostic.warnUntilSwiftVersion(6);
}

break;
}
}
Expand Down
14 changes: 10 additions & 4 deletions test/Concurrency/actor_keypath_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func tryKeyPathsMisc(d : Door) {

func tryKeyPathsFromAsync() async {
_ = \Door.unsafeGlobActor_immutable
_ = \Door.unsafeGlobActor_mutable // okay for now
_ = \Door.unsafeGlobActor_mutable // expected-warning {{cannot form key path to main actor-isolated property 'unsafeGlobActor_mutable'; this is an error in Swift 6}}
}

func tryNonSendable() {
Expand All @@ -69,7 +69,7 @@ func tryNonSendable() {

func tryKeypaths() {
_ = \Door.unsafeGlobActor_immutable
_ = \Door.unsafeGlobActor_mutable // okay for now
_ = \Door.unsafeGlobActor_mutable // expected-warning {{cannot form key path to main actor-isolated property 'unsafeGlobActor_mutable'; this is an error in Swift 6}}

_ = \Door.immutable
_ = \Door.globActor_immutable
Expand All @@ -84,7 +84,13 @@ func tryKeypaths() {
let _ : PartialKeyPath<Door> = \.mutable // expected-error{{cannot form key path to actor-isolated property 'mutable'}}
let _ : AnyKeyPath = \Door.mutable // expected-error{{cannot form key path to actor-isolated property 'mutable'}}

_ = \Door.globActor_mutable // okay for now
_ = \Door.globActor_mutable // expected-warning {{cannot form key path to main actor-isolated property 'globActor_mutable'; this is an error in Swift 6}}
_ = \Door.[0] // expected-error{{cannot form key path to actor-isolated subscript 'subscript(_:)'}}
_ = \Door.["hello"] // okay for now
_ = \Door.["hello"] // expected-warning {{cannot form key path to main actor-isolated subscript 'subscript(_:)'; this is an error in Swift 6}}
}

@MainActor func testGlobalActorRefInSameContext() {
_ = \Door.unsafeGlobActor_mutable // Ok
_ = \Door.globActor_mutable // Ok
_ = \Door.["hello"] // Ok
}
14 changes: 10 additions & 4 deletions test/Concurrency/actor_keypath_isolation_swift6.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func tryKeyPathsMisc(d : Door) {

func tryKeyPathsFromAsync() async {
_ = \Door.unsafeGlobActor_immutable
_ = \Door.unsafeGlobActor_mutable // expected-error {{cannot form key path to actor-isolated property 'unsafeGlobActor_mutable'}}
_ = \Door.unsafeGlobActor_mutable // expected-error {{cannot form key path to main actor-isolated property 'unsafeGlobActor_mutable'}}
}

func tryNonSendable() {
Expand All @@ -68,7 +68,7 @@ func tryNonSendable() {

func tryKeypaths() {
_ = \Door.unsafeGlobActor_immutable
_ = \Door.unsafeGlobActor_mutable // expected-error {{cannot form key path to actor-isolated property 'unsafeGlobActor_mutable'}}
_ = \Door.unsafeGlobActor_mutable // expected-error {{cannot form key path to main actor-isolated property 'unsafeGlobActor_mutable'}}

_ = \Door.immutable
_ = \Door.globActor_immutable
Expand All @@ -83,7 +83,13 @@ func tryKeypaths() {
let _ : PartialKeyPath<Door> = \.mutable // expected-error{{cannot form key path to actor-isolated property 'mutable'}}
let _ : AnyKeyPath = \Door.mutable // expected-error{{cannot form key path to actor-isolated property 'mutable'}}

_ = \Door.globActor_mutable // expected-error{{cannot form key path to actor-isolated property 'globActor_mutable'}}
_ = \Door.globActor_mutable // expected-error{{cannot form key path to main actor-isolated property 'globActor_mutable'}}
_ = \Door.[0] // expected-error{{cannot form key path to actor-isolated subscript 'subscript(_:)'}}
_ = \Door.["hello"] // expected-error {{cannot form key path to actor-isolated subscript 'subscript(_:)'}}
_ = \Door.["hello"] // expected-error {{cannot form key path to main actor-isolated subscript 'subscript(_:)'}}
}

@MainActor func testGlobalActorRefInSameContext() {
_ = \Door.unsafeGlobActor_mutable // Ok
_ = \Door.globActor_mutable // Ok
_ = \Door.["hello"] // Ok
}
25 changes: 25 additions & 0 deletions test/Concurrency/global_actor_keypath_non_strict.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify

// REQUIRES: concurrency && asserts

actor Door {
@MainActor var globActor_mutable : Int = 0
@MainActor(unsafe) var unsafeGlobActor_mutable : Int = 0
@MainActor subscript(byName: String) -> Int { 0 }
}

func tryKeyPathsFromAsync() async {
_ = \Door.unsafeGlobActor_mutable // no warning
}

func tryKeypaths() {
_ = \Door.unsafeGlobActor_mutable // no warning
_ = \Door.globActor_mutable // no warning
_ = \Door.["hello"] // no warning
}

@MainActor func testGlobalActorRefInSameContext() {
_ = \Door.unsafeGlobActor_mutable // Ok
_ = \Door.globActor_mutable // Ok
_ = \Door.["hello"] // Ok
}

0 comments on commit b9d9174

Please sign in to comment.