Skip to content

[Concurrency] Diagnose non-Sendable 'self' arguments crossing actor isolation boundaries in member reference expressions. #68081

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 1 commit into from
Aug 23, 2023
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
37 changes: 20 additions & 17 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,8 @@ bool swift::diagnoseNonSendableTypes(
}

bool swift::diagnoseNonSendableTypesInReference(
ConcreteDeclRef declRef, const DeclContext *fromDC, SourceLoc refLoc,
Expr *base, ConcreteDeclRef declRef,
const DeclContext *fromDC, SourceLoc refLoc,
SendableCheckReason refKind, llvm::Optional<ActorIsolation> knownIsolation,
FunctionCheckKind funcCheckKind, SourceLoc diagnoseLoc) {
// Retrieve the actor isolation to use in diagnostics.
Expand All @@ -1010,6 +1011,17 @@ bool swift::diagnoseNonSendableTypesInReference(
return swift::getActorIsolation(declRef.getDecl());
};

// Check the 'self' argument.
if (base) {
if (diagnoseNonSendableTypes(
base->getType(),
fromDC, base->getStartLoc(),
diag::non_sendable_param_type,
(unsigned)refKind, declRef.getDecl(),
getActorIsolation()))
return true;
}

// For functions, check the parameter and result types.
SubstitutionMap subs = declRef.getSubstitutions();
if (auto function = dyn_cast<AbstractFunctionDecl>(declRef.getDecl())) {
Expand Down Expand Up @@ -2625,7 +2637,6 @@ namespace {
FoundAsync, // successfully marked an implicitly-async operation
NotFound, // fail: no valid implicitly-async operation was found
SyncContext, // fail: a valid implicitly-async op, but in sync context
NotSendable, // fail: valid op and context, but not Sendable
NotDistributed, // fail: non-distributed declaration in distributed actor
};

Expand Down Expand Up @@ -2744,16 +2755,6 @@ namespace {
}
}

if (result == AsyncMarkingResult::FoundAsync) {
// Check for non-sendable types.
bool problemFound =
diagnoseNonSendableTypesInReference(
concDeclRef, getDeclContext(), declLoc,
SendableCheckReason::SynchronousAsAsync);
if (problemFound)
result = AsyncMarkingResult::NotSendable;
}

return result;
}

