SE-0345: `if let` shorthand for shadowing an existing optional variable

Hmm...

// foo is external and doesn't know about CGPoint

if let pointOfIntersection {
    foo(pointOfIntersection.x, pointOfIntersection.y)
}

Do I really need to repeat "pointOfIntersection" 3 times in two lines?

This is much better IMHO:

if let v = pointOfIntersection {
    foo(v.x, v.y)
}

Anyway, one line or few lines - doesn't matter, example is showing the case where a variable is used several times in a short block, where "several" is as few as two. Short block is the key (if i was unfortunate to use and support code where a function spawns tens / hundreds lines i'd probably think differently.)

Here are a few relevant examples from my code:

example 1:

    let v = snapshot.itemIdentifiers(inSection: section)
    return indexPath.row < v.count - 1 ? v[indexPath.row + 1] : nil

example 2:

    if let v = details.leading {
        base = base.leading(v.leading)
    }
    if let v = details.weight {
        base = base.weight(v.weight)
    }
    if let v = details.opacity {
        base = base.opacity(v)
    }

example 3:

    if let v = visibleResources[key] {
        visibleResources[key] = v.visibleCount > 1 ? Resource(v.visibleCount - 1, v.version) : nil
    }

None of these examples would benefit from using longer names for "v" ("itemIdentifiers", "leading", "resource" correspondingly), at least not from my POV. I appreciate this is subjective.

1 Like

It’s subjective but ultimately you’re arguing against not only the Swift community but the industry as a whole. Additionally, you’ve made this point repeatedly so I’m not sure restating it yet again has much value.

3 Likes

You can opt out of the new shorthand sugar and just do what you like, right?

This continues to work:

3 Likes

I must admit I hope this review is over soon. I’ve read the same arguments over and over and over and over and over. It’s clear there are some who are vehemently opposed. And likely a vast majority who will gladly use this shorthand when it is approved and available. As it should be. Because I would use it everyday, gladly, for the convenience and clarity, dare I say, that it will provide.

  • What is your evaluation of the proposal?

I’m very positive. This will be a useful and very appreciated tweak to the language. Language is hard. But compared to all of the other stuff, generics, etc, this concept is easy as pie.

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

Definitely. The current pattern of redundancy is a wart on the language.

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

Yes.

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

I can’t think of an exactly analogous feature I’ve used, but many similar conveniences that have been appreciated.

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

I’ve followed every drop of hard spilled blood in the 200+ posts on this thread and the countless more in the previous sessions. Let’s put this baby to bed, by approving it and making this clear and useful shorthand available to those tens of thousands of coders who write swift day in and day out.

8 Likes
if past_due_date { // and it is
	let finish_this = one_way_or_another
	something_we_can_all_agree_upon()!
}

I agree that some subjectiveness is involved when people reviewing features according to Swift guidelines.

