From 20829faa4325d83c424e7be2e8056953bc91f6ac Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Fri, 19 Jan 2024 18:05:13 -0800 Subject: [PATCH 1/3] [Concurrency] Handle cases where a property initializer is subsumed by another property for IsolatedDefaultValues. For property wrappers and init accesors, skip property initializers that are subsumed, e.g. by an init accessor or a backing property wrapper initializer, and always consider the subsuming initializer to determine whether compiler synthesized initializers should have `nonisolated` applied. This change also lessens the source break of SE-0411 by still emitting member initializers in implicit constructors when the initializer violates actor isolation to preserve the behavior of existing code when concurrency diagnostics are downgraded to warnings in Swift 5 mode. --- lib/SILGen/SILGenConstructor.cpp | 15 +++++- lib/Sema/CodeSynthesis.cpp | 51 ++++++++++++++----- test/Concurrency/actor_isolation.swift | 3 ++ test/Concurrency/global_actor_inference.swift | 14 ++++- .../isolated_default_arguments.swift | 47 ++++++++++++++--- 5 files changed, 107 insertions(+), 23 deletions(-) diff --git a/lib/SILGen/SILGenConstructor.cpp b/lib/SILGen/SILGenConstructor.cpp index 820ad973ad5a5..110f63605e147 100644 --- a/lib/SILGen/SILGenConstructor.cpp +++ b/lib/SILGen/SILGenConstructor.cpp @@ -1583,9 +1583,20 @@ void SILGenFunction::emitMemberInitializer(DeclContext *dc, VarDecl *selfDecl, case ActorIsolation::GlobalActor: case ActorIsolation::GlobalActorUnsafe: - case ActorIsolation::ActorInstance: - if (requiredIsolation != contextIsolation) + case ActorIsolation::ActorInstance: { + if (requiredIsolation != contextIsolation) { + // Implicit initializers diagnose actor isolation violations + // for property initializers in Sema. Still emit the invalid + // member initializer here to avoid duplicate diagnostics and + // to preserve warn-until-Swift-6 behavior. + auto *init = + dyn_cast_or_null(dc->getAsDecl()); + if (init && init->isImplicit()) + break; + continue; + } + } } auto *varPattern = field->getPattern(i); diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 5a65c2c5c2f0d..1d2188e36f19f 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -351,34 +351,59 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, // and actor initializers do not run on the actor, so initial values // cannot be actor-instance-isolated. bool shouldAddNonisolated = true; - llvm::Optional existingIsolation = llvm::None; + ActorIsolation existingIsolation = getActorIsolation(decl); VarDecl *previousVar = nullptr; - // The memberwise init properties are also effectively what the - // default init uses, e.g. default initializers initialize via - // properties wrapped and init accessors. - for (auto var : decl->getMemberwiseInitProperties()) { + for (auto member : decl->getImplementationContext()->getAllMembers()) { + auto pbd = dyn_cast(member); + if (!pbd || pbd->isStatic()) + continue; + + auto *var = pbd->getSingleVar(); + if (!var) { + shouldAddNonisolated = false; + break; + } + + auto i = pbd->getPatternEntryIndexForVarDecl(var); + if (pbd->isInitializerSubsumed(i)) + continue; + + ActorIsolation initIsolation; + if (var->hasInitAccessor()) { + // Init accessors share the actor isolation of the property; + // the accessor body can call anything in that isolation domain, + // and we don't attempt to infer when the isolation isn't + // necessary. + initIsolation = getActorIsolation(var); + } else { + initIsolation = var->getInitializerIsolation(); + } + auto type = var->getTypeInContext(); auto isolation = getActorIsolation(var); if (isolation.isGlobalActor()) { if (!type->isSendableType() || - var->getInitializerIsolation().isGlobalActor()) { + initIsolation.isGlobalActor()) { // If different isolated stored properties require different // global actors, it is impossible to initialize this type. - if (existingIsolation && - *existingIsolation != isolation) { + if (existingIsolation != isolation) { ctx.Diags.diagnose(decl->getLoc(), diag::conflicting_stored_property_isolation, ICK == ImplicitConstructorKind::Memberwise, - decl->getDeclaredType(), *existingIsolation, isolation); - previousVar->diagnose( - diag::property_requires_actor, - previousVar->getDescriptiveKind(), - previousVar->getName(), *existingIsolation); + decl->getDeclaredType(), existingIsolation, isolation) + .warnUntilSwiftVersion(6); + if (previousVar) { + previousVar->diagnose( + diag::property_requires_actor, + previousVar->getDescriptiveKind(), + previousVar->getName(), existingIsolation); + } var->diagnose( diag::property_requires_actor, var->getDescriptiveKind(), var->getName(), isolation); + break; } existingIsolation = isolation; diff --git a/test/Concurrency/actor_isolation.swift b/test/Concurrency/actor_isolation.swift index bb621ef8af440..77e33acefac5e 100644 --- a/test/Concurrency/actor_isolation.swift +++ b/test/Concurrency/actor_isolation.swift @@ -168,9 +168,12 @@ func checkIsolationValueType(_ formance: InferredFromConformance, } // 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 Swift 6}} // 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}} + // 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 Swift 6}} diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift index eb676d24c78c7..c8f49f790745e 100644 --- a/test/Concurrency/global_actor_inference.swift +++ b/test/Concurrency/global_actor_inference.swift @@ -559,9 +559,13 @@ struct WrapperOnUnsafeActor { } } -// HasWrapperOnUnsafeActor gets an inferred @MainActor attribute. +// HasWrapperOnUnsafeActor does not have an inferred global actor attribute, +// because synced and $synced have different global actors. struct HasWrapperOnUnsafeActor { - @WrapperOnUnsafeActor var synced: Int = 0 // expected-complete-tns-warning {{global actor 'OtherGlobalActor'-isolated default value in a main actor-isolated context; this is an error in Swift 6}} +// expected-complete-tns-warning@-1 {{memberwise initializer for 'HasWrapperOnUnsafeActor' cannot be both nonisolated and global actor 'OtherGlobalActor'-isolated; this is an error in Swift 6}} +// expected-complete-tns-warning@-2 {{default initializer for 'HasWrapperOnUnsafeActor' cannot be both nonisolated and global actor 'OtherGlobalActor'-isolated; this is an error in Swift 6}} + + @WrapperOnUnsafeActor var synced: Int = 0 // expected-complete-tns-note 2 {{initializer for property '_synced' is global actor 'OtherGlobalActor'-isolated}} // expected-note @-1 3{{property declared here}} // expected-complete-tns-note @-2 3{{property declared here}} @@ -586,6 +590,10 @@ struct HasWrapperOnUnsafeActor { } } +nonisolated func createHasWrapperOnUnsafeActor() { + _ = HasWrapperOnUnsafeActor() +} + // ---------------------------------------------------------------------- // Nonisolated closures // ---------------------------------------------------------------------- @@ -670,7 +678,9 @@ func replacesDynamicOnMainActor() { // Global-actor isolation of stored property initializer expressions // ---------------------------------------------------------------------- +// expected-complete-tns-warning@+1 {{default initializer for 'Cutter' cannot be both nonisolated and main actor-isolated; this is an error in Swift 6}} class Cutter { + // expected-complete-tns-note@+1 {{initializer for property 'x' is main actor-isolated}} @MainActor var x = useFooInADefer() @MainActor var y = { () -> Bool in var z = statefulThingy diff --git a/test/Concurrency/isolated_default_arguments.swift b/test/Concurrency/isolated_default_arguments.swift index 73935a29ee861..1ea6d7bb8684a 100644 --- a/test/Concurrency/isolated_default_arguments.swift +++ b/test/Concurrency/isolated_default_arguments.swift @@ -1,7 +1,5 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking - // RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -enable-upcoming-feature IsolatedDefaultValues -parse-as-library -emit-sil -o /dev/null -verify %s // RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library -emit-sil -o /dev/null -verify -enable-upcoming-feature IsolatedDefaultValues -enable-experimental-feature RegionBasedIsolation %s @@ -196,21 +194,19 @@ extension A { } } -// expected-error@+1 {{default initializer for 'C1' cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}} +// expected-warning@+1 {{default initializer for 'C1' cannot be both nonisolated and main actor-isolated; this is an error in Swift 6}} class C1 { // expected-note@+1 {{initializer for property 'x' is main actor-isolated}} @MainActor var x = requiresMainActor() - // expected-note@+1 {{initializer for property 'y' is global actor 'SomeGlobalActor'-isolated}} @SomeGlobalActor var y = requiresSomeGlobalActor() } class NonSendable {} -// expected-error@+1 {{default initializer for 'C2' cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}} +// expected-warning@+1 {{default initializer for 'C2' cannot be both nonisolated and main actor-isolated; this is an error in Swift 6}} class C2 { // expected-note@+1 {{initializer for property 'x' is main actor-isolated}} @MainActor var x = NonSendable() - // expected-note@+1 {{initializer for property 'y' is global actor 'SomeGlobalActor'-isolated}} @SomeGlobalActor var y = NonSendable() } @@ -229,3 +225,42 @@ func callDefaultInit() async { _ = NonIsolatedInit() _ = NonIsolatedInit(x: 10) } + +@propertyWrapper +@preconcurrency @MainActor +struct RequiresMain { + var wrappedValue: Value + + init(wrappedValue: Value) { + self.wrappedValue = wrappedValue + } +} + +// This is okay; UseRequiresMain has an inferred 'MainActor' +// attribute. +struct UseRequiresMain { + @RequiresMain private var x = 10 +} + +nonisolated func test() async { + // expected-warning@+2 {{expression is 'async' but is not marked with 'await'; this is an error in Swift 6}} + // expected-note@+1 {{calls to initializer 'init()' from outside of its actor context are implicitly asynchronous}} + _ = UseRequiresMain() +} + +// expected-warning@+2 {{memberwise initializer for 'InitAccessors' cannot be both nonisolated and main actor-isolated; this is an error in Swift 6}} +// expected-warning@+1 {{default initializer for 'InitAccessors' cannot be both nonisolated and main actor-isolated; this is an error in Swift 6}} +struct InitAccessors { + private var _a: Int + + // expected-note@+1 2 {{initializer for property 'a' is main actor-isolated}} + @MainActor var a: Int = 5 { + @storageRestrictions(initializes: _a) + init { + _a = requiresMainActor() + } + get { + _a + } + } +} From cc1fd1ce8fb13518f8d7f42e37c2e995d03c3b23 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Sat, 20 Jan 2024 10:03:51 -0800 Subject: [PATCH 2/3] [Concurrency] Don't skip pattern bindings with multiple vars when inferring the isolation of implicit initializers. --- lib/Sema/CodeSynthesis.cpp | 94 ++++++++++--------- .../isolated_default_arguments.swift | 11 +++ 2 files changed, 59 insertions(+), 46 deletions(-) diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 1d2188e36f19f..a4f519e2058b1 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -355,62 +355,64 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, VarDecl *previousVar = nullptr; for (auto member : decl->getImplementationContext()->getAllMembers()) { + bool hasError = false; auto pbd = dyn_cast(member); if (!pbd || pbd->isStatic()) continue; - auto *var = pbd->getSingleVar(); - if (!var) { - shouldAddNonisolated = false; - break; - } - - auto i = pbd->getPatternEntryIndexForVarDecl(var); - if (pbd->isInitializerSubsumed(i)) - continue; - - ActorIsolation initIsolation; - if (var->hasInitAccessor()) { - // Init accessors share the actor isolation of the property; - // the accessor body can call anything in that isolation domain, - // and we don't attempt to infer when the isolation isn't - // necessary. - initIsolation = getActorIsolation(var); - } else { - initIsolation = var->getInitializerIsolation(); - } + for (auto i : range(pbd->getNumPatternEntries())) { + if (pbd->isInitializerSubsumed(i)) + continue; + + auto *var = pbd->getAnchoringVarDecl(i); + + ActorIsolation initIsolation; + if (var->hasInitAccessor()) { + // Init accessors share the actor isolation of the property; + // the accessor body can call anything in that isolation domain, + // and we don't attempt to infer when the isolation isn't + // necessary. + initIsolation = getActorIsolation(var); + } else { + initIsolation = var->getInitializerIsolation(); + } - auto type = var->getTypeInContext(); - auto isolation = getActorIsolation(var); - if (isolation.isGlobalActor()) { - if (!type->isSendableType() || - initIsolation.isGlobalActor()) { - // If different isolated stored properties require different - // global actors, it is impossible to initialize this type. - if (existingIsolation != isolation) { - ctx.Diags.diagnose(decl->getLoc(), - diag::conflicting_stored_property_isolation, - ICK == ImplicitConstructorKind::Memberwise, - decl->getDeclaredType(), existingIsolation, isolation) - .warnUntilSwiftVersion(6); - if (previousVar) { - previousVar->diagnose( + auto type = var->getTypeInContext(); + auto isolation = getActorIsolation(var); + if (isolation.isGlobalActor()) { + if (!type->isSendableType() || + initIsolation.isGlobalActor()) { + // If different isolated stored properties require different + // global actors, it is impossible to initialize this type. + if (existingIsolation != isolation) { + ctx.Diags.diagnose(decl->getLoc(), + diag::conflicting_stored_property_isolation, + ICK == ImplicitConstructorKind::Memberwise, + decl->getDeclaredType(), existingIsolation, isolation) + .warnUntilSwiftVersion(6); + if (previousVar) { + previousVar->diagnose( + diag::property_requires_actor, + previousVar->getDescriptiveKind(), + previousVar->getName(), existingIsolation); + } + var->diagnose( diag::property_requires_actor, - previousVar->getDescriptiveKind(), - previousVar->getName(), existingIsolation); + var->getDescriptiveKind(), + var->getName(), isolation); + hasError = true; + break; } - var->diagnose( - diag::property_requires_actor, - var->getDescriptiveKind(), - var->getName(), isolation); - break; - } - existingIsolation = isolation; - previousVar = var; - shouldAddNonisolated = false; + existingIsolation = isolation; + previousVar = var; + shouldAddNonisolated = false; + } } } + + if (hasError) + break; } if (shouldAddNonisolated) { diff --git a/test/Concurrency/isolated_default_arguments.swift b/test/Concurrency/isolated_default_arguments.swift index 1ea6d7bb8684a..3eeb13334062f 100644 --- a/test/Concurrency/isolated_default_arguments.swift +++ b/test/Concurrency/isolated_default_arguments.swift @@ -220,10 +220,21 @@ class C3 { var y = 0 } +@MainActor class MultipleVars { + var (x, y) = (0, 0) +} + func callDefaultInit() async { _ = C2() _ = NonIsolatedInit() _ = NonIsolatedInit(x: 10) + _ = MultipleVars() +} + +// expected-warning@+1 {{default initializer for 'MultipleVarsInvalid' cannot be both nonisolated and main actor-isolated; this is an error in Swift 6}} +class MultipleVarsInvalid { + // expected-note@+1 {{initializer for property 'x' is main actor-isolated}} + @MainActor var (x, y) = (requiresMainActor(), requiresMainActor()) } @propertyWrapper From a11dc3c60edc57301be20e46eaef7d3314e4618e Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Sat, 20 Jan 2024 10:42:55 -0800 Subject: [PATCH 3/3] [Concurrency] Don't call `getAllMembers()` while inferring actor isolation for implicit initializers. This messes with conformance synthesis for `RawRepresentable` and friends because it invokes conformance synthesis too early. --- lib/Sema/CodeSynthesis.cpp | 40 ++++++++++++------- .../isolated_default_arguments.swift | 9 ++++- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index a4f519e2058b1..2fd970421cece 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -350,21 +350,31 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, // initializers apply Sendable checking to arguments at the call-site, // and actor initializers do not run on the actor, so initial values // cannot be actor-instance-isolated. - bool shouldAddNonisolated = true; ActorIsolation existingIsolation = getActorIsolation(decl); VarDecl *previousVar = nullptr; + bool hasError = false; - for (auto member : decl->getImplementationContext()->getAllMembers()) { - bool hasError = false; - auto pbd = dyn_cast(member); - if (!pbd || pbd->isStatic()) - continue; + // FIXME: Calling `getAllMembers` here causes issues for conformance + // synthesis to RawRepresentable and friends. Instead, iterate over + // both the stored properties and the init accessor properties, as + // those can participate in implicit initializers. - for (auto i : range(pbd->getNumPatternEntries())) { - if (pbd->isInitializerSubsumed(i)) + auto stored = decl->getStoredProperties(); + auto initAccessor = decl->getInitAccessorProperties(); + + auto shouldAddNonisolated = [&](ArrayRef properties) { + if (hasError) + return false; + + bool addNonisolated = true; + for (auto *var : properties) { + auto *pbd = var->getParentPatternBinding(); + if (!pbd) continue; - auto *var = pbd->getAnchoringVarDecl(i); + auto i = pbd->getPatternEntryIndexForVarDecl(var); + if (pbd->isInitializerSubsumed(i)) + continue; ActorIsolation initIsolation; if (var->hasInitAccessor()) { @@ -401,21 +411,21 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, var->getDescriptiveKind(), var->getName(), isolation); hasError = true; - break; + return false; } existingIsolation = isolation; previousVar = var; - shouldAddNonisolated = false; + addNonisolated = false; } } } - if (hasError) - break; - } + return addNonisolated; + }; - if (shouldAddNonisolated) { + if (shouldAddNonisolated(stored) && + shouldAddNonisolated(initAccessor)) { addNonIsolatedToSynthesized(decl, ctor); } } diff --git a/test/Concurrency/isolated_default_arguments.swift b/test/Concurrency/isolated_default_arguments.swift index 3eeb13334062f..d64ec3d9b836f 100644 --- a/test/Concurrency/isolated_default_arguments.swift +++ b/test/Concurrency/isolated_default_arguments.swift @@ -149,7 +149,7 @@ struct S2 { } struct S3 { - // expected-error@+1 {{default argument cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}} + // expected-error@+1 3 {{default argument cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}} var (x, y, z) = (requiresMainActor(), requiresSomeGlobalActor(), 10) } @@ -275,3 +275,10 @@ struct InitAccessors { } } } + +// Make sure isolation inference for implicit initializers +// doesn't impact conformance synthesis. + +struct CError: Error, RawRepresentable { + var rawValue: CInt +}