SE-0231 — Optional iteration

Well, if we had a chance to do it over again, having ? exit the current function monadically (instead of just the current postfix expression) might’ve been a better option, the way Rust does, since it composes more generally, and better tracks what ! does too. Like you said, that’d be a major compatibility break, but maybe having some notion of “idiom brackets” to scope an optional chain expression could fit into the existing language somewhere.

7 Likes

Huge +1 for idiom brackets, especially if it's done in a way that is able to support user-defined applicative types.

Totally agree on "an optional sequence is a sign of ill-considered representation of the underlying data"

I know you meant this well, but it's best not to approach the line of suggesting that other people stop participating in a review, especially when it's apparent that they have strong opinions about the subject.

3 Likes

What is your evaluation of the proposal?

I'm opposed.

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

No.

  • Avoiding a level of indentation is not that important, and when it matters, we have guard.
  • Raw loops are to be discouraged anyway, and algorithms can be optional-chained.
  • As @duan pointed out, most optional sequences should just be empty instead.
  • Optional chaining is an expression-level construct, and I am opposed to extending it to the statement level. At the limit you end up with code that is logically identical to the old Objective-C “plow ahead and let the nils propagate” idiom, but littered with ?s that you're supposed to mostly ignore. One of the reasons Swift doesn't behave that way is because that kind of code is hard to reason about. Explicit control flow and error unwinding are better ways to manage nils.

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

I don't think so. The extension of optional chaining beyond the expression level is precedent-breaking, and this strikes me as a very minor syntactic convenience to be adding at a stage where there are still many fundamental issues of design and correctness to work out.

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

The analogy to Objective-C's pervasive nil propagation is hard to ignore ;-)

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

Read through the proposal and skimmed the review thread.

11 Likes

Thanks for so thoroughly investigating your own codebase for this! As review manager, I would be interested in seeing more hard evidence of this sort from anyone else willing to spend some time mining their own private Swift codebases (I can look at Github well enough myself, though in our experience, open-source repos have not always been an accurate representation of all code). If you (or others) have time to refine your methodology and get better results, I'd also appreciate that.

One concern that comes to my mind with this sort of approach, though, is whether the existing language design, by imposing this impediment, forces developers to code around it. By not providing a direct means to iterate over optional collections, a developer could be pushed to change their coding approach more drastically in an attempt to avoid having to do so, perhaps by handling optionals more eagerly elsewhere. One could argue that's a good thing, but it might also obscure how much of an immediate frustration this really is if you try to divine it from the code. You mention anecdotally that it hasn't been a problem for yourself; do you happen to know whether your coworkers feel the same?



At a meta-level, I feel like we've had a good and thorough discussion of both whether iterating sequences wrapped in Optional is an important problem to solve, and many approaches to how to solve it if so. I'll try to summarize, and please reply if I've left your position out:

  • We could take the proposed for? syntax as is
  • We could use some other special case syntax, such as for x in? optional
  • We could include for loops in optional chains, so that for x in optional?.sequence works. Some people, including the proposal as an alternative, have suggested "put a ? after the sequence", for x in optional?, which can be seen as the degenerate case of optional-chaining the for loop. This has also led to discussion about whether optional chaining ought to be generalized in more ways, such as to arguments, binary operators, or other statement forms, particularly if optionalBool?.
  • We could allow for loops to accept multiple statement conditions, in the manner of if and while, using the final in form as the sequence to be iterated, for let xs = optional, x in xs
  • We could make for loops implicitly accept Optional with a Wrapped sequence.
  • We could leave the language as is, and make a library change to address this problem:
    • Introducing an orEmpty method on Sequence that produces a wrapper
    • Making Optional conform to Sequence when it has a Wrapped sequence
    • Continue to promote x ?? [] (or equivalent empty literal syntax) as the idiom for handling optionals containing sequences

Note that by listing these out, I'm trying only to summarize and confirm that these positions have been heard, not implying anything about the merits of any particular solution, or whether making any change is the desired outcome of this review. Personally, I'd like to see deeper analysis of the "Is this problem important to solve" angle, and doing more data analysis like what @algal has provided would be very interesting.

19 Likes

There's one option that hasn't been actually proposed, but imho it's worth being considered as well:

3 Likes

Thanks! I added it to the post.

As another potential library change that could help address this issue, and would be more closely targeted than making Optional<Sequence> conform to Sequence, we could add a special overload for ??:

struct EmptySequence<Element> : Sequence, ExpressibleByArrayLiteral {
  typealias ArrayLiteralElement = Element

