From 267733aaf464d872c8842a1c9b4d89c2bedd42f7 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Mon, 24 May 2021 19:03:58 -0700 Subject: [PATCH 1/3] [Property Wrappers] Check property wrapper actor isolation and initializer effects in PropertyWrapperInitializerInfoRequest. Now that property wrapper initializer synthesis and checking is separated from creating the auxiliary variables, checking actor isolation from PropertyWrapperInitializerInfoRequest will not create a circular dependency. This also fixes a local property wrapper crash due to effects checking never being done on synthesized default wrapper initializers. --- lib/Sema/TypeCheckDeclPrimary.cpp | 27 ------------------------ lib/Sema/TypeCheckStorage.cpp | 5 +++++ test/SILGen/property_wrapper_local.swift | 27 ++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index d46556c02a64b..3f1d40d60f7c2 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -1792,31 +1792,6 @@ class DeclChecker : public DeclVisitor { }); } - /// If the given pattern binding has a property wrapper, check the - /// isolation and effects of the backing storage initializer. - void checkPropertyWrapperBackingInitializer(PatternBindingDecl *PBD) { - auto singleVar = PBD->getSingleVar(); - if (!singleVar) - return; - - if (!singleVar->hasAttachedPropertyWrapper()) - return; - - auto *backingVar = singleVar->getPropertyWrapperBackingProperty(); - if (!backingVar) - return; - - auto backingPBD = backingVar->getParentPatternBinding(); - if (!backingPBD) - return; - - auto initInfo = singleVar->getPropertyWrapperInitializerInfo(); - if (auto initializer = initInfo.getInitFromWrappedValue()) { - checkPropertyWrapperActorIsolation(backingPBD, initializer); - TypeChecker::checkPropertyWrapperEffects(backingPBD, initializer); - } - } - void visitPatternBindingDecl(PatternBindingDecl *PBD) { DeclContext *DC = PBD->getDeclContext(); @@ -1963,8 +1938,6 @@ class DeclChecker : public DeclVisitor { } } } - - checkPropertyWrapperBackingInitializer(PBD); } void visitSubscriptDecl(SubscriptDecl *SD) { diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index e6690f9eaeea2..22edcce35d0c2 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -2584,6 +2584,11 @@ static void typeCheckSynthesizedWrapperInitializer(VarDecl *wrappedVar, dyn_cast_or_null(parentPBD->getInitContext(i))) { TypeChecker::contextualizeInitializer(initializerContext, initializer); } + + auto *backingVar = wrappedVar->getPropertyWrapperBackingProperty(); + auto *backingPBD = backingVar->getParentPatternBinding(); + checkPropertyWrapperActorIsolation(backingPBD, initializer); + TypeChecker::checkPropertyWrapperEffects(backingPBD, initializer); } static PropertyWrapperMutability::Value diff --git a/test/SILGen/property_wrapper_local.swift b/test/SILGen/property_wrapper_local.swift index b6bccce9b76e9..12d2bfd4e5f0c 100644 --- a/test/SILGen/property_wrapper_local.swift +++ b/test/SILGen/property_wrapper_local.swift @@ -190,3 +190,30 @@ func testCaptures() { // closure #1 in testCaptures() // CHECK-LABEL: sil private [ossa] @$s22property_wrapper_local12testCapturesyyFyycfU_ : $@convention(thin) (@guaranteed { var Wrapper }) -> () } + +@propertyWrapper +struct DefaultInit { + var wrappedValue: Int + + // CHECK-LABEL: sil hidden [ossa] @$s22property_wrapper_local11DefaultInitVACycfC : $@convention(method) (@thin DefaultInit.Type) -> DefaultInit + init() { + self.wrappedValue = 0 + } + + // CHECK-LABEL: sil hidden [ossa] @$s22property_wrapper_local11DefaultInitV5valueACSi_tcfC : $@convention(method) (Int, @thin DefaultInit.Type) -> DefaultInit + init(value: Int) { + self.wrappedValue = value + } +} + +// CHECK-LABEL: sil hidden [ossa] @$s22property_wrapper_local20testLocalDefaultInityyF : $@convention(thin) () -> () +func testLocalDefaultInit() { + // CHECK: function_ref @$s22property_wrapper_local11DefaultInitVACycfC : $@convention(method) (@thin DefaultInit.Type) -> DefaultInit + @DefaultInit var x: Int + + // CHECK: function_ref @$s22property_wrapper_local11DefaultInitV5valueACSi_tcfC : $@convention(method) (Int, @thin DefaultInit.Type) -> DefaultInit + @DefaultInit(value: 10) var z: Int + + // CHECK: function_ref @$s22property_wrapper_local11DefaultInitVACycfC : $@convention(method) (@thin DefaultInit.Type) -> DefaultInit + @DefaultInit() var y: Int +} From 10684948542bd5cfd1cbac8aa37d1506604d2288 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Mon, 24 May 2021 19:39:14 -0700 Subject: [PATCH 2/3] [NFC][Test] Add tests for actor isolation checking for local property wrapper default initializers. --- test/Concurrency/global_actor_inference.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift index e2090445b7bc1..49552c1116867 100644 --- a/test/Concurrency/global_actor_inference.swift +++ b/test/Concurrency/global_actor_inference.swift @@ -361,6 +361,13 @@ actor WrapperActorBad2 { } } +@propertyWrapper +struct WrapperWithMainActorDefaultInit { + var wrappedValue: Int { fatalError() } + + @MainActor init() {} // expected-note {{calls to initializer 'init()' from outside of its actor context are implicitly asynchronous}} +} + actor ActorWithWrapper { @WrapperOnActor var synced: Int = 0 // expected-note@-1 3{{property declared here}} @@ -368,6 +375,8 @@ actor ActorWithWrapper { _ = synced // expected-error{{'synced' isolated to global actor}} _ = $synced // expected-error{{'$synced' isolated to global actor}} _ = _synced // expected-error{{'_synced' isolated to global actor}} + + @WrapperWithMainActorDefaultInit var value: Int // expected-error {{call to main actor-isolated initializer 'init()' in a synchronous actor-isolated context}} } } From aa8ad8e06b2fc7d159447abdd90fc79f61e39e13 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Mon, 24 May 2021 20:31:56 -0700 Subject: [PATCH 3/3] [Property Wrappers] Avoid creating an unnecessary property wrapper generator function for default-initialized local and static wrapped properties. --- lib/Sema/TypeCheckStorage.cpp | 12 +++++++++--- test/SILGen/property_wrapper_local.swift | 9 +++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 22edcce35d0c2..3e5a1d3511a8f 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -2876,10 +2876,16 @@ PropertyWrapperInitializerInfoRequest::evaluate(Evaluator &evaluator, if (!parentPBD->isInitialized(patternNumber) && wrapperInfo.defaultInit) { // FIXME: Record this expression somewhere so that DI can perform the // initialization itself. - Expr *initializer = nullptr; - typeCheckSynthesizedWrapperInitializer(var, initializer); - pbd->setInit(0, initializer); + Expr *defaultInit = nullptr; + typeCheckSynthesizedWrapperInitializer(var, defaultInit); + pbd->setInit(0, defaultInit); pbd->setInitializerChecked(0); + + // If a static, global, or local wrapped property has a default + // initializer, this is the only initializer that will be used. + if (var->isStatic() || !dc->isTypeContext()) { + initializer = defaultInit; + } } else if (var->hasObservers() && !dc->isTypeContext()) { var->diagnose(diag::observingprop_requires_initializer); } diff --git a/test/SILGen/property_wrapper_local.swift b/test/SILGen/property_wrapper_local.swift index 12d2bfd4e5f0c..e806ef50ec79d 100644 --- a/test/SILGen/property_wrapper_local.swift +++ b/test/SILGen/property_wrapper_local.swift @@ -206,6 +206,12 @@ struct DefaultInit { } } +@propertyWrapper +struct DefaultWrappedValue { + // CHECK-LABEL: sil hidden [ossa] @$s22property_wrapper_local19DefaultWrappedValueVACycfC : $@convention(method) (@thin DefaultWrappedValue.Type) -> DefaultWrappedValue + var wrappedValue: Int = 10 +} + // CHECK-LABEL: sil hidden [ossa] @$s22property_wrapper_local20testLocalDefaultInityyF : $@convention(thin) () -> () func testLocalDefaultInit() { // CHECK: function_ref @$s22property_wrapper_local11DefaultInitVACycfC : $@convention(method) (@thin DefaultInit.Type) -> DefaultInit @@ -216,4 +222,7 @@ func testLocalDefaultInit() { // CHECK: function_ref @$s22property_wrapper_local11DefaultInitVACycfC : $@convention(method) (@thin DefaultInit.Type) -> DefaultInit @DefaultInit() var y: Int + + // CHECK: function_ref @$s22property_wrapper_local19DefaultWrappedValueVACycfC : $@convention(method) (@thin DefaultWrappedValue.Type) -> DefaultWrappedValue + @DefaultWrappedValue var w: Int }