Skip to content

Commit e9912d2

Browse files
theblixguyairspeedswift
authored andcommitted
Add the didSet Semantics proposal (#1068)
* Squash changes into single commit * Kick off review
1 parent dfaee0e commit e9912d2

File tree

1 file changed

+168
-0
lines changed

1 file changed

+168
-0
lines changed

proposals/0268-didset-semantics.md

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
# didSet Semantics
2+
3+
* Proposal: [SE-0268](0268-didset-semantics.md)
4+
* Author: [Suyash Srijan](https://www.github.com/theblixguy)
5+
* Review Manager: [Ben Cohen](https://www.github.com/airspeedswift)
6+
* Status: **Active Review (21-31 October 2019)**
7+
* Implementation: [apple/swift#26632](https://github.com/apple/swift/pull/26632)
8+
9+
## Introduction
10+
11+
Introduce two changes to `didSet` semantics -
12+
13+
1. If a `didSet` observer does not reference the `oldValue` in its body, then the call to fetch the `oldValue` will be skipped. We refer to this as a "simple" didSet.
14+
2. If we have a "simple" `didSet` and no `willSet`, then we could allow modifications to happen in-place.
15+
16+
Swift-evolution thread: [didSet Semantics](https://forums.swift.org/t/pitch-didset-semantics/27858)
17+
18+
## Motivation
19+
20+
Currently, Swift always calls the property's getter to get the `oldValue` if we have a `didSet` observer, even if the observer does not refer to the `oldValue` in its body. For example:
21+
22+
```swift
23+
class Foo {
24+
var bar: Int {
25+
didSet { print("didSet called") }
26+
}
27+
28+
init(bar: Int) { self.bar = bar }
29+
}
30+
31+
let foo = Foo(bar: 0)
32+
// This calls the getter on 'bar' to get
33+
// the 'oldValue', even though we never
34+
// refer to the oldValue inside bar's 'didSet'
35+
foo.bar = 1
36+
```
37+
38+
This might look harmless, but it is doing redundant work (by allocating storage and loading a value which isn't used). It could also be expensive if the getter performs some non-trivial task and/or returns a large value.
39+
40+
For example:
41+
42+
```swift
43+
struct Container {
44+
var items: [Int] = .init(repeating: 1, count: 100) {
45+
didSet {
46+
// Do some stuff, but don't access oldValue
47+
}
48+
}
49+
50+
mutating func update() {
51+
for index in 0..<items.count {
52+
items[index] = index + 1
53+
}
54+
}
55+
}
56+
57+
var container = Container()
58+
container.update()
59+
```
60+
61+
This will create 100 copies of the array to provide the `oldValue`, even though they're not used at all.
62+
63+
It also prevents us from writing certain features. For example, a `@Delayed` property wrapper may be implemented like this:
64+
65+
```swift
66+
@propertyWrapper
67+
struct Delayed<Value> {
68+
var wrappedValue: Value {
69+
get {
70+
guard let value = value else {
71+
preconditionFailure("Property \(String(describing: self)) has not been set yet")
72+
}
73+
return value
74+
}
75+
76+
set {
77+
guard value == nil else {
78+
preconditionFailure("Property \(String(describing: self)) has already been set")
79+
}
80+
value = newValue
81+
}
82+
}
83+
84+
private var value: Value?
85+
}
86+
87+
class Foo {
88+
@Delayed var bar: Int {
89+
didSet { print("didSet called") }
90+
}
91+
}
92+
93+
let foo = Foo()
94+
foo.bar = 1
95+
```
96+
97+
However, this code will currently crash when we set `bar`'s value to be `1`. This is because Swift will fetch the `oldValue`, which is `nil` initially and thus will trigger the precondition in the getter.
98+
99+
## Proposed Solution
100+
101+
The property's getter is no longer called if we do not refer to the `oldValue` inside the body of the `didSet`.
102+
103+
```swift
104+
class Foo {
105+
var bar = 0 {
106+
didSet { print("didSet called") }
107+
}
108+
109+
var baz = 0 {
110+
didSet { print(oldValue) }
111+
}
112+
}
113+
114+
let foo = Foo()
115+
// This will not call the getter to fetch the oldValue
116+
foo.bar = 1
117+
// This will call the getter to fetch the oldValue
118+
foo.baz = 2
119+
```
120+
121+
This also resolves some pending bugs such as [SR-11297](https://bugs.swift.org/browse/SR-11297), [SR-11280](https://bugs.swift.org/browse/SR-11280) and [SR-5982](https://bugs.swift.org/browse/SR-5982).
122+
123+
As a bonus, if the property has a "simple" `didSet` and no `willSet`, then we could allow for modifications to happen in-place. For example:
124+
125+
```swift
126+
// This is how we currently synthesize the _modify coroutine
127+
_modify {
128+
var newValue = underlyingStorage
129+
yield &newValue
130+
// Call the setter, which then calls
131+
// willSet (if present) and didSet
132+
observedStorage = newValue
133+
}
134+
135+
// This is how we're going to synthesize it instead
136+
_modify {
137+
// Since we don't have a willSet and
138+
// we have a "simple" didSet, we can
139+
// yield the storage directly and
140+
// call didSet
141+
yield &underlyingStorage
142+
didSet()
143+
}
144+
```
145+
146+
This will provide a nice performance boost in some cases (for example, in the earlier array copying example).
147+
148+
## Source compatibility
149+
150+
This does not break source compatibility, _unless_ someone is explicitly relying on the current buggy behavior (i.e. the property's getter being called even if the `oldValue` isn't referenced). However, I think the possibility of that is very small.
151+
152+
## Effect on ABI stability
153+
154+
This does not affect the ABI as observers are not a part of it.
155+
156+
## Effect on API resilience
157+
158+
This does not affect API resilience - library authors can freely switch between a `didSet` which does not refer to the `oldValue` in its body and one which does and freely add or remove `didSet` from the property.
159+
160+
## Alternatives considered
161+
162+
Leave the existing behavior as is.
163+
164+
## Future Directions
165+
166+
We can apply the same treatment to `willSet` i.e. not pass the `newValue` if it does not refer to it in its body, although it wouldn't provide any real benefit as not passing `newValue` to `willSet` does not avoid anything, where as not passing `oldValue` to `didSet` avoids loading it.
167+
168+
We can also deprecate the implicit `oldValue` and request users to explicitly provide `oldValue` in parenthesis (`didSet(oldValue) { ... }`) if they want to use it in the body of the observer. This will make the new behavior more obvious and self-documenting.

0 commit comments

Comments
 (0)