  public init(arrayLiteral elements: ArrayLiteralElement...) {
    guard elements.isEmpty else { abort() }
  }

  struct Iterator : IteratorProtocol {
    public mutating func next() -> Element? { return nil }
  }
  public func makeIterator() -> Iterator { return Iterator() }
}

func ??<S>(lhs: S?, rhs: EmptySequence<S.Element>) -> AnySequence<S.Element> where S : Sequence
{
  switch lhs {
  case .some(let s):
    return AnySequence(s)
  case .none:
    return AnySequence(rhs)
  }
}

With this addition, the existing ?? [] idiom would work in a for loop for every type of optional Sequence.

4 Likes

(Edit to add: looks like you also need

func ??<S>(lhs: S?, rhs: S) -> S where S : Sequence, S: ExpressibleByArrayLiteral {
  switch lhs {
  case .some(let s):
    return s
  case .none:
    return rhs
  }
}

In order to make sure existing sequences that are expressible by [] aren't accidentally wrapped in an AnySequence.)

I did a quick check of the compatibility suite. It was a simple regex + eyeball so I think it avoids false positives but will probably be off +- a few.

I could see 20 examples of for with ?? [] and 55 examples of if let immediately followed by use in a for. Overall 24 projects used one of these patterns, so just over a quarter of the projects.

edit: adding in ?.forEach, of which there are 15 occurrences.

Anecdotally, this is a pretty high occurrence rate based on searches for similar patterns for other reviews I've done in the past.

13 Likes

Not sure if you looked at this: did the if let cases have any else condition?

I tried to spot and exclude those. Also there were quite a few cases to exclude where the let was used as an argument to the sequence-producing function rather than the iteree itself, which I also excluded, though strictly speaking, if we had an optional for syntax, that would allow map to be used in those cases.

1 Like

In our codebase (about 100k lines of Swift mixed in with about 1M lines of ObjC) there are 13 matches for @algal's for _ in _ ?? [] regex. 6 of them involve optional chaining, and all of those are related to an optional NSView/UIView (e.g. for subview in superview?.subviews ?? []).

Our in-house Swift style is generally to guard exclude optionals as soon as is practical and avoid dealing with them. I even notice that there are at least 5 places where it might have been natural to use optional chaining with sequences (outside of a for) and I see constructions like (getOptionalSequence() ?? []).map({...}).filter({...}) instead.

5 Likes

Using @algal 's strategy:

Our codebase has ~70,000 lines of swift code, we match 11 times for the regex looking for if let statements followed by for loops. 2 is the result of poor representation of data, resulting in conditional casting, the rest could have used ?.forEach or ?? [] (though using the ?.forEach on some of those would involve unfortunate parenthesis).

5 Likes

I have 56 instance of for x in sequenceExpression ?? [] in my code base, roughly 10% of all the for loops. Half of those come from XML parsing code (where you must always worry about parts of the structure being missing). Things like this:

for noteElement in unitElement.first("notes")?.all("note") ?? [] {
    // do something with the note
}

Currently the filters for XML elements of interest all return eagerly constructed arrays, so using sequenceExpression ?? [] works nicely. It'd be better to return lazy sequences, but then all the loops would need to be wrapped like this:

if let notesElement = unitElement.first("notes") {
    for noteElement in notesElement.all("note") {
        // do something with the note
    } 
}

Also, trying to mix ?? [] with lazy sequences could trigger the the trap unearthed by @Ben_Cohen, which could make the algorithm eager again without notice. That's a concern, although it's not anything specific to for loops.

Edit: code base is about 60k lines of Swift code.

4 Likes

Using @algal 's strategy:

LOC: 47,057

Matches: 3 [All of them use an else case].

Pods matches: 9 [5 of them use an else case].

Total: 12 matches.
8 of them using else case.

Note: I don't think the change it's worth it.

1 Like

Also, some overlays and source kit:

My memory is that I reach for this pattern frequently, but I don't currently have any code on-hand that does this.

3 Likes

In 255948 lines of just Swift code, excluding empty lines and comments (collected by cloc):

Occurrence of @algal's regex

if[:space:]*let[:space:]*\w+[:space:]*=[:space:]*.*[:space:]*\{[:space:]*for

5

Occurrence of for .+ in .+ ?? \[\]: 14

In a 500k+ line codebase I have access to I see 210 results for this regex. A lot of this code was written by people not familiar with Swift idioms so probably half of those fall into the stored optional array anti-pattern, however many of them are the result of optional chains and other reasonable code patterns.