Pitch: didSet Semantics

Has anyone considered making "does not take the oldvalue" behavior explicit? This seems like a power user feature, and making it explicit in code is good.

For example, it could be as a simple as didset() { where the () indicate that oldvalue isn't taken.

Edit: Yes this was discussed upthread. This seems like the way to go to me. If we did this, there is a simple and clean way to handle this:

The compiler emits a warning "oldValue not used" for a didSet without an (oldValue) specifier. It would produce two notes along the lines of:

  1. "insert () to explicitly turn off oldvalue"
  2. "insert (oldvalue) to explicitly take and ignore it.
7 Likes

Well, we can propose eliminating the implicit parameter, but I think that will be much more controversial.

Should we also eliminate the implicit parameter from willSet? The parallelism argument is strong, and it would have some concrete advantages — for example, we could trigger a parameterless willSet before an entire read-write operation instead of at the end just before setting, which might allow the modification to happen in-place.

3 Likes

That would be fantastic, in my view.

Any source-breaking change would (and should) be controversial but I think being able to reap concrete benefits, while allowing the user a clear (and, ideally as here, automatable) upgrade path is the strongest argument for such a change.

2 Likes

So didSet, alone among the accessors, just has to be given this empty parameter clause forever because we got the semantics wrong for awhile?

2 Likes

It feels like this pitch started with an idea for a relatively minor optimization and is trending toward a source-breaking change that would make these kinds of handlers more cumbersome to write. I can understand how a change like this would lead to a more rigorous language, but I don't see what problem it's solving beyond the initial, very limited-in-scope optimization.

3 Likes

Well, it's not just an optimization because (1) reading from the old value can be semantically problematic and (2) if we can enable in-place modification, the "optimization" can eliminate linear costs and thus unblock the use of observers for certain kinds of work. But yes, there is a substantial breadth-of-scope inflation going on here.

6 Likes

Wouldn't the expected form be didSet(_), like it is done in pattern matching ?

I'm personally in favour of the following:

  • didSet(_) when ignoring oldValue,
  • didSet(x) when the developer wants a different name,
  • didSet when the developer wants oldValue

Swift tries to avoid implicit things as much as possible and set, didSet and willSet are almost always implicitly using arguments so I don't like them much in their base form, though I understand the regression and source-breaking change it would be to change that.

That is how you would express performing a read for its side effects while ignoring the result; it can’t mean not performing a read.

9 Likes

I don't understand the undercurrent of your question. Are you saying that didset() or didset(oldvalue) is onerous to write or difficult to understand?

Accessors aren't that common and are not important to sugar at this level (imo) given the semantic issues that sugar raises. I'd rather have the behavior be explicit - this makes it easier to reason about.

2 Likes

You're right, I missed that.

It's inconsistent that didSet would be required to have a parameter clause, even when not taking any parameters, when no other accessors are. Imagine teaching this to a newcomer: how would you explain that you can write get and set and willSet, but you can't just write didSet?

Here's how I think about it. Your proposed warning practically forces programmers using oldValue in didSet to change their code in a future release of the Swift tools. I don't see the benefit of doing that over making it mandatory, but conditional on a future compatibility version (i.e. it's an error to use an implicit oldValue in Swift 6+ mode; that's an easy special-case diagnostic/fixit/migration). Once we're doing that, we might as well leave the people who aren't using oldValue alone.

9 Likes

Do we have an idea what fraction of didSet implementations do and do not use oldValue?

2 Likes

Quite a lot of them don’t use oldValue: Sign in to GitHub · GitHub

A lot of code uses didSet to trigger some other work and doesn’t really care about the oldValue.

9 Likes

400K+ results on GitHub where didSet doesn’t has a reference to oldValue.

30K+ results on GitHub where didSet has a reference to oldValue.

11 Likes

Looks like I’m wrong about what the common case is.

I still think requiring didSet(oldValue) would be an overall usability and teachability regression. For the language to make sense as a whole, we’d have to require willSet(newValue) too. Requiring set(newValue) is clearly out of the question.

So then we’d end up in the situation that willSet and didSet require writing out the parameter if you need it, but not set. Maybe that’s an okay place to draw the line – willSet and didSet are definitely more advanced features than get and set. But to a newcomer, the distinction would feel arbitrary and pointless. Imagine someone just learning to program:

– ”Why can't I just write didSet when I can write set?”
– ”Because otherwise there would be an implicit read that might have side effects.”
– ”Come again?”

4 Likes

Why?

2 Likes

I don't see how the source breakage could possibly be justified. Practically every use of set references newValue. That's very different from the situation with didSet and willSet.

I skipped a few last posts but will study them in a moment.

Maybe this idea is completely wrong, so don‘t judge me too hard on this.

Could accessors act syntactically like functions with a trailing closure? (This would however imply the deprecation of didSet(oldValue), set(newValue) etc.)

Instead we would either have a closure parameter list or not.

didSet { oldValue in
  ...
}

If the user used an implicit oldValue the compiler would raise a warning and provide a fixit to add a missing parameter list.


This is just a syntactic alternative. As I mentioned, I‘m not sure it will solve any of the issues mentioned upthread, but I wanted to throw this into the bucket of proposed options.

1 Like

I don’t see how that achieves anything.

3 Likes

Wow...that's much more heavily weighted than I expected.

In that case this change will not only fix a few bugs, but will also have a meaningful positive performance impact in aggregate.

To avoid a positive change getting caught up in tangential BikeShedding gridlock, IMO this should go through with no syntax changes. If/when someone wants to put a proposal together to re-evaluate the syntax for PropertyAccessors/PropertyObservers, (is there a single name for them?), it should be considered separately.

8 Likes