Skip to content

Commit 87b43e6

Browse files
committed
Sendable checking for overrides.
When an override means crossing an actor boundary, check Sendability of parameters and results. (cherry picked from commit a8e1629)
1 parent 50ddeae commit 87b43e6

File tree

7 files changed

+63
-24
lines changed

7 files changed

+63
-24
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4687,8 +4687,8 @@ ERROR(isolated_parameter_not_actor,none,
46874687
WARNING(non_sendable_param_type,none,
46884688
"non-sendable type %0 %select{passed in call to %4 %2 %3|"
46894689
"passed in implicitly asynchronous call to %4 %2 %3|"
4690-
"in parameter of %4 %2 %3 satisfying non-isolated protocol "
4691-
"requirement|"
4690+
"in parameter of %4 %2 %3 satisfying protocol requirement|"
4691+
"in parameter of %4 overriding %2 %3|"
46924692
"in parameter of %4 '@objc' %2 %3}1 cannot cross actor boundary",
46934693
(Type, unsigned, DescriptiveDeclKind, DeclName, ActorIsolation))
46944694
WARNING(non_sendable_call_param_type,none,
@@ -4698,7 +4698,8 @@ WARNING(non_sendable_call_param_type,none,
46984698
WARNING(non_sendable_result_type,none,
46994699
"non-sendable type %0 returned by %select{call to %4 %2 %3|"
47004700
"implicitly asynchronous call to %4 %2 %3|"
4701-
"%4 %2 %3 satisfying non-isolated protocol requirement|"
4701+
"%4 %2 %3 satisfying protocol requirement|"
4702+
"%4 overriding %2 %3|"
47024703
"%4 '@objc' %2 %3}1 cannot cross actor boundary",
47034704
(Type, unsigned, DescriptiveDeclKind, DeclName, ActorIsolation))
47044705
WARNING(non_sendable_call_result_type,none,
@@ -4709,7 +4710,8 @@ WARNING(non_sendable_property_type,none,
47094710
"non-sendable type %0 in %select{"
47104711
"%select{asynchronous access to %5 %1 %2|"
47114712
"implicitly asynchronous access to %5 %1 %2|"
4712-
"conformance of %5 %1 %2 to non-isolated protocol requirement|"
4713+
"conformance of %5 %1 %2 to protocol requirement|"
4714+
"%5 overriding %1 %2|"
47134715
"%5 '@objc' %1 %2}4|captured local %1 %2}3 cannot "
47144716
"cross %select{actor|task}3 boundary",
47154717
(Type, DescriptiveDeclKind, DeclName, bool, unsigned, ActorIsolation))

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3489,19 +3489,25 @@ static ActorIsolation getOverriddenIsolationFor(ValueDecl *value) {
34893489
return isolation.subst(subs);
34903490
}
34913491

3492+
static ConcreteDeclRef getDeclRefInContext(ValueDecl *value) {
3493+
auto declContext = value->getInnermostDeclContext();
3494+
if (auto genericEnv = declContext->getGenericEnvironmentOfContext()) {
3495+
return ConcreteDeclRef(
3496+
value, genericEnv->getForwardingSubstitutionMap());
3497+
}
3498+
3499+
return ConcreteDeclRef(value);
3500+
}
3501+
34923502
/// Generally speaking, the isolation of the decl that overrides
34933503
/// must match the overridden decl. But there are a number of exceptions,
34943504
/// e.g., the decl that overrides can be nonisolated.
34953505
/// \param isolation the isolation of the overriding declaration.
34963506
static OverrideIsolationResult validOverrideIsolation(
34973507
ValueDecl *value, ActorIsolation isolation,
34983508
ValueDecl *overridden, ActorIsolation overriddenIsolation) {
3499-
ConcreteDeclRef valueRef(value);
3509+
ConcreteDeclRef valueRef = getDeclRefInContext(value);
35003510
auto declContext = value->getInnermostDeclContext();
3501-
if (auto genericEnv = declContext->getGenericEnvironmentOfContext()) {
3502-
valueRef = ConcreteDeclRef(
3503-
value, genericEnv->getForwardingSubstitutionMap());
3504-
}
35053511

35063512
auto refResult = ActorReferenceResult::forReference(
35073513
valueRef, SourceLoc(), declContext, None, None,
@@ -3520,9 +3526,7 @@ static OverrideIsolationResult validOverrideIsolation(
35203526
if (isAsyncDecl(overridden) ||
35213527
isAccessibleAcrossActors(
35223528
overridden, refResult.isolation, declContext)) {
3523-
// FIXME: Perform Sendable checking here because we're entering an
3524-
// actor.
3525-
return OverrideIsolationResult::Allowed;
3529+
return OverrideIsolationResult::Sendable;
35263530
}
35273531

35283532
// If the overridden declaration is from Objective-C with no actor
@@ -3898,7 +3902,9 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
38983902
return;
38993903

39003904
case OverrideIsolationResult::Sendable:
3901-
// FIXME: Do the Sendable check.
3905+
diagnoseNonSendableTypesInReference(
3906+
getDeclRefInContext(value), value->getInnermostDeclContext(),
3907+
value->getLoc(), SendableCheckReason::Override);
39023908
return;
39033909

39043910
case OverrideIsolationResult::Disallowed:

lib/Sema/TypeCheckConcurrency.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ enum class SendableCheckReason {
8383
/// actor isolation.
8484
Conformance,
8585

86+
/// An override of a function.
87+
Override,
88+
8689
/// The declaration is being exposed to Objective-C.
8790
ObjC,
8891
};

test/Concurrency/concurrent_value_checking.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ protocol AsyncProto {
223223
}
224224

225225
extension A1: AsyncProto {
226-
func asyncMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of actor-isolated instance method 'asyncMethod' satisfying non-isolated protocol requirement cannot cross actor boundary}}
226+
func asyncMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of actor-isolated instance method 'asyncMethod' satisfying protocol requirement cannot cross actor boundary}}
227227
}
228228

229229
protocol MainActorProto {
@@ -232,7 +232,7 @@ protocol MainActorProto {
232232

233233
class SomeClass: MainActorProto {
234234
@SomeGlobalActor
235-
func asyncMainMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of global actor 'SomeGlobalActor'-isolated instance method 'asyncMainMethod' satisfying non-isolated protocol requirement cannot cross actor boundary}}
235+
func asyncMainMethod(_: NotConcurrent) async { } // expected-warning{{non-sendable type 'NotConcurrent' in parameter of global actor 'SomeGlobalActor'-isolated instance method 'asyncMainMethod' satisfying protocol requirement cannot cross actor boundary}}
236236
}
237237

238238
// ----------------------------------------------------------------------

test/Concurrency/sendable_conformance_checking.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ protocol AsyncProtocolWithNotSendable {
4040
// actor's domain.
4141
@available(SwiftStdlib 5.1, *)
4242
actor A3: AsyncProtocolWithNotSendable {
43-
func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
43+
func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
4444

45-
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
45+
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
4646
get async {
4747
NotSendable()
4848
}
@@ -53,9 +53,9 @@ actor A3: AsyncProtocolWithNotSendable {
5353
// actor's domain.
5454
@available(SwiftStdlib 5.1, *)
5555
actor A4: AsyncProtocolWithNotSendable {
56-
func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
56+
func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
5757

58-
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
58+
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
5959
get {
6060
NotSendable()
6161
}
@@ -98,9 +98,9 @@ protocol AsyncThrowingProtocolWithNotSendable {
9898
// actor's domain.
9999
@available(SwiftStdlib 5.1, *)
100100
actor A7: AsyncThrowingProtocolWithNotSendable {
101-
func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
101+
func f() async -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
102102

103-
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
103+
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
104104
get async {
105105
NotSendable()
106106
}
@@ -111,9 +111,9 @@ actor A7: AsyncThrowingProtocolWithNotSendable {
111111
// actor's domain.
112112
@available(SwiftStdlib 5.1, *)
113113
actor A8: AsyncThrowingProtocolWithNotSendable {
114-
func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying non-isolated protocol requirement cannot cross actor boundary}}
114+
func f() -> NotSendable { NotSendable() } // expected-warning{{non-sendable type 'NotSendable' returned by actor-isolated instance method 'f()' satisfying protocol requirement cannot cross actor boundary}}
115115

116-
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to non-isolated protocol requirement cannot cross actor boundary}}
116+
var prop: NotSendable { // expected-warning{{non-sendable type 'NotSendable' in conformance of actor-isolated property 'prop' to protocol requirement cannot cross actor boundary}}
117117
get {
118118
NotSendable()
119119
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-typecheck-verify-swift
2+
// REQUIRES: concurrency
3+
4+
@available(SwiftStdlib 5.1, *)
5+
class NotSendable { // expected-note 2{{class 'NotSendable' does not conform to the 'Sendable' protocol}}
6+
}
7+
8+
@available(SwiftStdlib 5.1, *)
9+
@available(*, unavailable)
10+
extension NotSendable: Sendable { }
11+
12+
@available(SwiftStdlib 5.1, *)
13+
class Super {
14+
func f(_: NotSendable) async { }
15+
@MainActor func g1(_: NotSendable) { }
16+
@MainActor func g2(_: NotSendable) async { }
17+
}
18+
19+
@available(SwiftStdlib 5.1, *)
20+
class Sub: Super {
21+
@MainActor override func f(_: NotSendable) async { }
22+
// expected-warning@-1{{non-sendable type 'NotSendable' in parameter of main actor-isolated overriding instance method 'f' cannot cross actor boundary}}
23+
24+
nonisolated override func g1(_: NotSendable) { } // okay, synchronous
25+
26+
nonisolated override func g2(_: NotSendable) async { }
27+
// expected-warning@-1{{non-sendable type 'NotSendable' in parameter of nonisolated overriding instance method 'g2' cannot cross actor boundary}}
28+
}

test/Distributed/distributed_protocol_isolation.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ protocol Server {
127127
func send<Message: Codable>(message: Message) async throws -> String
128128
}
129129
actor MyServer : Server {
130-
func send<Message: Codable>(message: Message) throws -> String { "" } // expected-warning{{non-sendable type 'Message' in parameter of actor-isolated instance method 'send(message:)' satisfying non-isolated protocol requirement cannot cross actor boundary}}
130+
func send<Message: Codable>(message: Message) throws -> String { "" } // expected-warning{{non-sendable type 'Message' in parameter of actor-isolated instance method 'send(message:)' satisfying protocol requirement cannot cross actor boundary}}
131131
}
132132

133133
protocol AsyncThrowsAll {

0 commit comments

Comments
 (0)