Skip to content

Commit e152c19

Browse files
theblixguytheblixguy
authored
[Typechecker] Allow property observers on lazy variables (#31097)
* [Typechecker] Allow property observers on lazy variables * [Typechecker] Pass 'isLazy' via argument to synthesizeObservedSetterBody * [Test] Update lazy properties test with more cases * [Test] Update lazy properties SILGen test with more cases * [Test] Update lazy properties SILGen test with struct test case
1 parent 9cdfb0e commit e152c19

File tree

4 files changed

+187
-16
lines changed

4 files changed

+187
-16
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2943,8 +2943,6 @@ ERROR(lazy_requires_single_var,none,
29432943

29442944
ERROR(lazy_must_be_property,none,
29452945
"lazy is only valid for members of a struct or class", ())
2946-
ERROR(lazy_not_observable,none,
2947-
"lazy properties must not have observers", ())
29482946
ERROR(lazy_not_strong,none,
29492947
"lazy properties cannot be %0", (ReferenceOwnership))
29502948

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,7 +1492,7 @@ static Expr *maybeWrapInOutExpr(Expr *expr, ASTContext &ctx) {
14921492
/// setter which calls them.
14931493
static std::pair<BraceStmt *, bool>
14941494
synthesizeObservedSetterBody(AccessorDecl *Set, TargetImpl target,
1495-
ASTContext &Ctx) {
1495+
ASTContext &Ctx, bool isLazy = false) {
14961496
auto VD = cast<VarDecl>(Set->getStorage());
14971497

14981498
SourceLoc Loc = VD->getLoc();
@@ -1559,10 +1559,9 @@ synthesizeObservedSetterBody(AccessorDecl *Set, TargetImpl target,
15591559
// parameter or it's provided explicitly in the parameter list.
15601560
if (!didSet->isSimpleDidSet()) {
15611561
Expr *OldValueExpr =
1562-
buildStorageReference(Set, VD, target,
1562+
buildStorageReference(Set, VD, isLazy ? TargetImpl::Ordinary : target,
15631563
/*isUsedForGetAccess=*/true,
1564-
/*isUsedForSetAccess=*/true,
1565-
Ctx);
1564+
/*isUsedForSetAccess=*/true, Ctx);
15661565

15671566
// Error recovery.
15681567
if (OldValueExpr == nullptr) {
@@ -1590,8 +1589,9 @@ synthesizeObservedSetterBody(AccessorDecl *Set, TargetImpl target,
15901589
// Create an assignment into the storage or call to superclass setter.
15911590
auto *ValueDRE = new (Ctx) DeclRefExpr(ValueDecl, DeclNameLoc(), true);
15921591
ValueDRE->setType(ValueDecl->getType());
1593-
createPropertyStoreOrCallSuperclassSetter(Set, ValueDRE, VD, target,
1594-
SetterBody, Ctx);
1592+
createPropertyStoreOrCallSuperclassSetter(
1593+
Set, ValueDRE, isLazy ? VD->getLazyStorageProperty() : VD, target,
1594+
SetterBody, Ctx);
15951595

15961596
if (auto didSet = VD->getParsedAccessor(AccessorKind::DidSet))
15971597
callObserver(didSet, OldValue);
@@ -1619,6 +1619,10 @@ synthesizeSetterBody(AccessorDecl *setter, ASTContext &ctx) {
16191619
if (auto var = dyn_cast<VarDecl>(storage)) {
16201620
if (var->getAttrs().hasAttribute<LazyAttr>()) {
16211621
// Lazy property setters write to the underlying storage.
1622+
if (var->hasObservers()) {
1623+
return synthesizeObservedSetterBody(setter, TargetImpl::Storage, ctx,
1624+
/*isLazy=*/true);
1625+
}
16221626
auto *storage = var->getLazyStorageProperty();
16231627
return synthesizeTrivialSetterBodyWithStorage(setter, TargetImpl::Storage,
16241628
storage, ctx);
@@ -2791,11 +2795,10 @@ static void finishLazyVariableImplInfo(VarDecl *var,
27912795
}
27922796

27932797
// Lazy properties must be written as stored properties in the source.
2794-
if (!info.isSimpleStored()) {
2795-
diagnoseAndRemoveAttr(var, attr,
2796-
info.hasStorage()
2797-
? diag::lazy_not_observable
2798-
: diag::lazy_not_on_computed);
2798+
if (info.getReadImpl() != ReadImplKind::Stored &&
2799+
(info.getWriteImpl() != WriteImplKind::Stored &&
2800+
info.getWriteImpl() != WriteImplKind::StoredWithObservers)) {
2801+
diagnoseAndRemoveAttr(var, attr, diag::lazy_not_on_computed);
27992802
invalid = true;
28002803
}
28012804

‎test/SILGen/lazy_property_with_observers.swift

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
// RUN: %target-swift-emit-silgen %s | %FileCheck %s
2+
3+
class Foo {
4+
lazy var bar: Int = 0 {
5+
didSet(oldValue) {}
6+
willSet {}
7+
}
8+
9+
lazy var baz: Int = 0 {
10+
didSet {}
11+
willSet {}
12+
}
13+
14+
lazy var observable1: Int = 0 {
15+
didSet(oldValue) {}
16+
}
17+
18+
lazy var observable2: Int = 0 {
19+
didSet {}
20+
}
21+
22+
lazy var observable3: Int = 0 {
23+
willSet {}
24+
}
25+
}
26+
27+
struct Foo1 {
28+
lazy var bar = 1 {
29+
didSet(oldValue) {}
30+
}
31+
}
32+
33+
var foo1 = Foo1()
34+
foo1.bar = 2
35+
36+
// Setter which calls willSet and didSet (which fetches the oldValue) //
37+
38+
// CHECK-LABEL: sil hidden [ossa] @$s28lazy_property_with_observers3FooC3barSivs : $@convention(method) (Int, @guaranteed Foo) -> () {
39+
// CHECK: bb0([[VALUE:%.*]] : $Int, [[FOO:%.*]] : @guaranteed $Foo):
40+
// CHECK-NEXT: debug_value [[VALUE]] : $Int, let, name "value", argno 1
41+
// CHECK-NEXT: debug_value [[FOO]] : $Foo, let, name "self", argno 2
42+
// CHECK-NEXT: [[GETTER:%.*]] = class_method [[FOO]] : $Foo, #Foo.bar!getter : (Foo) -> () -> Int, $@convention(method) (@guaranteed Foo) -> Int
43+
// CHECK-NEXT: [[OLDVALUE:%.*]] = apply [[GETTER]]([[FOO]]) : $@convention(method) (@guaranteed Foo) -> Int
44+
// CHECK-NEXT: debug_value [[OLDVALUE]] : $Int, let, name "tmp"
45+
// CHECK-NEXT: // function_ref Foo.bar.willset
46+
// CHECK-NEXT: [[WILLSET:%.*]] = function_ref @$s28lazy_property_with_observers3FooC3barSivw : $@convention(method) (Int, @guaranteed Foo) -> ()
47+
// CHECK-NEXT: [[WILLSET_RESULT:%.*]] = apply [[WILLSET]]([[VALUE]], [[FOO]]) : $@convention(method) (Int, @guaranteed Foo) -> ()
48+
// CHECK-NEXT: [[ENUM:%.*]] = enum $Optional<Int>, #Optional.some!enumelt, [[VALUE]] : $Int
49+
// CHECK-NEXT: [[REF_ELEM:%.*]] = ref_element_addr [[FOO]] : $Foo, #Foo.$__lazy_storage_$_bar
50+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [dynamic] [[REF_ELEM]] : $*Optional<Int>
51+
// CHECK-NEXT: assign [[ENUM]] to [[BEGIN_ACCESS]] : $*Optional<Int>
52+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Optional<Int>
53+
// CHECK-NEXT: // function_ref Foo.bar.didset
54+
// CHECK-NEXT: [[DIDSET:%.*]] = function_ref @$s28lazy_property_with_observers3FooC3barSivW : $@convention(method) (Int, @guaranteed Foo) -> ()
55+
// CHECK-NEXT: [[DIDSET_RESULT:%.*]] = apply [[DIDSET]]([[OLDVALUE]], [[FOO]]) : $@convention(method) (Int, @guaranteed Foo) -> ()
56+
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
57+
// CHECK-NEXT: return [[TUPLE]] : $()
58+
// CHECK-END: }
59+
60+
// Setter which calls willSet and simple didSet (which doesn't fetch the oldValue) //
61+
62+
// CHECK-LABEL: sil hidden [ossa] @$s28lazy_property_with_observers3FooC3bazSivs : $@convention(method) (Int, @guaranteed Foo) -> () {
63+
// CHECK: bb0([[VALUE:%.*]] : $Int, [[FOO:%.*]] : @guaranteed $Foo):
64+
// CHECK-NEXT: debug_value [[VALUE]] : $Int, let, name "value", argno 1
65+
// CHECK-NEXT: debug_value [[FOO]] : $Foo, let, name "self", argno 2
66+
// CHECK-NEXT: // function_ref Foo.baz.willset
67+
// CHECK-NEXT: [[WILLSET:%.*]] = function_ref @$s28lazy_property_with_observers3FooC3bazSivw : $@convention(method) (Int, @guaranteed Foo) -> ()
68+
// CHECK-NEXT: [[WILLSET_RESULT:%.*]] = apply [[WILLSET]]([[VALUE]], [[FOO]]) : $@convention(method) (Int, @guaranteed Foo) -> ()
69+
// CHECK-NEXT: [[ENUM:%.*]] = enum $Optional<Int>, #Optional.some!enumelt, [[VALUE]] : $Int
70+
// CHECK-NEXT: [[REF_ELEM:%.*]] = ref_element_addr [[FOO]] : $Foo, #Foo.$__lazy_storage_$_baz
71+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [dynamic] [[REF_ELEM]] : $*Optional<Int>
72+
// CHECK-NEXT: assign [[ENUM]] to [[BEGIN_ACCESS]] : $*Optional<Int>
73+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Optional<Int>
74+
// CHECK-NEXT: // function_ref Foo.baz.didset
75+
// CHECK-NEXT: [[DIDSET:%.*]] = function_ref @$s28lazy_property_with_observers3FooC3bazSivW : $@convention(method) (@guaranteed Foo) -> ()
76+
// CHECK-NEXT: [[DIDSET_RESULT:%.*]] = apply [[DIDSET]]([[FOO]]) : $@convention(method) (@guaranteed Foo) -> ()
77+
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
78+
// CHECK-NEXT: return [[TUPLE]] : $()
79+
// CHECK-END: }
80+
81+
// Setter which calls didSet (which fetches oldValue) only //
82+
83+
// CHECK-LABEL: sil hidden [ossa] @$s28lazy_property_with_observers3FooC11observable1Sivs : $@convention(method) (Int, @guaranteed Foo) -> () {
84+
// CHECK: bb0([[VALUE:%.*]] : $Int, [[FOO:%.*]] : @guaranteed $Foo):
85+
// CHECK-NEXT: debug_value [[VALUE]] : $Int, let, name "value", argno 1
86+
// CHECK-NEXT: debug_value [[FOO]] : $Foo, let, name "self", argno 2
87+
// CHECK-NEXT: [[GETTER:%.*]] = class_method [[FOO]] : $Foo, #Foo.observable1!getter : (Foo) -> () -> Int, $@convention(method) (@guaranteed Foo) -> Int
88+
// CHECK-NEXT: [[OLDVALUE:%.*]] = apply [[GETTER]]([[FOO]]) : $@convention(method) (@guaranteed Foo) -> Int
89+
// CHECK-NEXT: debug_value [[OLDVALUE]] : $Int, let, name "tmp"
90+
// CHECK-NEXT: [[ENUM:%.*]] = enum $Optional<Int>, #Optional.some!enumelt, [[VALUE]] : $Int
91+
// CHECK-NEXT: [[REF_ELEM:%.*]] = ref_element_addr [[FOO]] : $Foo, #Foo.$__lazy_storage_$_observable1
92+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [dynamic] [[REF_ELEM]] : $*Optional<Int>
93+
// CHECK-NEXT: assign [[ENUM]] to [[BEGIN_ACCESS]] : $*Optional<Int>
94+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Optional<Int>
95+
// CHECK-NEXT: // function_ref Foo.observable1.didset
96+
// CHECK-NEXT: [[DIDSET:%.*]] = function_ref @$s28lazy_property_with_observers3FooC11observable1SivW : $@convention(method) (Int, @guaranteed Foo) -> ()
97+
// CHECK-NEXT: [[DIDSET_RESULT:%.*]] = apply [[DIDSET]]([[OLDVALUE]], [[FOO]]) : $@convention(method) (Int, @guaranteed Foo) -> ()
98+
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
99+
// CHECK-NEXT: return [[TUPLE]] : $()
100+
// CHECK-END: }
101+
102+
// Setter which calls simple didSet (which doesn't fetch the oldValue) only //
103+
104+
// CHECK-LABEL: sil hidden [ossa] @$s28lazy_property_with_observers3FooC11observable2Sivs : $@convention(method) (Int, @guaranteed Foo) -> () {
105+
// CHECK: bb0([[VALUE:%.*]] : $Int, [[FOO:%.*]] : @guaranteed $Foo):
106+
// CHECK-NEXT: debug_value [[VALUE]] : $Int, let, name "value", argno 1
107+
// CHECK-NEXT: debug_value [[FOO]] : $Foo, let, name "self", argno 2
108+
// CHECK-NEXT: [[ENUM:%.*]] = enum $Optional<Int>, #Optional.some!enumelt, [[VALUE]] : $Int
109+
// CHECK-NEXT: [[REF_ELEM:%.*]] = ref_element_addr [[FOO]] : $Foo, #Foo.$__lazy_storage_$_observable2
110+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [dynamic] [[REF_ELEM]] : $*Optional<Int>
111+
// CHECK-NEXT: assign [[ENUM]] to [[BEGIN_ACCESS]] : $*Optional<Int>
112+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Optional<Int>
113+
// CHECK-NEXT: // function_ref Foo.observable2.didset
114+
// CHECK-NEXT: [[DIDSET:%.*]] = function_ref @$s28lazy_property_with_observers3FooC11observable2SivW : $@convention(method) (@guaranteed Foo) -> ()
115+
// CHECK-NEXT: [[DIDSET_RESULT:%.*]] = apply [[DIDSET]]([[FOO]]) : $@convention(method) (@guaranteed Foo) -> ()
116+
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
117+
// CHECK-NEXT: return [[TUPLE]] : $()
118+
// CHECK-END: }
119+
120+
// Setter which calls willSet only //
121+
122+
// CHECK-LABEL: sil hidden [ossa] @$s28lazy_property_with_observers3FooC11observable3Sivs : $@convention(method) (Int, @guaranteed Foo) -> () {
123+
// CHECK: bb0([[VALUE:%.*]] : $Int, [[FOO:%.*]] : @guaranteed $Foo):
124+
// CHECK-NEXT: debug_value [[VALUE]] : $Int, let, name "value", argno 1
125+
// CHECK-NEXT: debug_value [[FOO]] : $Foo, let, name "self", argno 2
126+
// CHECK-NEXT: // function_ref Foo.observable3.willset
127+
// CHECK-NEXT: [[WILLSET:%.*]] = function_ref @$s28lazy_property_with_observers3FooC11observable3Sivw : $@convention(method) (Int, @guaranteed Foo) -> ()
128+
// CHECK-NEXT: [[WILLSET_RESULT:%.*]] = apply [[WILLSET]]([[VALUE]], [[FOO]]) : $@convention(method) (Int, @guaranteed Foo) -> ()
129+
// CHECK-NEXT: [[ENUM:%.*]] = enum $Optional<Int>, #Optional.some!enumelt, [[VALUE]] : $Int
130+
// CHECK-NEXT: [[REF_ELEM:%.*]] = ref_element_addr [[FOO]] : $Foo, #Foo.$__lazy_storage_$_observable3
131+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [dynamic] [[REF_ELEM]] : $*Optional<Int>
132+
// CHECK-NEXT: assign [[ENUM]] to [[BEGIN_ACCESS]] : $*Optional<Int>
133+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Optional<Int>
134+
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
135+
// CHECK-NEXT: return [[TUPLE]] : $()
136+
// CHECK-END: }
137+
138+
// Setter which calls a didSet (which fetches the oldValue) and uses a mutating getter
139+
140+
// CHECK-LABEL: sil hidden [ossa] @$s28lazy_property_with_observers4Foo1V3barSivs : $@convention(method) (Int, @inout Foo1) -> () {
141+
// CHECK: bb0([[VALUE:%.*]] : $Int, [[FOO1:%.*]] : $*Foo1):
142+
// CHECK-NEXT: debug_value [[VALUE]] : $Int, let, name "value", argno 1
143+
// CHECK-NEXT: debug_value_addr [[FOO1]] : $*Foo1, var, name "self", argno 2
144+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [unknown] %1 : $*Foo1
145+
// CHECK-NEXT: // function_ref Foo1.bar.getter
146+
// CHECK-NEXT: [[GETTER:%.*]] = function_ref @$s28lazy_property_with_observers4Foo1V3barSivg : $@convention(method) (@inout Foo1) -> Int
147+
// CHECK-NEXT: [[OLDVALUE:%.*]] = apply [[GETTER]]([[BEGIN_ACCESS]]) : $@convention(method) (@inout Foo1) -> Int
148+
// CHECK-NEXT: debug_value [[OLDVALUE]] : $Int, let, name "tmp"
149+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Foo1
150+
// CHECK-NEXT: [[ENUM:%.*]] = enum $Optional<Int>, #Optional.some!enumelt, [[VALUE]] : $Int
151+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [unknown] [[FOO1]] : $*Foo1
152+
// CHECK-NEXT: [[REF_ELEM:%.*]] = struct_element_addr [[BEGIN_ACCESS]] : $*Foo1, #Foo1.$__lazy_storage_$_bar
153+
// CHECK-NEXT: assign [[ENUM]] to [[REF_ELEM]] : $*Optional<Int>
154+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Foo1
155+
// CHECK-NEXT: [[BEGIN_ACCESS:%.*]] = begin_access [modify] [unknown] [[FOO1]] : $*Foo1
156+
// CHECK-NEXT: // function_ref Foo1.bar.didset
157+
// CHECK-NEXT: [[DIDSET:%.*]] = function_ref @$s28lazy_property_with_observers4Foo1V3barSivW : $@convention(method) (Int, @inout Foo1) -> ()
158+
// CHECK-NEXT: [[DIDSET_RESULT:%.*]] = apply [[DIDSET]]([[OLDVALUE]], [[BEGIN_ACCESS]]) : $@convention(method) (Int, @inout Foo1) -> ()
159+
// CHECK-NEXT: end_access [[BEGIN_ACCESS]] : $*Foo1
160+
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
161+
// CHECK-NEXT: return [[TUPLE]] : $()
162+
// CHECK-END: }

‎test/decl/var/lazy_properties.swift

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,17 @@ class TestClass {
4141

4242
lazy var k : Int = { () -> Int in return 0 }()+1 // multi-stmt closure
4343

44-
lazy var l : Int = 42 { // expected-error {{lazy properties must not have observers}} {{3-8=}}
45-
didSet {
46-
}
44+
lazy var l : Int = 42 { // Okay
45+
didSet {}
46+
willSet {}
47+
}
48+
49+
lazy var m : Int = 42 { // Okay
50+
didSet {}
51+
}
52+
53+
lazy var n : Int = 42 {
54+
willSet {} // Okay
4755
}
4856

4957
init() {

0 commit comments

Comments
 (0)