Skip to content

Commit bd024ca

Browse files
kavonkavon
committed
[SE-327] Remove redundant global-actor isolation.
As part of SE-327, global-actor isolation applied to the instance-stored properties of a value type do not require any isolation, since there is no way to create a race on access to that storage. https://github.com/apple/swift-evolution/blob/main/proposals/0327-actor-initializers.md#removing-redundant-isolation This change turns global-actor annotations on such properties into an error in Swift 6+, and a warning in Swift 5 and earlier. In addition, inference for global-actor isolation no longer applies global-actor isolation to such properties. Since this latter change only results in warnings in existing Swift 5 code, about a now superflous 'await', this change will happen in Swift 5+. Fixes rdar://87568381
1 parent 5cc85b4 commit bd024ca

File tree

9 files changed

+183
-33
lines changed

9 files changed

+183
-33
lines changed

‎include/swift/AST/Decl.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3538,6 +3538,14 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
35383538
/// Whether this nominal type qualifies as any actor (plain or distributed).
35393539
bool isAnyActor() const;
35403540

3541+
/// Whether this nominal type corresponds to a language-level value type
3542+
/// (i.e., structs and enums), meaning that a shallow copy is created when
3543+
/// reassigning an instance member of the type. That is in contrast with
3544+
/// reference types like classes and actors, where such mutations are
3545+
/// reflected for all instance holders. A protocol is not considered a
3546+
/// value type, even though their conformers might be a value type.
3547+
bool isValueType() const;
3548+
35413549
/// Return the range of semantics attributes attached to this NominalTypeDecl.
35423550
auto getSemanticsAttrs() const
35433551
-> decltype(getAttrs().getSemanticsAttrs()) {
@@ -5238,6 +5246,10 @@ class VarDecl : public AbstractStorageDecl {
52385246
/// Is this an "async let" property?
52395247
bool isAsyncLet() const;
52405248

5249+
/// Is this a stored property that will _not_ trigger any user-defined code
5250+
/// upon any kind of access?
5251+
bool isOrdinaryStoredProperty() const;
5252+
52415253
Introducer getIntroducer() const {
52425254
return Introducer(Bits.VarDecl.Introducer);
52435255
}

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4573,6 +4573,9 @@ ERROR(distributed_actor_remote_func_implemented_manually,none,
45734573
ERROR(nonisolated_distributed_actor_storage,none,
45744574
"'nonisolated' can not be applied to distributed actor stored properties",
45754575
())
4576+
ERROR(nonisolated_storage_value_type,none,
4577+
"'nonisolated' is redundant on %0's stored properties",
4578+
(DescriptiveDeclKind))
45764579
ERROR(distributed_actor_func_nonisolated, none,
45774580
"cannot declare method %0 as both 'nonisolated' and 'distributed'",
45784581
(DeclName))
@@ -4773,6 +4776,9 @@ ERROR(global_actor_on_actor_class,none,
47734776
"actor %0 cannot have a global actor", (Identifier))
47744777
ERROR(global_actor_on_local_variable,none,
47754778
"local variable %0 cannot have a global actor", (DeclName))
4779+
ERROR(global_actor_on_storage_of_value_type,none,
4780+
"stored property %0 within %1 cannot have a global actor",
4781+
(DeclName, DescriptiveDeclKind))
47764782
ERROR(global_actor_non_unsafe_init,none,
47774783
"global actor attribute %0 argument can only be '(unsafe)'", (Type))
47784784
ERROR(global_actor_non_final_class,none,

‎lib/AST/Decl.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4286,6 +4286,10 @@ bool NominalTypeDecl::isAnyActor() const {
42864286
return isActor() || isDistributedActor();
42874287
}
42884288

4289+
bool NominalTypeDecl::isValueType() const {
4290+
return isa<StructDecl>(this) || isa<EnumDecl>(this);
4291+
}
4292+
42894293
GenericTypeDecl::GenericTypeDecl(DeclKind K, DeclContext *DC,
42904294
Identifier name, SourceLoc nameLoc,
42914295
ArrayRef<InheritedEntry> inherited,
@@ -6280,6 +6284,16 @@ bool VarDecl::isAsyncLet() const {
62806284
return getAttrs().hasAttribute<AsyncAttr>();
62816285
}
62826286

6287+
bool VarDecl::isOrdinaryStoredProperty() const {
6288+
// we assume if it hasAttachedPropertyWrapper, it has no storage.
6289+
//
6290+
// also, we don't expect someone to call this on a local property, so for
6291+
// efficiency we don't check if it's not async-let. feel free to promote
6292+
// the assert into a full-fledged part of the condition if needed.
6293+
assert(!isAsyncLet());
6294+
return hasStorage() && !hasObservers();
6295+
}
6296+
62836297
void ParamDecl::setSpecifier(Specifier specifier) {
62846298
// FIXME: Revisit this; in particular shouldn't __owned parameters be
62856299
// ::Let also?

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5605,29 +5605,39 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
56055605
if (auto var = dyn_cast<VarDecl>(D)) {
56065606
// stored properties have limitations as to when they can be nonisolated.
56075607
if (var->hasStorage()) {
5608-
auto nominal = dyn_cast<NominalTypeDecl>(dc);
5609-
5610-
// 'nonisolated' can not be applied to stored properties inside
5611-
// distributed actors. Attempts of nonisolated access would be
5612-
// cross-actor, and that means they might be accessing on a remote actor,
5613-
// in which case the stored property storage does not exist.
5614-
//
5615-
// The synthesized "id" and "actorSystem" are the only exceptions,
5616-
// because the implementation mirrors them.
5617-
if (nominal && nominal->isDistributedActor() &&
5618-
!(var->isImplicit() &&
5619-
(var->getName() == Ctx.Id_id ||
5620-
var->getName() == Ctx.Id_actorSystem))) {
5621-
diagnoseAndRemoveAttr(attr,
5622-
diag::nonisolated_distributed_actor_storage);
5623-
return;
5624-
}
56255608

56265609
// 'nonisolated' can not be applied to mutable stored properties.
56275610
if (var->supportsMutation()) {
56285611
diagnoseAndRemoveAttr(attr, diag::nonisolated_mutable_storage);
56295612
return;
56305613
}
5614+
5615+
if (auto nominal = dyn_cast<NominalTypeDecl>(dc)) {
5616+
// 'nonisolated' can not be applied to stored properties inside
5617+
// distributed actors. Attempts of nonisolated access would be
5618+
// cross-actor, which means they might be accessing on a remote actor,
5619+
// in which case the stored property storage does not exist.
5620+
//
5621+
// The synthesized "id" and "actorSystem" are the only exceptions,
5622+
// because the implementation mirrors them.
5623+
if (nominal->isDistributedActor() &&
5624+
!(var->isImplicit() &&
5625+
(var->getName() == Ctx.Id_id ||
5626+
var->getName() == Ctx.Id_actorSystem))) {
5627+
diagnoseAndRemoveAttr(attr,
5628+
diag::nonisolated_distributed_actor_storage);
5629+
return;
5630+
}
5631+
5632+
// 'nonisolated' is redundant for the stored properties of a value type.
5633+
if (nominal->isValueType() && !var->isStatic() &&
5634+
var->isOrdinaryStoredProperty()) {
5635+
diagnoseAndRemoveAttr(attr, diag::nonisolated_storage_value_type,
5636+
nominal->getDescriptiveKind())
5637+
.warnUntilSwiftVersion(6);
5638+
return;
5639+
}
5640+
}
56315641
}
56325642

56335643
// nonisolated can not be applied to local properties.

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,15 +341,33 @@ GlobalActorAttributeRequest::evaluate(
341341
} else if (auto storage = dyn_cast<AbstractStorageDecl>(decl)) {
342342
// Subscripts and properties are fine...
343343
if (auto var = dyn_cast<VarDecl>(storage)) {
344+
345+
// ... but not if it's an async-context top-level global
344346
if (var->isTopLevelGlobal() && var->getDeclContext()->isAsyncContext()) {
345347
var->diagnose(diag::global_actor_top_level_var)
346348
.highlight(globalActorAttr->getRangeWithAt());
347349
return None;
348-
} else if (var->getDeclContext()->isLocalContext()) {
350+
}
351+
352+
// ... and not if it's local property
353+
if (var->getDeclContext()->isLocalContext()) {
349354
var->diagnose(diag::global_actor_on_local_variable, var->getName())
350355
.highlight(globalActorAttr->getRangeWithAt());
351356
return None;
352357
}
358+
359+
// ... and not if it's the instance storage of a value type
360+
if (!var->isStatic() && var->isOrdinaryStoredProperty()) {
361+
if (auto *nominal = var->getDeclContext()->getSelfNominalTypeDecl()) {
362+
if (nominal->isValueType()) {
363+
var->diagnose(diag::global_actor_on_storage_of_value_type,
364+
var->getName(), nominal->getDescriptiveKind())
365+
.highlight(globalActorAttr->getRangeWithAt())
366+
.warnUntilSwiftVersion(6);
367+
return None;
368+
}
369+
}
370+
}
353371
}
354372
} else if (isa<ExtensionDecl>(decl)) {
355373
// Extensions are okay.
@@ -3604,6 +3622,14 @@ ActorIsolation ActorIsolationRequest::evaluate(
36043622

36053623
case ActorIsolation::GlobalActorUnsafe:
36063624
case ActorIsolation::GlobalActor: {
3625+
// Stored properties of a value type don't need global-actor isolation.
3626+
if (auto *var = dyn_cast<VarDecl>(value))
3627+
if (!var->isStatic() && var->isOrdinaryStoredProperty())
3628+
if (auto *varDC = var->getDeclContext())
3629+
if (auto *nominal = varDC->getSelfNominalTypeDecl())
3630+
if (nominal->isValueType())
3631+
return ActorIsolation::forUnspecified();
3632+
36073633
auto typeExpr = TypeExpr::createImplicit(inferred.getGlobalActor(), ctx);
36083634
auto attr = CustomAttr::create(
36093635
ctx, SourceLoc(), typeExpr, /*implicit=*/true);

‎test/Concurrency/actor_isolation.swift

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ actor MySuperActor {
4646
}
4747
}
4848

49-
class Point { // expected-note{{class 'Point' does not conform to the 'Sendable' protocol}}
49+
class Point { // expected-note 3{{class 'Point' does not conform to the 'Sendable' protocol}}
5050
var x : Int = 0
5151
var y : Int = 0
5252
}
@@ -109,6 +109,78 @@ func checkAsyncPropertyAccess() async {
109109
_ = act.point // expected-warning{{non-sendable type 'Point' in asynchronous access to actor-isolated property 'point' cannot cross actor boundary}}
110110
}
111111

112+
/// ------------------------------------------------------------------
113+
/// -- Value types do not need isolation on their stored properties --
114+
115+
@available(SwiftStdlib 5.1, *)
116+
protocol MainCounter {
117+
@MainActor var counter: Int { get set }
118+
@MainActor var ticker: Int { get set }
119+
}
120+
121+
@available(SwiftStdlib 5.1, *)
122+
struct InferredFromConformance: MainCounter {
123+
var counter = 0
124+
var ticker: Int {
125+
get { 1 }
126+
set {}
127+
}
128+
}
129+
130+
@MainActor
131+
@available(SwiftStdlib 5.1, *)
132+
struct InferredFromContext {
133+
var point = Point()
134+
var polygon: [Point] {
135+
get { [] }
136+
}
137+
138+
nonisolated var status: Bool = true // expected-error {{'nonisolated' can not be applied to stored properties}}{{3-15=}}
139+
140+
nonisolated let flag: Bool = false // expected-warning {{'nonisolated' is redundant on struct's stored properties; this is an error in Swift 6}}{{3-15=}}
141+
142+
subscript(_ i: Int) -> Int { return i }
143+
144+
static var stuff: [Int] = []
145+
}
146+
147+
@available(SwiftStdlib 5.1, *)
148+
func checkIsolationValueType(_ formance: InferredFromConformance,
149+
_ ext: InferredFromContext,
150+
_ anno: NoGlobalActorValueType) async {
151+
// these do not need an await, since it's a value type
152+
_ = ext.point
153+
_ = formance.counter
154+
_ = anno.point
155+
_ = anno.counter
156+
157+
// make sure it's just a warning if someone was awaiting on it previously
158+
_ = await ext.point // expected-warning {{no 'async' operations occur within 'await' expression}}
159+
_ = await formance.counter // expected-warning {{no 'async' operations occur within 'await' expression}}
160+
_ = await anno.point // expected-warning {{no 'async' operations occur within 'await' expression}}
161+
_ = await anno.counter // expected-warning {{no 'async' operations occur within 'await' expression}}
162+
163+
// these do need await, regardless of reference or value type
164+
_ = await (formance as MainCounter).counter
165+
_ = await ext[1]
166+
_ = await formance.ticker
167+
_ = await ext.polygon // expected-warning {{non-sendable type '[Point]' in implicitly asynchronous access to main actor-isolated property 'polygon' cannot cross actor boundary}}
168+
_ = await InferredFromContext.stuff
169+
_ = await NoGlobalActorValueType.polygon // expected-warning {{non-sendable type '[Point]' in implicitly asynchronous access to main actor-isolated static property 'polygon' cannot cross actor boundary}}
170+
}
171+
172+
// check for instance members that do not need global-actor protection
173+
@available(SwiftStdlib 5.1, *)
174+
struct NoGlobalActorValueType {
175+
@SomeGlobalActor var point: Point // expected-warning {{stored property 'point' within struct cannot have a global actor; this is an error in Swift 6}}
176+
177+
@MainActor let counter: Int // expected-warning {{stored property 'counter' within struct cannot have a global actor; this is an error in Swift 6}}
178+
179+
@MainActor static var polygon: [Point] = []
180+
}
181+
182+
/// -----------------------------------------------------------------
183+
112184
@available(SwiftStdlib 5.1, *)
113185
extension MyActor {
114186
nonisolated var actorIndependentVar: Int {

‎test/Concurrency/global_actor_inference.swift

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,22 @@ func barSync() {
262262
foo() // expected-error {{call to global actor 'SomeGlobalActor'-isolated global function 'foo()' in a synchronous nonisolated context}}
263263
}
264264

265+
// ----------------------------------------------------------------------
266+
// Property observers
267+
// ----------------------------------------------------------------------
268+
269+
@OtherGlobalActor
270+
struct Observed {
271+
var thing: Int = 0 { // expected-note {{property declared here}}
272+
didSet {}
273+
willSet {}
274+
}
275+
}
276+
277+
func checkObserved(_ o: Observed) { // expected-note {{add '@OtherGlobalActor' to make global function 'checkObserved' part of global actor 'OtherGlobalActor'}}
278+
_ = o.thing // expected-error {{property 'thing' isolated to global actor 'OtherGlobalActor' can not be referenced from this synchronous context}}
279+
}
280+
265281
// ----------------------------------------------------------------------
266282
// Property wrappers
267283
// ----------------------------------------------------------------------
@@ -318,13 +334,13 @@ actor WrapperActor<Wrapped: Sendable> {
318334

319335
struct HasWrapperOnActor {
320336
@WrapperOnActor var synced: Int = 0
321-
// expected-note@-1 3{{property declared here}}
337+
// expected-note@-1 2{{property declared here}}
322338

323-
// expected-note@+1 3{{to make instance method 'testErrors()'}}
339+
// expected-note@+1 2{{to make instance method 'testErrors()'}}
324340
func testErrors() {
325341
_ = synced // expected-error{{property 'synced' isolated to global actor 'MainActor' can not be referenced from this synchronous context}}
326342
_ = $synced // expected-error{{property '$synced' isolated to global actor 'SomeGlobalActor' can not be referenced from this synchronous context}}
327-
_ = _synced // expected-error{{property '_synced' isolated to global actor 'OtherGlobalActor' can not be referenced from this synchronous context}}
343+
_ = _synced
328344
}
329345

330346
@MainActor mutating func testOnMain() {
@@ -483,7 +499,7 @@ struct WrapperOnUnsafeActor<Wrapped> {
483499

484500
struct HasWrapperOnUnsafeActor {
485501
@WrapperOnUnsafeActor var synced: Int = 0
486-
// expected-note@-1 3{{property declared here}}
502+
// expected-note@-1 2{{property declared here}}
487503

488504
func testUnsafeOkay() {
489505
_ = synced
@@ -494,7 +510,7 @@ struct HasWrapperOnUnsafeActor {
494510
nonisolated func testErrors() {
495511
_ = synced // expected-error{{property 'synced' isolated to global actor 'MainActor' can not be referenced from}}
496512
_ = $synced // expected-error{{property '$synced' isolated to global actor 'SomeGlobalActor' can not be referenced from}}
497-
_ = _synced // expected-error{{property '_synced' isolated to global actor 'OtherGlobalActor' can not be referenced from}}
513+
_ = _synced
498514
}
499515

500516
@MainActor mutating func testOnMain() {

‎test/SILGen/hop_to_executor_async_prop.swift

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -602,10 +602,8 @@ struct Container {
602602
// CHECK: [[SOME_BB]]:
603603
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr [[ACCESS]] : $*Optional<Container>, #Optional.some!enumelt
604604
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr [[DATA_ADDR]] : $*Container, #Container.iso
605-
// CHECK: hop_to_executor {{%[0-9]+}} : $Cat
606605
// CHECK: {{%[0-9]+}} = load [trivial] [[ELEM_ADDR]] : $*Float
607-
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
608-
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
606+
// CHECK: hop_to_executor [[PREV]] : $Optional<Builtin.Executor>
609607
// CHECK: } // end sil function '$s4test9ContainerV10getOrCrashSfyYaFZ'
610608
static func getOrCrash() async -> Float {
611609
return await this!.iso
@@ -630,10 +628,8 @@ struct Container {
630628
// CHECK: [[SOME_BB]]:
631629
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr [[ACCESS]] : $*Optional<Container>, #Optional.some!enumelt
632630
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr [[DATA_ADDR]] : $*Container, #Container.iso
633-
// CHECK: hop_to_executor {{%[0-9]+}} : $Cat
634631
// CHECK: {{%[0-9]+}} = load [copy] [[ELEM_ADDR]] : $*CatBox
635-
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
636-
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
632+
// CHECK: hop_to_executor [[PREV]] : $Optional<Builtin.Executor>
637633
// CHECK: } // end sil function '$s4test9ContainerV13getRefOrCrashAA6CatBoxCyYaFZ'
638634
static func getRefOrCrash() async -> CatBox {
639635
return await this!.isoRef
@@ -705,5 +701,3 @@ class Polar {
705701
func accessStaticIsolated() async -> Int {
706702
return await Polar.temperature
707703
}
708-
709-

‎test/attr/global_actor.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ struct OtherGlobalActor {
6767
}
6868

6969
@GA1 struct X {
70-
@GA1 var member: Int
70+
@GA1 var member: Int // expected-warning {{stored property 'member' within struct cannot have a global actor; this is an error in Swift 6}}
7171
}
7272

7373
struct Y {

0 commit comments

Comments
 (0)