Skip to content

[Concurrency] nonisolated can only be applied to actor properties with Sendable type. #70909

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
Jan 28, 2024
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
7 changes: 4 additions & 3 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5602,9 +5602,6 @@ WARNING(non_sendable_property_type,none,
WARNING(non_sendable_keypath_capture,none,
"cannot form key path that captures non-sendable type %0",
(Type))
WARNING(non_sendable_keypath_access,none,
"cannot form key path that accesses non-sendable type %0",
(Type))
ERROR(non_concurrent_type_member,none,
"%select{stored property %2|associated value %2}1 of "
"'Sendable'-conforming %kind3 has non-sendable type %0",
Expand Down Expand Up @@ -5668,6 +5665,10 @@ NOTE(nonisolated_mutable_storage_note,none,
"convert %0 to a 'let' constant or consider declaring it "
"'nonisolated(unsafe)' if manually managing concurrency safety",
(const VarDecl *))
ERROR(nonisolated_non_sendable,none,
"'nonisolated' can not be applied to variable with non-'Sendable' "
"type %0",
(Type))
ERROR(nonisolated_local_var,none,
"'nonisolated' can not be applied to local variables",
())
Expand Down
5 changes: 2 additions & 3 deletions lib/SILOptimizer/Mandatory/FlowIsolation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -497,9 +497,8 @@ static bool accessIsConcurrencySafe(ModuleDecl *module,
RefElementAddrInst *inst) {
VarDecl *var = inst->getField();

// must be accessible from nonisolated and Sendable
return isLetAccessibleAnywhere(module, var)
&& var->getTypeInContext()->isSendableType();
// must be accessible from nonisolated.
return isLetAccessibleAnywhere(module, var);
}

/// \returns true iff the ref_element_addr instruction is only used
Expand Down
11 changes: 11 additions & 0 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6884,6 +6884,17 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
return;
}

// 'nonisolated' without '(unsafe)' is not allowed on non-Sendable variables.
auto type = var->getTypeInContext();
if (!attr->isUnsafe() && !type->hasError() &&
!type->isSendableType()) {
Ctx.Diags.diagnose(attr->getLocation(),
diag::nonisolated_non_sendable,
type)
.warnUntilSwiftVersion(6);
return;
}

if (auto nominal = dyn_cast<NominalTypeDecl>(dc)) {
// 'nonisolated' can not be applied to stored properties inside
// distributed actors. Attempts of nonisolated access would be
Expand Down
48 changes: 27 additions & 21 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,22 @@ static bool varIsSafeAcrossActors(const ModuleDecl *fromModule,
return false;
}

bool accessWithinModule =
(fromModule == var->getDeclContext()->getParentModule());

// If the type is not 'Sendable', it's unsafe
if (!var->getTypeInContext()->isSendableType()) {
// Compiler versions <= 5.10 treated this variable as nonisolated,
// so downgrade async access errors in the effects checker to
// warnings prior to Swift 6.
if (accessWithinModule)
options = ActorReferenceResult::Flags::Preconcurrency;

return false;
}

// If it's actor-isolated but in the same module, then it's OK too.
return (fromModule == var->getDeclContext()->getParentModule());
return accessWithinModule;
}
}

Expand Down Expand Up @@ -3642,34 +3656,26 @@ namespace {
LLVM_FALLTHROUGH;
}

case ActorIsolation::ActorInstance:
// If this entity is always accessible across actors, just check
// Sendable.
case ActorIsolation::ActorInstance: {
ActorReferenceResult::Options options = llvm::None;
if (isAccessibleAcrossActors(decl, isolation, getDeclContext(),
llvm::None)) {
if (diagnoseNonSendableTypes(
component.getComponentType(), getDeclContext(),
/*inDerivedConformance*/Type(),
component.getLoc(),
diag::non_sendable_keypath_access)) {
diagnosed = true;
}
options)) {
break;
}

{
auto diagnostic = ctx.Diags.diagnose(
component.getLoc(), diag::actor_isolated_keypath_component,
isolation, decl);
bool downgrade = isolation.isGlobalActor() ||
options.contains(
ActorReferenceResult::Flags::Preconcurrency);

if (isolation == ActorIsolation::ActorInstance)
diagnosed = true;
else
diagnostic.warnUntilSwiftVersion(6);
}
ctx.Diags.diagnose(
component.getLoc(), diag::actor_isolated_keypath_component,
isolation, decl)
.warnUntilSwiftVersionIf(downgrade, 6);

diagnosed = !downgrade;
break;
}
}
}

