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

I think it takes everyone a bit of time to acclimate to implicit parameters, but once I got used to them they quickly became my preference in many contexts.

There seems to be a common assumption that implicit parameters are necessarily unclear and the only solution is to make them explicit. However, there are tools in the toolbox that increase clarity without requiring users to write/read any code.

In that vain, I think there's an argument that the clarity issue with implicit parameters may be a tooling problem not a language problem. Xcode's code completion for them was rough early on, and they're still highlighted as just generic text. Compare this with how IntelliJ/AndroidStudio handle them in Kotlin:

20%20AM

Not only is the implicit parameter (it) bold, but it also adds labeling to make any implicit information clear.

I doubt something that complex would be added anytime soon, but there is currently zero indication in Xcode of what those names are, or where they come from. You have to already "know" about them. At the very least the font should be differentiated from other parameters, and the Quick Help would actually be, well, helpful :slightly_smiling_face:. Right now it just point you to where it's "declared" (e.g. didSet) with 0 explanation.

5 Likes

I agree that tooling can make it better, but it also has to be consistent across all platforms. While the vast majority of people use Xcode, there are many people who don’t (given Swift is available on Linux and Windows which doesn’t have Xcode).

I think a change to Xcode like above would take time (it would be great if we could do it in a way that can be utilised by third party IDEs as well) but in the short term, if we can at least improve code completion (which has been one of the reasons why people have got used to using implicit variables), then I don’t see the harm in writing oldValue explicitly. Now, whether you consider it “noise” is subjective, but writing it would pretty much be effortless if we make those improvements as well.

I've always found these special variable names odd and it's hard to know which ones are in effect. I think it'd be much simpler if the only available ones were $0 ($1 ...) and self.

Forgive me if this has been mentioned - I'm late to the discussion - or totally infeasible, but has it been considered to use the $0 and the alternative { oldValue in syntax as for closures here?

I.e.

didSet { _ = $0 }

and

didSet { oldValue in _ = oldValue }

I'd find that much more consistent with the rest of the language.

The same could possible apply to error as well.

1 Like

The main problem with it is that it creates inconsistency with other accessors. We would have to go and change the rest as well to make it consistent.

In contrast, didSet(oldValue) is something that already exists (and you can do the same with setter and willSet as well today).

Deprecating the implicit oldValue doesn’t need new syntax or any other changes - we can take advantage of existing machinery and reduce breakage.

I did and this was the response to that: Pitch: didSet Semantics - #79 by John_McCall

2 Likes

The Stack Overflow referenced by @suyashsrijan seems to indicate confusion over using the old value in general in didSet. The post references seeing oldValue used in another tutorial, but it's not clear whether that code used implicit oldValue or explicitly used that name. But the confusion noted is the poster seemed to think that they must do something with oldValue.

But if you remove every piece of Swift that has at least one Stack Overflow question about it, you end up with a very tiny language. :wink:

There are many things about Swift that can be confusing to people who are new to the language, or even more so new to programming.

Early on in Swift the philosophy of progressive disclosure was talked about. I don't know if it is still a guiding principle. But I think in general, when people begin using a feature in the language, some people are going to have questions, regardless of the topic. So I think it's natural to see questions. For me the things to look at would be whether something is a regular and constant point of confusion for many new developers, and if it's something that, once explained, just about all people seem to get pretty quickly and move on.

As I said in my previous post, I think the rationale stated in the proposal of needing to deprecate implicit oldValue to avoid confusion is not a strong enough rationale.

I also think the notion of getting rid of implicit variables deserves its own discussion thread with a full discussion of all of the reasons for doing so.

I have a feeling this review would receive more attention if it were titled Deprecate implicit oldValue in didSet because I suspect people have strong feelings about implicit variables in the language. I think the topic of potentially removing/deprecating implicit variables deserves its own proposal and conversation.

5 Likes

I would add that there's a difference between something which is simply unknown and something which is regularly misunderstood. Even if something unknown is asked about once by every developer who first comes across it, there's no problem. However, if developers continue to ask questions about a feature because it's difficult to use correctly, there may be room for improvement.

3 Likes

Sure, but clarity of variables is not the only motivation behind the deprecation. We can surely have a separate pitch about implicit variables, but I’d imagine it would be quite subjective, where as deprecating oldValue is not just about the subjective or stylistic choices of people, but also to make the performance implication more explicit to the reader. If we remove implicit error from catch or newValue from the setter, then it only makes the variable explicit. There isn’t any other effect. With oldValue specifically it’s different, because there is a performance impact associated with it, so in addition to just making the variable explicit, we are making the performance implication explicit as well.

But as I said before, I’m totally fine with it being a separate discussion. However, the impact are going to be the same regardless of whether we do it now or later.

Not just a performance impact, but as the proposal title says, new semantics.

1 Like

Yes, I think the point at hand in the review is whether people will be confused by the fact that using the implicit oldValue will fetch the old value.

I don't think there will be much confusion since you are explicitly typing oldValue into the body of didSet so you are very much expecting to get the old value.

My understanding is that the performance impact is the time/resources to get and store the old value. I think if you understand the impact when you explicitly ask for the old value, you would also understand when you use the implicit form.

