Skip to content

[Concurrency] Allow struct members to have isolation annotations. #72214

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
Mar 10, 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
6 changes: 0 additions & 6 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5489,9 +5489,6 @@ ERROR(distributed_actor_remote_func_implemented_manually,none,
ERROR(nonisolated_distributed_actor_storage,none,
"'nonisolated' can not be applied to distributed actor stored properties",
())
ERROR(nonisolated_storage_value_type,none,
"'nonisolated' is redundant on %0's stored properties",
(DescriptiveDeclKind))
ERROR(distributed_actor_func_nonisolated, none,
"cannot declare method %0 as both 'nonisolated' and 'distributed'",
(DeclName))
Expand Down Expand Up @@ -5768,9 +5765,6 @@ ERROR(global_actor_on_actor_class,none,
"actor %0 cannot have a global actor", (Identifier))
ERROR(global_actor_on_local_variable,none,
"local variable %0 cannot have a global actor", (DeclName))
ERROR(global_actor_on_storage_of_value_type,none,
"stored property %0 within struct cannot have a global actor",
(DeclName))
ERROR(unsafe_global_actor,none,
"'(unsafe)' global actors are deprecated; "
"use '@preconcurrency' instead",
Expand Down
11 changes: 0 additions & 11 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6909,17 +6909,6 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
diag::nonisolated_distributed_actor_storage);
return;
}

// 'nonisolated' is redundant for the stored properties of a struct.
if (isa<StructDecl>(nominal) &&
!var->isStatic() &&
var->isOrdinaryStoredProperty() &&
!isWrappedValueOfPropWrapper(var)) {
diagnoseAndRemoveAttr(attr, diag::nonisolated_storage_value_type,
nominal->getDescriptiveKind())
.warnUntilSwiftVersion(6);
return;
}
}
}

Expand Down
12 changes: 0 additions & 12 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,18 +421,6 @@ GlobalActorAttributeRequest::evaluate(
.highlight(globalActorAttr->getRangeWithAt());
return std::nullopt;
}

// ... and not if it's the instance storage of a struct
if (isStoredInstancePropertyOfStruct(var)) {
var->diagnose(diag::global_actor_on_storage_of_value_type,
var->getName())
.highlight(globalActorAttr->getRangeWithAt())
.warnUntilSwiftVersion(6);

// In Swift 6, once the diag above is an error, it is disallowed.
if (var->getASTContext().isSwiftVersionAtLeast(6))
return std::nullopt;
}
}
} else if (isa<ExtensionDecl>(decl)) {
// Extensions are okay.
Expand Down
8 changes: 3 additions & 5 deletions test/Concurrency/actor_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ struct InferredFromContext {
nonisolated var status: Bool = true // expected-error {{'nonisolated' cannot be applied to mutable stored properties}}{{3-15=}}{{3-15=}}{{14-14=(unsafe)}}
// expected-note@-1{{convert 'status' to a 'let' constant or consider declaring it 'nonisolated(unsafe)' if manually managing concurrency safety}}

nonisolated let flag: Bool = false // expected-warning {{'nonisolated' is redundant on struct's stored properties; this is an error in the Swift 6 language mode}}{{3-15=}}
nonisolated let flag: Bool = false

subscript(_ i: Int) -> Int { return i }

Expand All @@ -170,15 +170,13 @@ func checkIsolationValueType(_ formance: InferredFromConformance,
_ = await NoGlobalActorValueType.polygon // expected-warning {{non-sendable type '[Point]' in implicitly asynchronous access to main actor-isolated static property 'polygon' cannot cross actor boundary}}
}

// check for instance members that do not need global-actor protection

// expected-warning@+2 {{memberwise initializer for 'NoGlobalActorValueType' cannot be both nonisolated and global actor 'SomeGlobalActor'-isolated; this is an error in the Swift 6 language mode}}
// 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 the Swift 6 language mode}}
@SomeGlobalActor var point: Point
// expected-note@-1 {{initializer for property 'point' is global actor 'SomeGlobalActor'-isolated}}

@MainActor let counter: Int // expected-warning {{stored property 'counter' within struct cannot have a global actor; this is an error in the Swift 6 language mode}}
@MainActor let counter: Int

@MainActor static var polygon: [Point] = []
}
Expand Down
16 changes: 10 additions & 6 deletions test/Concurrency/actor_isolation_swift6.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct InferredFromContext {
get { [] }
}

nonisolated let flag: Bool = false // expected-error {{'nonisolated' is redundant on struct's stored properties}}{{3-15=}}
nonisolated let flag: Bool = false

subscript(_ i: Int) -> Int { return i }

Expand All @@ -49,15 +49,20 @@ func checkIsolationValueType(_ formance: InferredFromConformance,
// these do not need an await, since it's a value type
_ = ext.point
_ = formance.counter
_ = anno.point
_ = anno.counter

// make sure it's just a warning if someone was awaiting on it previously
_ = await ext.point // expected-warning {{no 'async' operations occur within 'await' expression}}
_ = await formance.counter // expected-warning {{no 'async' operations occur within 'await' expression}}
_ = await anno.point // expected-warning {{no 'async' operations occur within 'await' expression}}
_ = await anno.counter // expected-warning {{no 'async' operations occur within 'await' expression}}

// We could extend the 'nonisolated within the module' rule to vars
// value types types if the property type is 'Sendable'.
_ = anno.point
// expected-error@-1 {{expression is 'async' but is not marked with 'await'}}
// expected-note@-2 {{property access is 'async'}}
_ = await anno.point

// these do need await, regardless of reference or value type
_ = await (formance as any MainCounter).counter
// expected-error@-1 {{non-sendable type 'any MainCounter' passed in implicitly asynchronous call to main actor-isolated property 'counter' cannot cross actor boundary}}
Expand All @@ -68,11 +73,10 @@ func checkIsolationValueType(_ formance: InferredFromConformance,
_ = await NoGlobalActorValueType.polygon
}

// check for instance members that do not need global-actor protection
struct NoGlobalActorValueType {
@SomeGlobalActor var point: ImmutablePoint // expected-error {{stored property 'point' within struct cannot have a global actor}}
@SomeGlobalActor var point: ImmutablePoint

@MainActor let counter: Int // expected-error {{stored property 'counter' within struct cannot have a global actor}}
@MainActor let counter: Int

@MainActor static var polygon: [ImmutablePoint] = []
}
Expand Down
2 changes: 1 addition & 1 deletion test/attr/global_actor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct OtherGlobalActor {
}

@GA1 struct X {
@GA1 var member: Int // expected-warning {{stored property 'member' within struct cannot have a global actor; this is an error in the Swift 6 language mode}}
@GA1 var member: Int
}

struct Y {
Expand Down