From a6411923d85e21a17f4391183f84b4ac9074bc0d 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. (cherry picked from commit 20829faa4325d83c424e7be2e8056953bc91f6ac) --- 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 3ce8a58e2ac77..cd072f82913d9 100644 --- a/lib/SILGen/SILGenConstructor.cpp +++ b/lib/SILGen/SILGenConstructor.cpp @@ -1544,9 +1544,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 ce558b1f65b2e..c0745de98c874 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -350,34 +350,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 (!isSendableType(decl->getModuleContext(), type) || - 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 1b406679d7876..1728b34620c76 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 f832e8a6379b1..616f6db864811 100644 --- a/test/Concurrency/global_actor_inference.swift +++ b/test/Concurrency/global_actor_inference.swift @@ -558,9 +558,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-sns-warning {{global actor 'OtherGlobalActor'-isolated default value in a main actor-isolated context; this is an error in Swift 6}} +// expected-complete-sns-warning@-1 {{memberwise initializer for 'HasWrapperOnUnsafeActor' cannot be both nonisolated and global actor 'OtherGlobalActor'-isolated; this is an error in Swift 6}} +// expected-complete-sns-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-sns-note 2 {{initializer for property '_synced' is global actor 'OtherGlobalActor'-isolated}} // expected-note @-1 3{{property declared here}} // expected-complete-sns-note @-2 3{{property declared here}} @@ -585,6 +589,10 @@ struct HasWrapperOnUnsafeActor { } } +nonisolated func createHasWrapperOnUnsafeActor() { + _ = HasWrapperOnUnsafeActor() +} + // ---------------------------------------------------------------------- // Nonisolated closures // ---------------------------------------------------------------------- @@ -669,7 +677,9 @@ func replacesDynamicOnMainActor() { // Global-actor isolation of stored property initializer expressions // ---------------------------------------------------------------------- +// expected-complete-sns-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-sns-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 ad8bcc62a9231..92361d1ca2120 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 SendNonSendable %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-error@+2 {{expression is 'async' but is not marked with 'await'}} + // 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 07d63b63b0d014df3933936469cf6bcfd64bf474 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. (cherry picked from commit cc1fd1ce8fb13518f8d7f42e37c2e995d03c3b23) --- 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 c0745de98c874..90ff061404e08 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -354,62 +354,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 (!isSendableType(decl->getModuleContext(), type) || - 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 (!isSendableType(decl->getModuleContext(), type) || + 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 92361d1ca2120..4001ec18ec97c 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 38a9faa794bbf244613573e685d03bc0e09e5ce4 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. (cherry picked from commit a11dc3c60edc57301be20e46eaef7d3314e4618e) --- 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 90ff061404e08..40e0ecb83e90c 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -349,21 +349,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()) { @@ -400,21 +410,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 4001ec18ec97c..9e80787467b21 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 +}