From 982e14bc5248e09801f12a9077feb4f860d8989c Mon Sep 17 00:00:00 2001 From: BJ Homer Date: Thu, 9 Feb 2023 00:16:23 -0700 Subject: [PATCH 1/7] Stop inferring actor isolation from property wrappers in Swift 6. In Swift 5, isolation will continue, but will now produce a warning and a fix-it suggesting that the user add explicit isolation --- include/swift/AST/DiagnosticsSema.def | 6 ++++ lib/Sema/TypeCheckConcurrency.cpp | 43 +++++++++++++++++++++------ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 9fbbb00b54ee6..36f121bf9def5 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5122,6 +5122,12 @@ ERROR(async_unavailable_decl,none, "%0 %1 is unavailable from asynchronous contexts%select{|; %2}2", (DescriptiveDeclKind, DeclBaseName, StringRef)) +WARNING(actor_isolation_inferred_from_property_wrapper,none, + "%0 is implicitly %1 because it uses %2. This implicit isolation " + "will no longer happen in Swift 6.", + (DeclName, ActorIsolation, StringRef)) + + //------------------------------------------------------------------------------ // MARK: String Processing //------------------------------------------------------------------------------ diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 014facd557ae4..6ee42cd64b212 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -3449,8 +3449,16 @@ static Optional getIsolationFromWrappers( if (!nominal->getParentSourceFile()) return None; - - Optional foundIsolation; + + ASTContext &ctx = nominal->getASTContext(); + if (ctx.isSwiftVersionAtLeast(6)) { + // In Swift 6, we no longer infer isolation of a nominal type based + // on property wrappers used in its stored properties + return None; + } + + Optional> foundIsolationAndType; + for (auto member : nominal->getMembers()) { auto var = dyn_cast(member); if (!var || !var->isInstanceMember()) @@ -3475,19 +3483,36 @@ static Optional getIsolationFromWrappers( case ActorIsolation::GlobalActor: case ActorIsolation::GlobalActorUnsafe: - if (!foundIsolation) { - foundIsolation = isolation; - continue; + if (!foundIsolationAndType) { + if (auto propertyWrapperType = var->getAttachedPropertyWrapperType(0)) { + foundIsolationAndType = { isolation, propertyWrapperType }; + continue; + } } - if (*foundIsolation != isolation) + if (foundIsolationAndType->first != isolation) return None; break; } } - return foundIsolation; + if (foundIsolationAndType) { + // We are inferring isolation for the type because + // it contains an actor-isolated property wrapper. + // Warn that this inferrence will be going away in + // Swift 6 + const ActorIsolation isolation = foundIsolationAndType->first; + const Type type = foundIsolationAndType->second; + + nominal->diagnose(diag::actor_isolation_inferred_from_property_wrapper, + nominal->getName(), isolation, "'@"+type->getString()+"'") + .fixItInsert(nominal->getAttributeInsertionLoc(false), "@" + isolation.getGlobalActor().getString()); + return isolation; + } + else { + return None; + } } namespace { @@ -4063,8 +4088,8 @@ ActorIsolation ActorIsolationRequest::evaluate( if (auto inferred = inferredIsolation(*conformanceIsolation)) return inferred; - // If the declaration is a nominal type and any property wrappers on - // its stored properties require isolation, use that. + // Before Swift 6: If the declaration is a nominal type and any property + // wrappers on its stored properties require isolation, use that. if (auto wrapperIsolation = getIsolationFromWrappers(nominal)) { if (auto inferred = inferredIsolation(*wrapperIsolation)) return inferred; From a1ea82260c5980d1ca7c3929a27a0e5278bee2b0 Mon Sep 17 00:00:00 2001 From: BJ Homer Date: Thu, 9 Feb 2023 00:46:02 -0700 Subject: [PATCH 2/7] Don't actually warn about about the property wrapper isolation in Swift 5. In many cases, the inferrence was unexpected and undesired. We shouldn't warn the user about something that is reasonably likely to be correct. --- include/swift/AST/DiagnosticsSema.def | 6 ----- lib/Sema/TypeCheckConcurrency.cpp | 34 +++++++-------------------- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 36f121bf9def5..9fbbb00b54ee6 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5122,12 +5122,6 @@ ERROR(async_unavailable_decl,none, "%0 %1 is unavailable from asynchronous contexts%select{|; %2}2", (DescriptiveDeclKind, DeclBaseName, StringRef)) -WARNING(actor_isolation_inferred_from_property_wrapper,none, - "%0 is implicitly %1 because it uses %2. This implicit isolation " - "will no longer happen in Swift 6.", - (DeclName, ActorIsolation, StringRef)) - - //------------------------------------------------------------------------------ // MARK: String Processing //------------------------------------------------------------------------------ diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 6ee42cd64b212..676ec5fb7deba 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -3452,13 +3452,12 @@ static Optional getIsolationFromWrappers( ASTContext &ctx = nominal->getASTContext(); if (ctx.isSwiftVersionAtLeast(6)) { - // In Swift 6, we no longer infer isolation of a nominal type based - // on property wrappers used in its stored properties + // In Swift 6, we no longer infer isolation of a nominal type + // based on the property wrappers used in its stored properties return None; } - Optional> foundIsolationAndType; - + Optional foundIsolation; for (auto member : nominal->getMembers()) { auto var = dyn_cast(member); if (!var || !var->isInstanceMember()) @@ -3483,36 +3482,19 @@ static Optional getIsolationFromWrappers( case ActorIsolation::GlobalActor: case ActorIsolation::GlobalActorUnsafe: - if (!foundIsolationAndType) { - if (auto propertyWrapperType = var->getAttachedPropertyWrapperType(0)) { - foundIsolationAndType = { isolation, propertyWrapperType }; - continue; - } + if (!foundIsolation) { + foundIsolation = isolation; + continue; } - if (foundIsolationAndType->first != isolation) + if (*foundIsolation != isolation) return None; break; } } - if (foundIsolationAndType) { - // We are inferring isolation for the type because - // it contains an actor-isolated property wrapper. - // Warn that this inferrence will be going away in - // Swift 6 - const ActorIsolation isolation = foundIsolationAndType->first; - const Type type = foundIsolationAndType->second; - - nominal->diagnose(diag::actor_isolation_inferred_from_property_wrapper, - nominal->getName(), isolation, "'@"+type->getString()+"'") - .fixItInsert(nominal->getAttributeInsertionLoc(false), "@" + isolation.getGlobalActor().getString()); - return isolation; - } - else { - return None; - } + return foundIsolation; } namespace { From bf472c295455644f27fcd7f9ae08e3aad6a5d93c Mon Sep 17 00:00:00 2001 From: BJ Homer Date: Tue, 21 Feb 2023 23:44:14 -0700 Subject: [PATCH 3/7] Adjust tests new actor isolation rules in Swift 6. `HasMainActorWrappedProp` was testing the inference rule that a type that uses actor-isolated property wrappers its itself isolated to that same actor. In Swift 6, that inference has been removed, so this test now verifies that the type is _not_ actor isolated. --- test/Concurrency/global_actor_inference_swift6.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Concurrency/global_actor_inference_swift6.swift b/test/Concurrency/global_actor_inference_swift6.swift index 6e2624dd26cc2..f195912bd5642 100644 --- a/test/Concurrency/global_actor_inference_swift6.swift +++ b/test/Concurrency/global_actor_inference_swift6.swift @@ -78,7 +78,7 @@ struct HasMainActorWrappedProp { var plainStorage: Int - var computedProp: Int { 0 } // expected-note {{property declared here}} + var computedProp: Int { 0 } nonisolated func testErrors() { _ = thing // expected-error {{main actor-isolated property 'thing' can not be referenced from a non-isolated context}} @@ -89,7 +89,7 @@ struct HasMainActorWrappedProp { _ = plainStorage - _ = computedProp // expected-error {{main actor-isolated property 'computedProp' can not be referenced from a non-isolated context}} + _ = computedProp } } From f876f26aafe6061cac7e5942b6a58663ab3c8432 Mon Sep 17 00:00:00 2001 From: BJ Homer Date: Wed, 22 Feb 2023 08:45:44 -0700 Subject: [PATCH 4/7] Add a feature flag for DisableActorInferenceFromPropertyWrapperUsage. This will be enabled by default in Swift 6, but can now also be enabled by passing `-enable-upcoming-feature DisableActorInferenceFromPropertyWrapperUsage` to the compiler. --- include/swift/Basic/Features.def | 1 + lib/AST/ASTPrinter.cpp | 4 ++++ lib/Sema/TypeCheckConcurrency.cpp | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def index 612875e16838a..7cc3f4b0d49f7 100644 --- a/include/swift/Basic/Features.def +++ b/include/swift/Basic/Features.def @@ -97,6 +97,7 @@ UPCOMING_FEATURE(ConciseMagicFile, 274, 6) UPCOMING_FEATURE(ForwardTrailingClosures, 286, 6) UPCOMING_FEATURE(BareSlashRegexLiterals, 354, 6) UPCOMING_FEATURE(ExistentialAny, 335, 6) +UPCOMING_FEATURE(DisableActorInferenceFromPropertyWrapperUsage, 0, 6) EXPERIMENTAL_FEATURE(StaticAssert, false) EXPERIMENTAL_FEATURE(VariadicGenerics, false) diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index e849c12fae903..64922ec95b736 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3170,6 +3170,10 @@ static bool usesFeatureBuiltinMacros(Decl *decl) { return false; } +static bool usesFeatureDisableActorInferenceFromPropertyWrapperUsage(Decl *decl) { + return false; +} + static void suppressingFeatureNoAsyncAvailability(PrintOptions &options, llvm::function_ref action) { diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 676ec5fb7deba..fb6faf6fd6187 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -3451,7 +3451,7 @@ static Optional getIsolationFromWrappers( return None; ASTContext &ctx = nominal->getASTContext(); - if (ctx.isSwiftVersionAtLeast(6)) { + if (ctx.LangOpts.hasFeature(Feature::DisableActorInferenceFromPropertyWrapperUsage)) { // In Swift 6, we no longer infer isolation of a nominal type // based on the property wrappers used in its stored properties return None; From 4024b6eee2b0456d0fecb6fbe87c4fbd7cc0aca7 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 27 Jun 2023 15:16:22 -0700 Subject: [PATCH 5/7] Add SE number to upcoming feature for disabling actor inference from property wrappers --- include/swift/Basic/Features.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def index 91dcac1954a73..6374dea921d46 100644 --- a/include/swift/Basic/Features.def +++ b/include/swift/Basic/Features.def @@ -114,7 +114,7 @@ UPCOMING_FEATURE(ForwardTrailingClosures, 286, 6) UPCOMING_FEATURE(BareSlashRegexLiterals, 354, 6) UPCOMING_FEATURE(ExistentialAny, 335, 6) UPCOMING_FEATURE(ImportObjcForwardDeclarations, 384, 6) -UPCOMING_FEATURE(DisableActorInferenceFromPropertyWrapperUsage, 0, 6) +UPCOMING_FEATURE(DisableActorInferenceFromPropertyWrapperUsage, 401, 6) EXPERIMENTAL_FEATURE(StaticAssert, false) EXPERIMENTAL_FEATURE(NamedOpaqueTypes, false) From 312bc78a57e56b61c78731f2b93d3c80eabc98b2 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 27 Jun 2023 15:53:08 -0700 Subject: [PATCH 6/7] llvm::None fix --- lib/Sema/TypeCheckConcurrency.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 21d0b5139e59b..3fb358c73415c 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -3651,7 +3651,7 @@ getIsolationFromWrappers(NominalTypeDecl *nominal) { return llvm::None; if (!nominal->getParentSourceFile()) - return None; + return llvm::None; ASTContext &ctx = nominal->getASTContext(); if (ctx.LangOpts.hasFeature(Feature::DisableActorInferenceFromPropertyWrapperUsage)) { From 03ae003be5e6d4dc36c61057a1294cc2b27e3b76 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 27 Jun 2023 15:53:17 -0700 Subject: [PATCH 7/7] llvm::None fix --- lib/Sema/TypeCheckConcurrency.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 3fb358c73415c..58d2a4b6ad9a7 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -3657,7 +3657,7 @@ getIsolationFromWrappers(NominalTypeDecl *nominal) { if (ctx.LangOpts.hasFeature(Feature::DisableActorInferenceFromPropertyWrapperUsage)) { // In Swift 6, we no longer infer isolation of a nominal type // based on the property wrappers used in its stored properties - return None; + return llvm::None; } llvm::Optional foundIsolation;