Pitch: didSet Semantics

[hisekaldma (Jonathan Hise Kaldma)]

@jrose

I'd be happier if we said "using oldValue without explicitly declaring it is deprecated" (i.e. you should write didSet(oldValue) , as you've always been able to do, if you want to use it).

I think this would be pretty bad UX. One very common use case for didSet is to do:

var value: String {
  didSet {
    if value != oldValue {
      // Do something
    }
  }
}

In all of those use cases you’d then have to write didSet(oldValue) instead, making your code harder to read without any real benefit to you. We’d essentially be penalizing the common case for an improvement to the uncommon case. Feels like the wrong trade-off.

[DevAndArtist (Adrian Meister)]

This may be true to some degree, but I think the fixes to the problems that are mentioned overweights the lost convenience. As @Chris_Lattner3 once said in response to some language sugar pitch of mine, it would be bad if we provide sugar for convenience which will prevent us from having other important language features.

This proposal I assume will also fix the key-path bug that I hit so many times already. Every time I hit it, it forces me to uglify the implementation with a closure based workaround because it prevents me from using key-paths since they suffer the same issue and call the getter where I expect them to call the setter only.

I‘m +1 on this change.

Keypaths subscript setter accesses the properties getter?

I was surprised that the key-paths subscript setter does access the properties getter. This made this example crash: struct DelayedMutable<Value> { private var _value: Value? = nil var isInitialized: Bool { return _value != nil } var value: Value { get { guard let value = _value else { fatalError("property accessed before being initialized") } return value } set { _value = newValue } } /// "Reset" the delegate so it can be initi…

[GetSwifty]

I'm +1.

It seem to fix some potential and/or real bugs without otherwise causing problems.

I think it's ok not pass it implicitly, as it's simply not doing something the user isn't asking to happen anyway. This could potentially see a slight performance hit if oldValue is added later, but that seems trivial and not worth worrying about.

It also doesn't seem necessary to require declaring oldValue. It's not source compatible and feels like the opposite direction for what approach should add complexity. The default use case for most users probably will still be to use oldValue, so if usage should be explicit it should be for when users don't use it. e.g:

var foo: String {
    didSet(_) {
        //Explicity saying oldValue isn't wanted, so it won't be passed
    }
}

[jrose (Jordan Rose)]

@Panajev

Considering the references to Swift 5.1 elsewhere (do not want to say Xcode beta docs) and what are you saying now, risking to sound mega dense, could you just clarify whether the upcoming Xcode 11 will get both ABI and module stability with the compiler that ships with it please?

The concerns in this thread is about whether there will be an additional language mode (-swift-version, or "Swift Language Version" in Xcode) for Swift 5.1 that's different from Swift 5 mode (there will not be) and when the next such language mode will be introduced (unknown, up to the core team). Nothing has changed with the compiler shipping in Xcode 11 betas (Swift 5.1), or whether that compiler supports ABI and module stability (yes). Sorry for any confusion!

[Karl]

@jrose

I'd be happier if we said "using oldValue without explicitly declaring it is deprecated" (i.e. you should write didSet(oldValue) , as you've always been able to do, if you want to use it). That way it's obvious from the code whether the getter will be called or not.

I agree with this. I find the current behaviour quite strange (AFAIK nothing else in the language injects named variables in to your scope — except perhaps ‘self’ for instance methods).

It will break source, but at the same time, it should be quite simple to detect this and offer fix-its and implement a compatibility behaviour for older language versions.

[bzamayo (Benjamin Mayo)]

@Karl

(AFAIK nothing else in the language injects named variables in to your scope — except perhaps ‘self’ for instance methods)

This kind of implicit variable binding arises in try/catch too for a magic error variable. I also think it is strange and wouldn't mind it being removed. Frankly, it is so obscure people always write catch let error explicitly anyway.

In case it's not clear what I'm describing, the language currently allows this code:

do {
    try operation()
} catch {
    print(error)
}

[Karl]

That’s a good point, I forgot about that. Still, I consider that more like ‘self’ because no additional code can be executed by binding the thing to a name.

[DevAndArtist (Adrian Meister)]

set does inject newValue.

[GetSwifty]

@Karl

AFAIK nothing else in the language injects named variables in to your scope — except perhaps ‘self’ for instance methods

My known list of injected variables off the top of my head:

• self in instance scope
• self in static/class scope
• $0, $1 etc. in clousures
• newValue in willSet
• oldValue in didSet
• newValue in set
• error in catch

I have a feeling there a couple more I can't think of off the top of my head, but that's not too small of a list. as it is.

