# SE-0268 (Review #2) — Review didSet Semantics

**URL:** <https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396>\
**Category:** Proposal Reviews\
**Created:** [December 6, 2019, 12:15am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396 "2019-12-06T00:15:49Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![ben-cohen](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/ben-cohen/32/296_2.png) [@ben-cohen](https://forums.swift.org/u/ben-cohen)\
**Post date:** [December 6, 2019, 12:15am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/1 "2019-12-06T00:15:49Z")

</div>

The second review of [SE-0268 — Refine `didSet` Semantics](https://github.com/apple/swift-evolution/blob/master/proposals/0268-didset-semantics.md) begins now and runs through December 9th, 2019.

This is a follow-up to the [previous review discussion](https://forums.swift.org/t/se-0268-refine-didset-semantics/30049). The review was discussed with the core team, and the team believes the proposal should be accepted with a specific amendment to address concerns in the prior review: that in cases where an implicit `oldValue` parameter is used, causing the value to be fetched, a warning should be generated asking the user to add the parameter explicitly.

You can find a diff from the prior proposal [here](https://github.com/apple/swift-evolution/commit/8f49dfe1a7590b55be8d4d1e2061cbb753ea04a4).

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via [email](mailto:ben_cohen@apple.com) or direct message in the Swift forums).

## What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

- What is your evaluation of the proposal?

- Is the problem being addressed significant enough to warrant a change to Swift?

- Does this proposal fit well with the feel and direction of Swift?

- If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

- How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,  
Ben Cohen  
Review Manager

---

<div class="post-metadata">

**Author:** ![anandabits](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/anandabits/32/355_2.png) [@anandabits](https://forums.swift.org/u/anandabits)\
**Post date:** [December 6, 2019, 12:24am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/2 "2019-12-06T00:24:48Z")

</div>

I agree with the core team’s decision. Looks like a good change.

---

<div class="post-metadata">

**Author:** ![ben-cohen](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/ben-cohen/32/296_2.png) [@ben-cohen](https://forums.swift.org/u/ben-cohen)\
**Post date:** [December 6, 2019, 12:27am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/3 "2019-12-06T00:27:01Z")

</div>

Note this warning can be silenced in a way that is compatible with older compilers (important for people who want to build without warnings with both compilers during a transition between the two).

---

<div class="post-metadata">

**Author:** ![Karl](https://avatars.discourse-cdn.com/v4/letter/k/34f0e0/32.png) [@Karl](https://forums.swift.org/u/Karl)\
**Post date:** [December 6, 2019, 12:29am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/4 "2019-12-06T00:29:49Z")

</div>

> [@ben-cohen](#):
>
> begins now and runs through October 9th, 2019

Wow, when you said "short re-review", you weren't kidding! That's only -58 days from now!

---

<div class="post-metadata">

**Author:** ![ben-cohen](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/ben-cohen/32/296_2.png) [@ben-cohen](https://forums.swift.org/u/ben-cohen)\
**Post date:** [December 6, 2019, 12:40am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/5 "2019-12-06T00:40:40Z")

</div>

🤦🏻‍♂️ thanks, fixed

---

<div class="post-metadata">

**Author:** ![beccadax](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/beccadax/32/34217_2.png) [@beccadax](https://forums.swift.org/u/beccadax)\
**Post date:** [December 6, 2019, 1:34am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/6 "2019-12-06T01:34:01Z")

</div>

Implicit use of `oldValue` appears to be _extremely_ common in the wild. I may have messed up the analysis somehow, but I searched the source compatibility suite, and it appears to contain:

- 1,051 instances of `didSet`
- 186 uses of `oldValue` (this will include other variables that coincidentally have that name)
- _Literally zero_ uses of `didSet` with an explicit name

Are we prepared to warn on nearly every use of `oldValue` in existing Swift code?

My personal opinion is that this warning will be unnecessarily disruptive to common usage and the proposal is just fine without the warning.

---

<div class="post-metadata">

**Author:** ![Nevin](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/nevin/32/617_2.png) [@Nevin](https://forums.swift.org/u/Nevin)\
**Post date:** [December 6, 2019, 1:55am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/7 "2019-12-06T01:55:41Z")

</div>

I am opposed to removing (or deprecating) the implicit `oldValue`. This feature, alongside the implicit `newValue` in setters, is one of the many nice things that make Swift pleasant and convenient to use.

I would view it as a net negative to have to write “`didSet(oldValue) { … }`”, just as I would view it as a negative to have to write “`set(newValue) { … }`”. It is by far preferable to write “`set { … }`” and “`didSet { … }`”.

Notably, if it were required to manually specify the name, then I would probably call it “`x`” most of the time, and I expect many others would choose similarly short names. These short names would reduce (but not eliminate) the inconvenience of having to provide a name manually, but at the cost of clarity and consistency.

I do not think that simplifying the compiler implementation is worth the certain reduction in convenience, and the probable reduction in clarity and consistency of code, that this would bring.

---

<div class="post-metadata">

**Author:** ![jarod](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/jarod/32/364_2.png) [@jarod](https://forums.swift.org/u/jarod)\
**Post date:** [December 6, 2019, 1:57am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/8 "2019-12-06T01:57:57Z")

</div>

I have to agree with Brent -- I have 165 uses of `oldValue` in my project, and I don't look forward to having to add explicit declarations for them. Xcode will make the process easier, but I don't really want the declarations there in the first place. I find it intuitive that if you use `oldValue` you will access the property, and I don't really see how adding the declaration serves to clarify that.

I still think this is a great proposal even with the change, but I definitely prefer the previous revision.

---

<div class="post-metadata">

**Author:** ![michelf](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/michelf/32/1180_2.png) [@michelf](https://forums.swift.org/u/michelf)\
**Post date:** [December 6, 2019, 2:55am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/9 "2019-12-06T02:55:03Z")

</div>

I don't think deprecating implicit `oldValue` is a good move.

For one thing, it's very disruptive because it forces the declaration of the parameter where almost everyone was relying on the implicit name.

For another, it somewhat forces you to keep in sync the `didSet` declaration with its body if you want to keep things efficient. If you add `oldValue` in the body you need to add it in the declaration; if you remove it from the body you should remove it from the declaration too (unless you're relying on the semantics of calling the getter, which pretty much no one cares about). It's not like you're expressing a commitment in the API with this parameter, so why not let the compiler maintain this boilerplate for you?

It does not help with preserving source compatibility since cases where implicit `oldValue` is unused but you were relying on the getter being called are not going to get any warning. Maybe the idea is for the warning elsewhere to teach people `oldValue` has the side effect of calling the getter so they think to add it at places they need the getter to be called, but is the lesson worth all this disruption?

And it's only a warning: the compiler still has to support implicit `oldValue` and is not saving any CPU cycle.

In the end, I don't see any reason why the implicit declaration for `oldValue` should be deprecated.

---

<div class="post-metadata">

**Author:** ![John\_McCall](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/john_mccall/32/1351_2.png) [@John\_McCall](https://forums.swift.org/u/John_McCall)\
**Post date:** [December 6, 2019, 2:55am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/10 "2019-12-06T02:55:49Z")

</div>

> [@Nevin](#):
>
> I do not think that simplifying the compiler implementation is worth the certain reduction in convenience, and the probable reduction in clarity and consistency of code, that this would bring.

To be clear, simplifying the compiler implementation is not the rationale for deprecating `oldValue`.

---

<div class="post-metadata">

**Author:** ![Nevin](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/nevin/32/617_2.png) [@Nevin](https://forums.swift.org/u/Nevin)\
**Post date:** [December 6, 2019, 3:06am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/11 "2019-12-06T03:06:48Z")

</div>

> [@John\_McCall](#):
>
> To be clear, simplifying the compiler implementation is not the rationale for deprecating `oldValue` .

Oh.

In that case, if the implementation complexity is not of concern here, then I feel _strongly_ that `oldValue` should not be deprecated.

---

<div class="post-metadata">

**Author:** ![xwu](https://avatars.discourse-cdn.com/v4/letter/x/b487fb/32.png) [@xwu](https://forums.swift.org/u/xwu)\
**Post date:** [December 6, 2019, 4:49am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/12 "2019-12-06T04:49:16Z")

</div>

> [@michelf](#):
>
> If you add `oldValue` in the body you need to add it in the declaration; if you remove it from the body you should remove it from the declaration too (unless you're relying on the semantics of calling the getter, which pretty much no one cares about).

The issue about calling the getter is the entire rationale for this proposal; if the argument is that pretty much no one cares about it, then no part of this proposal should be adopted. I think the premise here is that people _do_ care about it, and that requiring people who use the old value in the body to update the declaration reflects that.

> [@Nevin](#):
>
> Notably, if it were required to manually specify the name, then I would probably call it “ `x` ” most of the time, and I expect many others would choose similarly short names. These short names would reduce (but not eliminate) the inconvenience of having to provide a name manually, but at the cost of clarity and consistency.

I don’t buy this argument. Instead of requiring you to write any particular variable name _n_ times, this proposal would require you to write the name _n + 1_ times. But the additional one time that you would need to write the name is _so inconvenient_ that you would choose a shorter name, even though you simultaneously _strongly_ favor the clarity of a longer name?

I would argue, on the contrary, that both convenience and clarity argue in _favor_ of the new proposal: writing `x` twice is still fewer keypresses than writing `oldValue` even once, and explicitly declaring a variable name (even if it is one letter long) is surely more clear than using a magical name declared nowhere (even if perfectly named).

To my mind there is one major reason to argue against deprecating implicit `oldValue`—which is that it is a source-breaking deprecation. That it can be automatically migrated in all cases, and that the new syntax to which it is migrated is backwards compatible in all cases, are major mitigating factors. But nonetheless the degree to which it is source-breaking still weighs on the proposal. That much, I think, is unarguable.

---

<div class="post-metadata">

**Author:** ![michelf](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/michelf/32/1180_2.png) [@michelf](https://forums.swift.org/u/michelf)\
**Post date:** [December 6, 2019, 5:01am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/13 "2019-12-06T05:01:32Z")

</div>

> [@xwu](#):
>
> > If you add `oldValue` in the body you need to add it in the declaration; if you remove it from the body you should remove it from the declaration too (unless you're relying on the semantics of calling the getter, which pretty much no one cares about).
> 
> The issue about calling the getter is the entire rationale for this proposal; if the argument is that pretty much no one cares about it, then no part of this proposal should be adopted.

Perhaps I expressed myself unclearly. What I meant is that pretty much no one cares about _the semantics_ of calling the getter (which in most cases should have no side effect). Avoiding calling the getter when you can is very desirable for performance reasons though.

---

<div class="post-metadata">

**Author:** ![Nevin](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/nevin/32/617_2.png) [@Nevin](https://forums.swift.org/u/Nevin)\
**Post date:** [December 6, 2019, 5:07am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/14 "2019-12-06T05:07:12Z")

</div>

> [@xwu](#):
>
> I don’t buy this argument. Instead of requiring you to write any particular variable name _n_ times, this proposal would require you to write the name _n + 1_ times. But the additional one time that you would need to write the name is _so inconvenient_ that you would choose a shorter name, even though you simultaneously _strongly_ favor the clarity of a longer name?

In the common case, n=1, so yes, doubling the number of times the name appears is in opposition to the principle of “Don’t repeat yourself”.

This is especially true for single-line bodies, where the long-but-clear name is great when it appears once, and excessive when it appears twice.

---

<div class="post-metadata">

**Author:** ![griotspeak](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/griotspeak/32/1966_2.png) [@griotspeak](https://forums.swift.org/u/griotspeak)\
**Post date:** [December 6, 2019, 5:33am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/15 "2019-12-06T05:33:40Z")

</div>

I am in favor of deprecating implicit use of `oldValue`.

---

<div class="post-metadata">

**Author:** ![clayellis](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/clayellis/32/43646_2.png) [@clayellis](https://forums.swift.org/u/clayellis)\
**Post date:** [December 6, 2019, 6:08am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/16 "2019-12-06T06:08:04Z")

</div>

I am strongly against deprecating the implicit `oldValue`. I understand the rationale behind the amendment — it changes the behavior/semantics — but when a user reaches for the `oldValue` they're, in a way, opting for a different behavior anyways. So why is this a concern? Isn't it as it should be?

I think @Nevin's arguments against the change are absolutely worth taking into account and I echo this:

> [@beccadax](#):
>
> Are we prepared to warn on nearly every use of `oldValue` in existing Swift code?
> 
> My personal opinion is that this warning will be unnecessarily disruptive to common usage and the proposal is just fine without the warning.

---

<div class="post-metadata">

**Author:** ![clayellis](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/clayellis/32/43646_2.png) [@clayellis](https://forums.swift.org/u/clayellis)\
**Post date:** [December 6, 2019, 6:17am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/17 "2019-12-06T06:17:14Z")

</div>

> [@xwu](#):
>
> I don’t buy this argument. Instead of requiring you to write any particular variable name _n_ times, this proposal would require you to write the name _n + 1_ times. But the additional one time that you would need to write the name is _so inconvenient_ that you would choose a shorter name, even though you simultaneously _strongly_ favor the clarity of a longer name?

I think this response is disingenuous because it leaves out the fact that we _all_ use code completion. Unless opening a parenthesis after `didSet` automatically suggested the spelling "`oldValue`" (which isn't a behavior Xcode exhibits anywhere) you leave it to the developer to type the full name out, which will lead to some misspellings or undesirable shortenings. On the flip side, with implicit `oldValue`, typing `o` then `Enter` to select the code completion for `oldValue` is just as many keystrokes as typing `x` twice (once for declaration, once for use.) Swift shouldn't be, and has never been, about counting keystrokes, though. So the entire argument really should be moved past. Clarity over brevity.

> [@xwu](#):
>
> (even if it is one letter long) is surely more clear than using a magical name declared nowhere (even if perfectly named).

`oldValue` is no less magical than the implicit `error` inside of `catch`, or `$0` inside of a closure, or `newValue` inside of `set`. This isn't magical in Swift, it's commonplace.

---

<div class="post-metadata">

**Author:** ![John\_McCall](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/john_mccall/32/1351_2.png) [@John\_McCall](https://forums.swift.org/u/John_McCall)\
**Post date:** [December 6, 2019, 6:46am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/18 "2019-12-06T06:46:24Z")

</div>

> [@michelf](#):
>
> Perhaps I expressed myself unclearly. What I meant is that pretty much no one cares about _the semantics_ of calling the getter (which in most cases should have no side effect). Avoiding calling the getter when you can is very desirable for performance reasons though.

We do know of some counter-examples. There's an API in Cocoa (I don't remember what) which checks that you don't read it before writing it; if you override it with a `didSet`, Swift will currently call the getter just to provide the `oldValue` parameter, and that call will violate the precondition. That's not just theoretical, it's an actual user-reported bug which will be fixed by this semantics change.

But the performance story certainly seems somewhat compelling to me on its own. We want performance to be more predictable in general.

To be clear, neither of those requires deprecating the implicit `oldValue` parameter.

---

<div class="post-metadata">

**Author:** ![bzamayo](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/bzamayo/32/764_2.png) [@bzamayo](https://forums.swift.org/u/bzamayo)\
**Post date:** [December 6, 2019, 10:09am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/19 "2019-12-06T10:09:51Z")

</div>

The current behaviour of calling the getter every time is unintuitive, I hadn't really thought about it happening at all before the first round of the proposal came up, so I think a change is warranted.

I regexed my current open project and I'd be facing about 80 fixits when this change was introduced, but realistically it would only take five minutes to go through and modernise them to the explicit form. (I've never had much luck getting the migrator wizard to work.) The real pain point will be all the `didSet` statements I will write in the future. Migration warnings are _fine_ IMO but you need the tooling support to be in lock-step.

As @clayellis mentioned, autocomplete needs to step up here. The status quo is:

![34](https://global.discourse-cdn.com/swift/original/2X/5/5f62ee724629dec9878d277afa17eacc7efbcd5c.png)

The fact this doesn't already have `didSet (oldValue)` in it is probably a primary factor as to why the implicit syntax form is so prevalent today. We collectively forget about using the explicit form because we aren't ever prompted to do it. The proposal text doesn't mention autocomplete at all — and while it may be obvious — it would be nice to get a firm assurance that it will be in place.

Swift rarely favours brevity over correctness and I personally would not want the fears of a minor 'source break' to stand in the way, particularly in this case where the change to modernise is so simple mechanically and effectively involves no decision making at all on the part of the programmer.

---

<div class="post-metadata">

**Author:** ![suyashsrijan](https://sea2.discourse-cdn.com/swift/user_avatar/forums.swift.org/suyashsrijan/32/5292_2.png) [@suyashsrijan](https://forums.swift.org/u/suyashsrijan)\
**Post date:** [December 6, 2019, 11:28am UTC](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396/20 "2019-12-06T11:28:28Z")

</div>

We can certainly improve autocomplete for `didSet` as a follow-up improvement where it would suggest adding `oldValue`.

[Next page](https://forums.swift.org/t/se-0268-review-2-review-didset-semantics/31396.md?page=2)