But regarding the fact that almost half the reviewers in the thread has +1 and the other half has -1 for this sugar syntax, I feel that the Core Team is not doing their job: such a controversial change should not come as the proposal for formal review (this thread is just as duplicated as the original pitch `if let` shorthand - #57 by mtsrodrigues, with so many opposite arguments and no convergence can be reached). The Core Team should at least help ensure the formal proposal address the most significant concerns in the pitch stage and reaches the bar for a formal review, which is not the case.

I should not be the only person has this feeling.

7 Likes

Please do not speak for the other. Reading this thread, there is no evidence that one side is vastly superior in number to the other side.

That's in part why I'm against this proposal. Having two ways to do the same thing is generally bad for a language as it source of fragmentation, code style flame war, stack overflow threads and more…

3 Likes

+1

I don't feel strongly about this, but I'll use it if it's available - I switched from if let nonOptionalVariable = variable to if let variable = variable as soon as that was possible, and I'll do the same in this case. I think the shorter version is clearer and easier to read. I generally prefer to use shorter forms of Swift syntax (closure syntax, omitting return etc.)

Effort: I've followed this thread and considered the arguments from both sides.

3 Likes

I think it’s important not to mischaracterize one way or the other. As of this post, it is running about 60% for the proposal among posts that state a preference (approximate because not every post clearly states a preference, some posts are follow ups to previous posts, and some folks change their evaluation over time).

Of course, a proposal review isn’t a scientific sampling of all Swift programmers and the Core Team doesn’t simply look at the number of upvotes and downvotes to make their decision.

Reading through the Swift Evolution Process document, there’s nothing requiring a proposal to have a particular level of convergence of opinions before it is reviewed. My understanding is one purpose of the pitch phase is to bring out various objections, alternatives, and edge cases so that they may be addressed in the proposal before it is reviewed. But people still may have differing opinions about a proposal while it is being reviewed.

Reading over the objections to the proposal, they seem to largely fall into two categories:

The first being that this proposal favors 'brevity over clarity' and potential downsides of omitting the RHS. The proposal makes the case that this change achieves 'clarity through brevity' by eliminating boilerplate.

The second being a desire for an alternate shorthand syntax. Rationale why the proposed syntax was chosen over various alternatives is detailed in the proposal.

So, I think the proposal does address the most significant concerns which is what is expected of a proposal when it comes to review.

I also agree and think there is always a degree of subjectiveness reviewing features. I think the review process gives people a chance to express their views of the pros or cons of a proposed feature, which gives the Core Team a variety of feedback and information they can evaluate in reaching their decision.

4 Likes

As others has pointed out, this proposal actually falls in common rejected changes. I am not saying a convergence is required for a formal proposal to be reviewed, I am just stating the fact that this formal proposal contains so many opposite opinions (with 200+ comments!) as the previous pitch. I don't think there will be a convergence at the end (as with most of other accepted proposals).

For the first category, this is just your opinion (as with half of the reviewers in this thread). For the second category, my understanding is that some reviewers in the thread just think the existing if let binding works fine and nicely.

2 Likes

What is your evaluation of the proposal?

+1 as written.

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

Borderline, but a nice UX improvement for many.

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

IMO yes, but I can see why others may disagree.

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

N/A

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

Read the proposal and followed the review thread.

3 Likes

I think there are are fair number of proposals where there isn't convergence of opinion of review participants at the end. A recent example would be SE-0307: Allow interchangeable use of CGFloat and Double types. People made their cases for and against and then the core team made their decision.

I wasn’t really making an argument for or against this proposal here. I was saying that I think the proposal does address the significant concerns brought up in the pitch phase.

2 Likes

My 2¢:

I was initially opposed to this proposal, agreeing with all the common opposing arguments. Then I looked at my own code - using the Xcode regex if let (\w+) = \1(,| \{) - and considered whether I honestly thought the code would look better or worse using this shorthand. I confess I think I would like it better.

16 Likes

Ooooh, thank you for this regex! I hadn't tried to do a regex search for this before.

Quick anecdotal data for this from Airbnb's iOS app codebase (over 2 million lines of Swift):

  • 13,951 instances of if let
    • 3,752 (27%!) of those are if let x = x
  • 15,294 instances of guard let
    • 6,195 (40%!) of these are guard let x = x
15 Likes

Yesterday @Douglas_Gregor pointed out an instance where while let foo = foo is meaningfully useful: when an actor has a mutable optional Task property, the loop body awaitS that Task’s value, and the task might either clear out the task property or populate it with a new task:

actor foo {
  var task: Task<Int, Never>
  while let task = task {
    _ = await task.value // might clear out the task property or populate it with a new task
  }
}
3 Likes

I'll speak on behalf of the Core Team here, because "when is a proposal ready for review?" is something we have discussed numerous times. We've been very explicit that the existence of controversy is not a reason to prevent a proposal from coming up for review. The reality is that some changes will be inherently controversial (especially with syntactic sugar), and no amount of pre-review discussion is going to change everyone's mind. The presence of controversy must not halt progress on the language.

When there is controversy in a pitch thread, it's the author's responsibility to capture the alternatives presented in Alternatives Considered with some commentary as to why they consider the proposed design to be the better choice. So while controversy doesn't prevent a proposal from moving into the review phase, unaddressed controversy can prevent it because it means the feedback from the pitch thread hasn't been considered. It's the Review Manager's job to ensure that the major alternatives from the pitch discussions have been captured adequately in the proposal document. Given that most of the debate is a rehash of the same alternatives listed the proposal (if let x? especially), I think the proposal document's treatment of alternatives considered was sufficient for this review.

The job of the Core Team is to decide and guide. We will evaluate the arguments presented here, in the pitch threads, and in the proposal, and make a decision on the proposal. That decision will provide rationale for that decision to aid future discussions.

Doug

26 Likes

Perhaps pot luck, but the if let x = x percentage in my current client's app is almost the same, at 25%. And I suspect there are a handful of usages where we did rename the variable to something else, but if the sugar had existed, we would have used that form instead. And we have hundreds of guard let self = self that could be simplified.

3 Likes

I know the review is technically over, and this topic has been fairly exhaustively discussed, but I’ll point out something I don’t recall seeing mentioned: by dropping the redundant half of guard let longVariableName = longVariableName else { return }, you alleviate the reader of the need to check for subtle differences or extra property accesses on the right hand side. I chose guard specifically for this example because those extra property accesses would be sandwiched between other syntax and thus much harder to visually scan for.

I suspect that if this feature had been in Swift from the beginning, it would be nearly universally adopted when possible, and the expanded if let x = x form would be a clear visual aid for readers to clue them in that something more than optional unwrapping is happening. I think adding that functionality is a justifiable goal of this new syntax, and we might even consider a linter rule or fixit to encourage migration and make up for lost time.

8 Likes

I like that particular bit. If we ever end up changing the language in this area one way or another I believe it would make sense to deprecate / prohibit "... let x = x" spelling if it is achieved by a shorthand spelling to the same effect. Thus any "... let x = x" being left would mean something extra is intended and will bring up attention.

My opposition to this proposal comes from the realisation that if it is approved just for "if/guard let/var x" case - it would make it much harder to pitch and agree on any future generalisations ("let/var x" shorthand that works outside if/guard contexts, explicit shadowing intent markers (e.g. "override let x = 123" and a shortcut version of it "override let x" - the latter in particular clashes with this proposal).

Within the boundaries of the proposal itself I see no harm supporting the very frequent use case if let c = a.b.c <---> if let a.b.c

This is interesting, because in earlier times the core team was explicit that lack of "strong" consensus did bar proposals coming to review. (See, for example, #438.)

9 Likes