Expand Down Expand Up @@ -3155,7 +3156,7 @@ namespace {
return true;

return diagnoseNonSendableTypesInReference(
declRef, getDeclContext(), loc,
base, declRef, getDeclContext(), loc,
SendableCheckReason::ExitingActor,
result.isolation);

Expand Down Expand Up @@ -3190,7 +3191,7 @@ namespace {
// Sendable checking and we're done.
if (!result.options) {
return diagnoseNonSendableTypesInReference(
declRef, getDeclContext(), loc,
base, declRef, getDeclContext(), loc,
SendableCheckReason::CrossActor);
}

Expand All @@ -3203,11 +3204,11 @@ namespace {
loc, declRef, context, result.isolation, isDistributed);
switch (implicitAsyncResult) {
case AsyncMarkingResult::FoundAsync:
// Success! We're done.
return false;
return diagnoseNonSendableTypesInReference(
base, declRef, getDeclContext(), loc,
SendableCheckReason::SynchronousAsAsync);

case AsyncMarkingResult::NotDistributed:
case AsyncMarkingResult::NotSendable:
// Failed, but diagnostics have already been emitted.
return true;

Expand Down Expand Up @@ -4426,12 +4427,14 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
case OverrideIsolationResult::Sendable:
// Check that the results of the overriding method are sendable
diagnoseNonSendableTypesInReference(
/*base=*/nullptr,
getDeclRefInContext(value), value->getInnermostDeclContext(),
value->getLoc(), SendableCheckReason::Override,
getActorIsolation(value), FunctionCheckKind::Results);

// Check that the parameters of the overridden method are sendable
diagnoseNonSendableTypesInReference(
/*base=*/nullptr,
getDeclRefInContext(overridden), overridden->getInnermostDeclContext(),
overridden->getLoc(), SendableCheckReason::Override,
getActorIsolation(value), FunctionCheckKind::Params,
Expand Down
6 changes: 5 additions & 1 deletion lib/Sema/TypeCheckConcurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ enum class FunctionCheckKind {
/// concurrency domain, whether in/out of an actor or in/or of a concurrent
/// function or closure.
///
/// \param base The base expression of the reference, which must be 'Sendable'
/// in order to cross actor isolation boundaries.
///
/// \param declRef The declaration being referenced from another concurrency
/// domain, including the substitutions so that (e.g.) we can consider the
/// specific types at the use site.
Expand All @@ -286,7 +289,8 @@ enum class FunctionCheckKind {
///
/// \returns true if an problem was detected, false otherwise.
bool diagnoseNonSendableTypesInReference(
ConcreteDeclRef declRef, const DeclContext *fromDC, SourceLoc refLoc,
Expr *base, ConcreteDeclRef declRef,
const DeclContext *fromDC, SourceLoc refLoc,
SendableCheckReason refKind,
llvm::Optional<ActorIsolation> knownIsolation = llvm::None,
FunctionCheckKind funcCheckKind = FunctionCheckKind::ParamsResults,
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckDeclObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ static bool checkObjCActorIsolation(const ValueDecl *VD, ObjCReason Reason) {

// FIXME: Substitution map?
diagnoseNonSendableTypesInReference(
const_cast<ValueDecl *>(VD), VD->getDeclContext(),
/*base=*/nullptr, const_cast<ValueDecl *>(VD), VD->getDeclContext(),
VD->getLoc(), SendableCheckReason::ObjC);
return false;

Expand Down
8 changes: 5 additions & 3 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3041,7 +3041,8 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,

case ActorReferenceResult::ExitsActorToNonisolated:
diagnoseNonSendableTypesInReference(
getDeclRefInContext(witness), DC, loc, SendableCheckReason::Conformance);
/*base=*/nullptr, getDeclRefInContext(witness),
DC, loc, SendableCheckReason::Conformance);
return llvm::None;
case ActorReferenceResult::EntersActor:
// Handled below.
Expand Down Expand Up @@ -3124,13 +3125,14 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,

// Check that the results of the witnessing method are sendable
diagnoseNonSendableTypesInReference(
getDeclRefInContext(witness), DC, loc, SendableCheckReason::Conformance,
/*base=*/nullptr, getDeclRefInContext(witness),
DC, loc, SendableCheckReason::Conformance,
getActorIsolation(witness), FunctionCheckKind::Results);

// If this requirement is a function, check that its parameters are Sendable as well
if (isa<AbstractFunctionDecl>(requirement)) {
diagnoseNonSendableTypesInReference(
getDeclRefInContext(requirement),
/*base=*/nullptr, getDeclRefInContext(requirement),
requirement->getInnermostDeclContext(), requirement->getLoc(),
SendableCheckReason::Conformance, getActorIsolation(witness),
FunctionCheckKind::Params, loc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ public dynamic func dynamicOnMainActor() { }

// property wrapper + main actor
@propertyWrapper
public struct IntWrapper {
public struct IntWrapper: Sendable {

public init(wrappedValue: Int) {
self.wrappedValue = wrappedValue
Expand Down
6 changes: 4 additions & 2 deletions test/Concurrency/actor_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,11 @@ func checkIsolationValueType(_ formance: InferredFromConformance,
_ = await ext.point // expected-warning {{non-sendable type 'Point' in implicitly asynchronous access to main actor-isolated property 'point' cannot cross actor boundary}}
_ = await formance.counter
_ = await anno.point // expected-warning {{non-sendable type 'Point' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated property 'point' cannot cross actor boundary}}
_ = anno.counter
// expected-warning@-1 {{non-sendable type 'NoGlobalActorValueType' passed in implicitly asynchronous call to global actor 'SomeGlobalActor'-isolated property 'point' cannot cross actor boundary}}
_ = anno.counter // expected-warning {{non-sendable type 'NoGlobalActorValueType' passed in call to main actor-isolated property 'counter' cannot cross actor boundary}}

// these will always need an await
_ = await (formance as MainCounter).counter
_ = await (formance as MainCounter).counter // expected-warning {{non-sendable type 'any MainCounter' passed in implicitly asynchronous call to main actor-isolated property 'counter' cannot cross actor boundary}}
_ = await ext[1]
_ = await formance.ticker
_ = await ext.polygon // expected-warning {{non-sendable type '[Point]' in implicitly asynchronous access to main actor-isolated property 'polygon' cannot cross actor boundary}}
Expand All @@ -159,6 +160,7 @@ func checkIsolationValueType(_ formance: InferredFromConformance,
}

// check for instance members that do not need global-actor protection
// expected-note@+1 2 {{consider making struct 'NoGlobalActorValueType' conform to the 'Sendable' protocol}}
struct NoGlobalActorValueType {
@SomeGlobalActor var point: Point // expected-warning {{stored property 'point' within struct cannot have a global actor; this is an error in Swift 6}}

Expand Down
1 change: 1 addition & 0 deletions test/Concurrency/actor_isolation_swift6.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func checkIsolationValueType(_ formance: InferredFromConformance,

// these do need await, regardless of reference or value type
_ = await (formance as any MainCounter).counter
// expected-warning@-1 {{non-sendable type 'any MainCounter' passed in implicitly asynchronous call to main actor-isolated property 'counter' cannot cross actor boundary}}
_ = await ext[1]
_ = await formance.ticker
_ = await ext.polygon
Expand Down
14 changes: 13 additions & 1 deletion test/Concurrency/sendable_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func testConversionsAndSendable(a: MyActor, s: any Sendable, f: @Sendable () ->

@available(SwiftStdlib 5.1, *)
final class NonSendable {
// expected-note@-1 3 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}
// expected-note@-1 6 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}
var value = ""

@MainActor
Expand All @@ -247,12 +247,24 @@ final class NonSendable {

await self.update()
// expected-warning@-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}

_ = await x
// expected-warning@-1 {{non-sendable type 'NonSendable' passed in implicitly asynchronous call to main actor-isolated property 'x' cannot cross actor boundary}}

_ = await self.x
// expected-warning@-1 {{non-sendable type 'NonSendable' passed in implicitly asynchronous call to main actor-isolated property 'x' cannot cross actor boundary}}
}

@MainActor
var x: Int { 0 }
}

@available(SwiftStdlib 5.1, *)
func testNonSendableBaseArg() async {
let t = NonSendable()
await t.update()
// expected-warning@-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}

_ = await t.x
// expected-warning@-1 {{non-sendable type 'NonSendable' passed in implicitly asynchronous call to main actor-isolated property 'x' cannot cross actor boundary}}
}
8 changes: 6 additions & 2 deletions validation-test/Sema/SwiftUI/rdar76252310.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Visibility: ObservableObject { // expected-note 2{{class 'Visibility' does
@Published var yes = false // some nonsense
}

struct CoffeeTrackerView: View { // expected-note{{consider making struct 'CoffeeTrackerView' conform to the 'Sendable' protocol}}
struct CoffeeTrackerView: View { // expected-note 4{{consider making struct 'CoffeeTrackerView' conform to the 'Sendable' protocol}}
@ObservedObject var showDrinkList: Visibility = Visibility()

var storage: Visibility = Visibility()
Expand Down Expand Up @@ -39,15 +39,19 @@ func fromConcurrencyAware() async {
// expected-warning@+1 {{non-sendable type 'CoffeeTrackerView' returned by call to main actor-isolated function cannot cross actor boundary}}
let view = CoffeeTrackerView()

// expected-warning@+4 {{non-sendable type 'CoffeeTrackerView' passed in implicitly asynchronous call to main actor-isolated property 'body' cannot cross actor boundary}}
// expected-note@+3 {{property access is 'async'}}
// expected-warning@+2 {{non-sendable type 'some View' in implicitly asynchronous access to main actor-isolated property 'body' cannot cross actor boundary}}
// expected-error@+1 {{expression is 'async' but is not marked with 'await'}}
_ = view.body

// expected-warning@+4 {{non-sendable type 'CoffeeTrackerView' passed in implicitly asynchronous call to main actor-isolated property 'showDrinkList' cannot cross actor boundary}}
// expected-note@+3 {{property access is 'async'}}
// expected-warning@+2 {{non-sendable type 'Visibility' in implicitly asynchronous access to main actor-isolated property 'showDrinkList' cannot cross actor boundary}}
// expected-error@+1 {{expression is 'async' but is not marked with 'await'}}
_ = view.showDrinkList

_ = await view.storage // expected-warning {{non-sendable type 'Visibility' in implicitly asynchronous access to main actor-isolated property 'storage' cannot cross actor boundary}}
// expected-warning@+2 {{non-sendable type 'CoffeeTrackerView' passed in implicitly asynchronous call to main actor-isolated property 'storage' cannot cross actor boundary}}
// expected-warning@+1 {{non-sendable type 'Visibility' in implicitly asynchronous access to main actor-isolated property 'storage' cannot cross actor boundary}}
_ = await view.storage
}