1 Like

It's not so much about the "you" who is "explicitly typing," but the many "yous" who will have to read and understand the meaning of code they didn't type. That the semantics meaning of didSet changes based on how it's implemented makes it harder to reason about.

Imagine if a member declared with let could be mutable based on the presence of a setter. You could create a language that does that and likewise argue "I don't think there will be much confusion since you are explicitly typing set { ... }"--but Swift doesn't.

1 Like

Yes, I mentioned in my original post that the explicit version has the benefit that you can see if the old value is being accessed without having to scan the body of the didSet implementation
to see if oldValue is being used.

But a method doesn't declare every property or variable that it accesses in its method declaration. If you want to see what properties a method accesses you need to read the body of the method. It does not seem to be a point of confusion that if a line of code within a method accesses a property that the value of that property is retrieved.

I see this as very similar. If you want to see what is accessed in didSet, read the body just as you would do a method. If you are accessing oldValue you are getting the old value.

If there is a very long implementation of didSet that accesses the old value, there is nothing stopping developers from using the explicit version that is already available, if they think it improves readability or makes it more visible that the old value is being accessed.

But for a very short implementation of didSet, it is very easy to scan to see if oldValue is being used, and clutters the code to have it need to be declared.

I would say that the example @xwu provided of a let member that can be mutated because you provide a set implementation is very confusing. Even if I wrote the code myself and came back to look at it a few days later, I think that would be confusing to me.

But if someone who didn't write the code reads the body of a didSet similar to how they would read the body of a method, to see what values were being accessed, I think reasoning that accessing oldValue is getting the old value would not be confusing.

If I understand correctly, the one thing that needs to be reasoned about with regards to didSet semantics introduced in this proposal is whether the property getter is called or not to retrieve the old value. (I could be missing something.)

I think that, just like reading the body of a method to see what properties / values are accessed, reading the body of didSet to see if a particular value, oldValue, is accessed is not out of the ordinary for reading code or particularly confusing. It seems to be exactly the same as how developers reason about what values are accessed in the bodies of methods.

5 Likes

If I understand correctly, the revised proposal is stating that simply writing

didSet (oldValue) { }

will cause oldValue to be invoked. If this is not the case, then the explicit mention buys absolutely nothing, in terms of conveying semantics. An explicit parameter name does not cause the variable to be read, or getter to be invoked. Why would anyone expect that to be the case here?

1 Like

If I create a function that takes one argument, then the caller must supply that argument. Here, if I implement didSet(oldValue), then the synthesized code which invokes it must actually obtain oldValue and pass it to my implementation as the first argument. Why would it be any different?

This still seems to me like it's fixing a problem the vast majority of users will never encounter.

  1. I would estimate the percentage of users to whom this will matter is very small (< 5%). The fact that it's taken several years for it to even be recognized as enough of a problem to warrant a change would indicate the percentage is <1%.
  2. IMO things like didSet shouldn't be longer than a few lines in the first place. If your didSet is so long you can't very quickly reason about what's happening, it should be put in a function.
  3. For the few remaining edge cases, the readability issue can be solved with tooling. Simply highlighting the implicit name should allow scanning the didSet body quickly to check if oldValue is used.
2 Likes

There are two distinct problems here. The problem that this re-review is tackling is that the semantics of didSet will depend on its implementation, which does not exist today but will exist with the introduction of this proposed change if the syntax does not change in tandem. The problem that has taken years to address is the issue that setting a value can require unnecessarily accessing the getter; this will be fixed.

It is not in contention that the problem in this re-review would be tackled by tooling. Either the tooling will insert the proposed syntax change once wherever didSet is written, or it will have to highlight or otherwise display some indication every time that didSet is read. One of these approaches is more straightforward and reliable than the other.

2 Likes

My response is specifically about the deprecation, which is what this second review is about. The change in semantics has already been approved and is something I've always been +1 about.

The thing is, the semantics are changing for didSets that don't use oldValue. For implementations that do use the value, it will be identical, yet that is what would be getting the change to now-required syntax.

If the motivation is to indicate a change to existing behavior, it's being done for the wrong use case.

5 Likes

Why is scanning the body to look for the word set very confusing to you, but scanning the body to look for the word oldValue not confusing to you?

@xwu

Why is scanning the body to look for the word set very confusing to you, but scanning the body to look for the word oldValue not confusing to you?

Because in your brief description of the the feature, I had thought that the set block you described could be added anywhere within the struct or class definition, which would mean needing to look through the entire struct or class to see if that overriding behavior had been added.

The motivation is to allow users more easily to distinguish the two behaviors.

As you state, there is broad agreement that didSet implementations that do not use oldValue should not incur the performance cost associated with preserving that value.

If you both support the change in semantics, and do not support changing syntax for the current behavior, then you must come out against any attempt to distinguish the two semantics at all. If you believe that having different syntax to distinguish semantics is important for the clarity of code, then it must be that the case of using didSet with oldValue changes its syntax.

(Unless you mean to suggest that the new semantics come with some new syntax à la nonmutating setnongetting didSet or something like that. If I recall, I had pointed this out as an option earlier, but it was not entertained. Thus, we are left with only two alternatives.)