Given how common it is in Computed Properties and Property Observers, how much (subjectively) cleaner it is to not have a parameter list there, and how long this behavior has been available it seems unnecessary to add such a requirement.

[thomasvl (Thomas Van Lenten)]

@jrose

The concerns in this thread is about whether there will be an additional language mode ( -swift-version , or "Swift Language Version" in Xcode) for Swift 5.1 that's different from Swift 5 mode (there will not be) and when the next such language mode will be introduced (unknown, up to the core team). Nothing has changed with the compiler shipping in Xcode 11 betas (Swift 5.1), or whether that compiler supports ABI and module stability (yes). Sorry for any confusion!

I'm likely not fully following the complexity from the compiler pov, but what you saying here sorta doesn't feel like it lines up in part with the already updated docs:

The Swift Programming Language: Redirect calls out

…the following changes are available only to code that uses Swift 5.1 or later…

(along with other parts there)

So I had read some of those docs thinking there already was a 5.0 vs. 5.1 mode to help catch/flag when using things that do require the newer runtime/etc. Does that mean things that require 5.1 runtime support instead would error if the min os version wasn't high enough (or things weren't gated with availability checks)?

TVL

[suyashsrijan]

Proposal Updated

1. API Resilience section has been updated.
2. Polished up the overall proposal a bit.

Proposal has also been submitted to Swift Evolution repo: https://github.com/apple/swift-evolution/pull/1068

If you have any suggestions or improvements, please feel free to let me know. Thank you!

[Alejandro (Alejandro Alonso)]

@suyashsrijan

Another title change - it's called "Accessor Semantics" now.

This title is too generic for what is actually being proposed. I think you’re previous “didSet semantics” title was in the right direction.

[suyashsrijan]

Okay, I will change the title. I agree "didSet Semantics" is good, but I found that it slightly misleading due to the change to _modify, but I guess the proposed changes are linked together so it's probably fine to not make it too generic and keep the "didSet" in the title for clarity.

EDIT: Title change reverted. Thanks!

[Rod_Brown]

Nice. I’m glad to see a PR up to fix this issue. I’m hoping it can come soon and doesn’t have to wait for a major version. If people were relying on the old behavior, I feel like this is relying on an odd defect in the compiler, more than a feature?

[clayellis]

I can appreciate that the suggestions to deprecate the implicit oldValue in didSet were called out as a potential future direction. I think we could probably go back and forth on whether or not that should happen now or later for a while. Thanks @suyashsrijan

[xwu (Xiaodi Wu)]

@GetSwifty

• self in instance scope
• self in static/class scope
• $0, $1 etc. in clousures
• newValue in willSet
• oldValue in didSet
• newValue in set
• error in catch

Self and self are obviously special for myriad reasons. $0 is an otherwise invalid variable name. Personally, I would be fine with all the rest going away.

If there is a semantic distinction that can be made clearer when we get rid of oldValue in didSet, then it seems like the ideal time to make this change would be when we introduce the distinct semantics.

[masters3d]

+1

[Vinicius_Vendramini]

Looks really good, +1 for me.

I do have a couple of issues with the wording of the text, though:

@suyashsrijan

However, the possibility of that is very small.

It feels weird to read this without some evidence backing it up. I understand why you’d think it’s true, but personally I have my doubts. Maybe just clarify that this is your opinion?

@suyashsrijan

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.

I only understood why it doesn’t provide any real benefit after reading the discussion here. If you’re gonna make this claim I’d add an explanation of why it’s true.

Still, great work :)

[suyashsrijan]

Thanks, I have updated the proposal text!

[daveanderson]

As author of the [SR-11280] PropertyWrapper property + didSet Property Observer results in get before set failure · Issue #53681 · apple/swift · GitHub bug where a "Delayed" property wrapper crashes due to the implicit property access to get oldValue I'm a strong +1 on this proposal.

I'd be happier if we said "using oldValue without explicitly declaring it is deprecated" (i.e. you should write didSet(oldValue) , as you've always been able to do, if you want to use it). That way it's obvious from the code whether the getter will be called or not.

I agree with this ^ statement that the use of oldValue without explicitly writing didSet(oldValue) should be deprecated. IMO it is overly "magic" for the getter to be called iff oldValue is referenced (and not calling the getter if it is not).

Pushing the $0, $1 shorthand for closures to the extreme, one could imagine a proposal that newValue from willSet, oldValue from didSet, newValue in set and error in catch should each be available via a shorthand $0. I would still strongly prefer explicit opt-in for each one of these vs a shorthand $0 value.