As I have said in the previous thread, if there were a clarity issue that could be improved by getting rid of implicit newValue or implicit error, I would be in favor of removing those features too. They are no less magical. In fact, were it on the table now, I would be in favor of removing those now if the consistency argument is considered to be paramount.
(Closure expression notation is another kettle of fish since their entire existence is premised on layers on layers of brevity to improve expressiveness.)
I'm a little surprised by the more negative feedback on this review compared to the previous one, although the last review did have a somewhat small subset of participants.
My overall feeling of this proposal remains the same however, and I'm strongly in favor of this change. The clear performance win that this will give us is, IMO, far greater than then the relatively small burden of running the automatic migrator. I'd also echo @xwu's feelings about removing some of the other implicit variables that come in Swift. They have always struck me as out-of-place in Swift.
And that bug ought to be fixed. That doesn't mean the syntax has to be changed.
I think adding an explicit parameter in the didSet declaration does very little to make people understand the getter will be called as a side effect. And if you understand that, you'll understand that writing oldValue in the body does it too, implicit declaration or not.
Before this proposal I was under the impression that using oldValue in didSet would be a little worse performance-wise because work had to be done to preserve the old value. That extra work would consist of calling the getter and storing the value in a temporary. Turns out I was wrong and it was naively doing that extra work in every cases.
This proposal is only bringing the semantics in line with what I had pictured they'd be when I first saw didSet could access that oldValue in Swift 1. The surprising part of the old semantics is gone. Great!
But now we'd like to replace the semantic surprise it with a syntactic inconsistency that you have to explicitly declare oldValue in didSet. This is like changing the syntax to make everyone know you've fixed a bug. It makes no sense to do so, especially since the warning isn't going to show up in the few cases that truly depended on the old semantics.
If we want to change the syntax to deprecate implicit declarations, we ought to consider changing it everywhere (set, willSet, catch), not just in didSet. Let's make it the main topic of a separate proposal, not the sidelines of a "Refine didSet semantics" proposal that sounds mostly like a formality to clear up the various implications of fixing a bug. I'm sure many people love those implicit parameters and will have something to say about this.
These are my thoughts exactly! This proposal is great and should be completed. The last-minute syntax change feels unnecessary for the reason Michel articulated.
Like I said before, even though I oppose it, I understand the rationale. Perhaps we can save the proposed syntax change for another pitch? Maybe we can take a look at implicit variables as a whole later?
It’s pretty clear there are some strong feelings about this change, yet we all agree on the core change of the proposal. Calling the getter inside of didSet when the oldValue isn’t referenced was a bug, we determined that, and we have a fix. So let’s fix it and then have a more focused conversation about Swift’s current use of implicit variables.
(I hadn’t read last paragraph of Michel’s reply above where he says the same thing.)
This makes sense to me. The discussion in that separate proposal would let us really drill into the cost-benefit of the changes. Furthermore, changing all of those at once would be easier to explain to users.
^^ This. IMO it's unrelated to the core of the proposal, which is fixing a clear performance issue. If implicit parameter names are being reconsidered, they should all be reconsidered at the same time. Not added onto a loosely-linked (and IMO obvious +1 ) proposal.
I think I said (read?) this in the original review/discussion, and I still agree: This change is so in the weeds that it will be incredibly rare for the nuances to cause problems. I don't think it should require any code/syntax changes, and I could even see it having been added without a proposal as a compiler optimization.
I may be in the minority here, but I prefer implicit parameters for short functions as IMO it removes redundant code in a context where clarity is not a problem. For me, extra syntax is an extra roadblock for writing and understanding code. It's also a roadblock for new programmers.
It seems programming languages on average are moving towards implicit naming, not away from it. That doesn't mean Swift should as well, but I think it should be considered. Removing the option (not requirement) for implicit oldValue is, for me, a UX regression.
After hearing some of the feedback, I'm also starting to have reservations about the deprecation as being part of this review. My hesitation is mostly around whether users really need to know about the possible performance implications of using oldValue so much that they should have write out the oldValue declaration explicitly. And even then, I imagine most users who weren't around for this review will not understand that the reason you need to write out oldValue is to make the performance implications explicit. Swift's performance is already somewhat hard to reason about for anyone who isn't fairly familiar with the nitty gritty of Swift's ARC and ownership model, and the Swift book doesn't go into a great deal about how to write the most performant code.
If we can fix the current performance issue without also introducing the deprecation, then I don't think we have much controversy here, and I think we could fire up a discussion to talk about removing all of these kind of implicit variables, which I am still behind.
I'm 100% in favor of the change to avoid accessing oldValue unless you actually use it. I'm only maybe 10% in favor of getting rid of the automatically synthesized oldValue label. That seems like a bridge too far and it should be another proposal that unifies that sort of magic across the entire language all at once and not just in one place.
Are you suggesting we just tie in the rest of the implicit variables with this proposal as well? I wouldn't in theory be against such a move, given that accepting this deprecation would introduce an inconsistency*. But I think we'd want @Ben_Cohen to weigh in on as review manager before go too far down that path.
*: I guess whether or not you call it an inconsistency depends if you view the reason for requiring the explicit declaration as being solely for making performance implications explicit vs just saying that all property observers must have explicit variables. Although I don't think most users are going to see the performance argument as being one that is very strong, given that it requires understand how Swift works at a lower level. (Which still should be mentioned in the Swift Book for property observers anyway)
Could we get more clarity on why adding the oldValue declaration helps to clarify that the getter is being accessed? The proposal simply states that it reduces confusion but does not say why. It seems obvious that the only way oldValue could work is by accessing the getter, so I don't think we really need any additional clarity. But even if we did, how does didSet(oldValue) make it any more clear that you're accessing the getter compared to just using oldValue? I don't see any connection there.
Right (well, localized to just didSet). We're getting feedback on the idea of deprecating implicit oldValue; if the community is against that idea, at least in isolation, then the Core Team will likely accept the unamended proposal.
The feedback appears to be that a (sizable?) portion community is against the idea in isolation, but mostly understanding and willing to further the conversation in the larger context of the rest of the implicit variables found throughout the language.
I'd like to expand on this a bit. I believe the material change to the proposal in this regard is the following.
In order to avoid confusion as to when the getter is called or not, the implicit oldValue is now deprecated.
I would appreciate a more detailed explanation (new or existing that I'm unaware of) that explains how the proposed change accomplishes the goal "to avoid confusion as to when the getter is called or not"
In order for this change to be warranted it, IMO, needs to meet these criteria.
The problem, (the getter being called unnecessarily), warrants a change in the language.
The behavior of the fix runs counter to user expectations.
The problem being solved is harmful enough to warrant actively requiring a user decision.
The proposed solution gives users enough information to intuit the implications in their application.
The proposed solutions, on balance, improves the UX of the language.
I don't see a strong case for anything other than #1.
For #2, my expectation is the getter should already not be called unless oldValue is accessed. Maybe others have different expectations?
For #3, the harmful behavior is the current status quo, and such nuanced performance implications are only a concern for a subset of users, only some of which would ever encounter this problem. This seems to be controlling for a corner case of a corner case of a corner case.
For #4, Having a named parameter does not, IMO, indicate a performance implication in any existing context. If I was unaware of the pitch, it would simply seem like an arbitrary syntax requirement.
For #5, As said previously, for me the depreciation would be a pure negative for my UX.
I don't like the idea of deprecating the implicit syntax, because forcing the developer to being explicit for something the compiler can easily handle is source of errors.
A developer may need oldValue at some point, and change the declaration. If later it changes the implementation and remove usage of oldValue, the compiler will still continue the generate the unoptimized version, just because the declaration was not updated.
With implicit oldValue, you don't have to remember to change the declaration each time you update the implementation, the compiler just know what it should do.
I'm strongly -1 on warning for use of oldValueas part of this proposal.
I think there is a discussion to be had about the pros and cons of implicit arguments, and when that happens it should span cases like oldValue, the implicit error in do/catch, implicit self and all the other places where it occurs.
I don't see any reason why oldValue is special. I don't see why the compiler should warn in this one case and not in any of the others. Most importantly, I don't think a decision as radical and controversial as this should be rushed, nor should SE-0268 be blocked from landing while we make it.
Well, it’s fortuitous then that you bring up self. We have special rules in the language where explicit self is required in order to minimize the risk of unintentional capture, and in fact we just reviewed a proposal refining those rules.
The reason that oldValue is special is that whether or not it is bound will make a difference in the behavior of your code going forward thanks to the rest of this proposal.
It is true that the proposed deprecation is meant to help human readers, and imperfectly at that: so too the rules around implicit versus explicit self. Helping humans reason about the behavior of code is important, and I’m glad the core team takes it seriously enough to consider incorporation of syntax changes in tandem with changes in behavior.
So, I guess I disagree with your conclusion. This isn’t about people’s general feelings about the value of implicit bindings. This proposal is about changing the behavior around accessing oldValue and correspondingly changing the syntax of the language to help humans reason about the code.
It is reasonable to argue that the breaking change is too much at this stage in Swift’s evolution, but it absolutely should be argued now and not at some later stage.
In essence, the core team has already agreed with that. So let’s have at the argument!
There is definitely more convenience in the old/current way; there's less typing, simple as that.
I don't think forgetting to remove it is an issue though. Presumably, the compiler would generate warnings if the oldValue is declared in the didSet but unused. Also, I would expect it to be a rare scenario for someone to declare oldValue and later not need it and then have to remove it.
Presumably, the compiler would generate warnings if the oldValue is declared in the didSet but unused
It won’t, to preserve the old behaviour (see the source compatibility section of the proposal), however that can changed if required.
As I mentioned above, typing is not really a huge concern because we can certainly update code completion to start suggesting oldValue in places where it didn’t before, and that will make the transition a lot easier.
) proposal.