// With `InferSendableFromCaptures` feature enabled the solver is
Expand Down
11 changes: 8 additions & 3 deletions lib/Sema/TypeCheckEffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,7 @@ class ApplyClassifier {

public:
ASTContext &Ctx;
DeclContext *DC = nullptr;
DeclContext *RethrowsDC = nullptr;
DeclContext *ReasyncDC = nullptr;

Expand Down Expand Up @@ -1083,9 +1084,12 @@ class ApplyClassifier {

if (auto *var = dyn_cast<VarDecl>(decl)) {
ActorReferenceResult::Options options = llvm::None;
// The newly-diagnosed cases are invalid regardless of the module context
// of the caller, i.e. isolated static and global 'let' variables.
auto *module = var->getDeclContext()->getParentModule();
ModuleDecl *module;
if (DC != nullptr) {
module = DC->getParentModule();
} else {
module = var->getDeclContext()->getParentModule();
}
if (!isLetAccessibleAnywhere(module, var, options)) {
return options.contains(ActorReferenceResult::Flags::Preconcurrency);
}
Expand Down Expand Up @@ -3064,6 +3068,7 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>

ApplyClassifier getApplyClassifier() const {
ApplyClassifier classifier(Ctx);
classifier.DC = CurContext.getDeclContext();
classifier.RethrowsDC = RethrowsDC;
classifier.ReasyncDC = ReasyncDC;
return classifier;
Expand Down
14 changes: 9 additions & 5 deletions test/Concurrency/actor_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ func checkAsyncPropertyAccess() async {

act.text[0] += "hello" // expected-error{{actor-isolated property 'text' can not be mutated from a non-isolated context}}

_ = act.point // expected-warning{{non-sendable type 'Point' in asynchronous access to actor-isolated property 'point' cannot cross actor boundary}}
_ = act.point // expected-warning{{non-sendable type 'Point' in implicitly asynchronous access to actor-isolated property 'point' cannot cross actor boundary}}
// expected-warning@-1 {{expression is 'async' but is not marked with 'await'}}
// expected-note@-2 {{property access is 'async'}}
}

/// ------------------------------------------------------------------
Expand Down Expand Up @@ -1538,11 +1540,11 @@ class OverridesNonsiolatedInit: SuperWithNonisolatedInit {
}
}

// expected-note@+1 2 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}
// expected-note@+1 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}
class NonSendable {}

actor ProtectNonSendable {
// expected-note@+1 {{property declared here}}
// expected-note@+1 2 {{property declared here}}
let ns = NonSendable()

init() {}
Expand All @@ -1560,7 +1562,9 @@ class ReferenceActor {
init() async {
self.a = ProtectNonSendable()

// expected-warning@+1 {{non-sendable type 'NonSendable' in asynchronous access to actor-isolated property 'ns' cannot cross actor boundary}}
// expected-warning@+3 {{non-sendable type 'NonSendable' in implicitly asynchronous access to actor-isolated property 'ns' cannot cross actor boundary}}
// expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
// expected-note@+1 {{property access is 'async'}}
_ = a.ns
}
}
Expand All @@ -1571,7 +1575,7 @@ actor AnotherActor {
init() {
self.a = ProtectNonSendable()

// expected-warning@+1 {{non-sendable type 'NonSendable' in asynchronous access to actor-isolated property 'ns' cannot cross actor boundary}}
// expected-warning@+1 {{actor-isolated property 'ns' can not be referenced from a non-isolated context}}
_ = a.ns
}
}
Expand Down
8 changes: 4 additions & 4 deletions test/Concurrency/actor_keypath_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// REQUIRES: concurrency
// REQUIRES: asserts

class Box { // expected-note 3{{class 'Box' does not conform to the 'Sendable' protocol}}
class Box {
let size : Int = 0
}

Expand Down Expand Up @@ -48,7 +48,7 @@ func tryKeyPathsMisc(d : Door) {

// in combination with other key paths

_ = (\Door.letBox).appending(path: // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
_ = (\Door.letBox).appending(path: // expected-warning {{cannot form key path to actor-isolated property 'letBox'; this is an error in Swift 6}}
\Box?.?.size)

_ = (\Door.varBox).appending(path: // expected-error {{cannot form key path to actor-isolated property 'varBox'}}
Expand All @@ -62,9 +62,9 @@ func tryKeyPathsFromAsync() async {
}

func tryNonSendable() {
_ = \Door.letDict[0] // expected-warning {{cannot form key path that accesses non-sendable type '[Int : Box]'}}
_ = \Door.letDict[0] // expected-warning {{cannot form key path to actor-isolated property 'letDict'; this is an error in Swift 6}}
_ = \Door.varDict[0] // expected-error {{cannot form key path to actor-isolated property 'varDict'}}
_ = \Door.letBox!.size // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
_ = \Door.letBox!.size // expected-warning {{cannot form key path to actor-isolated property 'letBox'; this is an error in Swift 6}}
}

func tryKeypaths() {
Expand Down
8 changes: 4 additions & 4 deletions test/Concurrency/actor_keypath_isolation_swift6.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

// REQUIRES: concurrency && asserts

class Box { // expected-note 3{{class 'Box' does not conform to the 'Sendable' protocol}}
class Box {
let size : Int = 0
}

Expand Down Expand Up @@ -47,7 +47,7 @@ func tryKeyPathsMisc(d : Door) {

// in combination with other key paths

_ = (\Door.letBox).appending(path: // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
_ = (\Door.letBox).appending(path: // expected-error {{cannot form key path to actor-isolated property 'letBox'}}
\Box?.?.size)

_ = (\Door.varBox).appending(path: // expected-error {{cannot form key path to actor-isolated property 'varBox'}}
Expand All @@ -61,9 +61,9 @@ func tryKeyPathsFromAsync() async {
}

func tryNonSendable() {
_ = \Door.letDict[0] // expected-warning {{cannot form key path that accesses non-sendable type '[Int : Box]'}}
_ = \Door.letDict[0] // expected-error {{cannot form key path to actor-isolated property 'letDict'}}
_ = \Door.varDict[0] // expected-error {{cannot form key path to actor-isolated property 'varDict'}}
_ = \Door.letBox!.size // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
_ = \Door.letBox!.size // expected-error {{cannot form key path to actor-isolated property 'letBox'}}
}

func tryKeypaths() {
Expand Down
6 changes: 4 additions & 2 deletions test/Concurrency/concurrent_value_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ extension A1 {
_ = await self.asynchronous(nil)

// Across to a different actor, so Sendable restriction is enforced.
_ = other.localLet // expected-warning{{non-sendable type 'NotConcurrent' in asynchronous access to actor-isolated property 'localLet' cannot cross actor boundary}}
_ = other.localLet // expected-warning{{non-sendable type 'NotConcurrent' in implicitly asynchronous access to actor-isolated property 'localLet' cannot cross actor boundary}}
// expected-warning@-1 {{expression is 'async' but is not marked with 'await'}}
// expected-note@-2 {{property access is 'async'}}
_ = await other.synchronous() // expected-warning{{non-sendable type 'NotConcurrent?' returned by call to actor-isolated function cannot cross actor boundary}}
_ = await other.asynchronous(nil) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into actor-isolated context may introduce data races}}
}
Expand Down Expand Up @@ -230,7 +232,7 @@ func testKeyPaths(dict: [NC: Int], nc: NC) {
// Sendable restriction on nonisolated declarations.
// ----------------------------------------------------------------------
actor ANI {
nonisolated let nc = NC()
nonisolated let nc = NC() // expected-warning {{'nonisolated' can not be applied to variable with non-'Sendable' type 'NC'; this is an error in Swift 6}}
nonisolated func f() -> NC? { nil }
}

Expand Down
3 changes: 3 additions & 0 deletions test/Concurrency/flow_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,22 @@ actor ExampleFromProposal {
let immutableSendable = SendableType()
var mutableSendable = SendableType()
let nonSendable = NonSendableType()
nonisolated(unsafe) let unsafeNonSendable = NonSendableType()
var nsItems: [NonSendableType] = []
var sItems: [SendableType] = []

init() {
_ = self.immutableSendable // ok
_ = self.mutableSendable // ok
_ = self.nonSendable // ok
_ = self.unsafeNonSendable

f() // expected-note 2 {{after calling instance method 'f()', only non-isolated properties of 'self' can be accessed from this init}}

_ = self.immutableSendable // ok
_ = self.mutableSendable // expected-warning {{cannot access property 'mutableSendable' here in non-isolated initializer; this is an error in Swift 6}}
_ = self.nonSendable // expected-warning {{cannot access property 'nonSendable' here in non-isolated initializer; this is an error in Swift 6}}
_ = self.unsafeNonSendable // ok
}


Expand Down
3 changes: 2 additions & 1 deletion test/Concurrency/global_variables.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/GlobalVariables.swiftmodule -module-name GlobalVariables -parse-as-library -strict-concurrency=minimal -swift-version 5 %S/Inputs/GlobalVariables.swift
// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify %s
// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -swift-version 6 -I %t %s -emit-sil -o /dev/null -verify %s

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down Expand Up @@ -43,6 +43,7 @@ struct TestStatics {
static let immutableNonsendable = TestNonsendable() // expected-error{{static property 'immutableNonsendable' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor}}
static nonisolated(unsafe) let immutableNonisolatedUnsafe = TestNonsendable()
static nonisolated let immutableNonisolated = TestNonsendable() // expected-error{{static property 'immutableNonisolated' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor}}
// expected-error@-1 {{'nonisolated' can not be applied to variable with non-'Sendable' type 'TestNonsendable'}}
static let immutableInferredSendable = 0
static var mutable = 0 // expected-error{{static property 'mutable' is not concurrency-safe because it is non-isolated global shared mutable state}}
// expected-note@-1{{isolate 'mutable' to a global actor, or convert it to a 'let' constant and conform it to 'Sendable'}}
Expand Down