SE-0231 — Optional iteration

First of all, that code would be better written as:

guard let sequence = sequence else {
    return
}

for element in sequence { }

Secondly, that's an insufficient solution in comparison to the proposed solution. It doesn't let you "carry on" to do stuff past the for loop, like:

alwaysCallMe()
for x in dontIterateMeIfImNil? {}
alwaysCallMe()
3 Likes

The proposal:

Coalescing with ?? [] is only valid with types that conform to ExpressibleByArrayLiteral .

davedelong:

return EmptyCollection() // yes I know this won't work; but you get the point

Could we add the following to Sequence and solve all this?

static var empty: Self { get }

Then you could do:

for x in (optionalSequence ?? .empty) {
...
}

I disagree with the proposal as written. Could maybe see in?. However, I don't think I'm alone in preferring forEach over the for-in construct.

It wouldn't work for CollectionOfOne and nonempty collections, unless you just fatalErrored in the getter

5 Likes

The whole point of my post was to argue why shadowing local variables are less readable than being able to unwrap the original optional value.

And that is the use case which I argue is a reason to support the proposal.

What is your evaluation of the proposal?

-1.

The problem this solves has not been a significant or noticeable problem in my own experience with Swift.

To double-check my intuition on this, I did a quick check of our Swift codebase at work, to see how many places there are where this language change would be useful.

To estimate the size of our codebase, I executed this command in the shell:

find . -name '*swift' | xargs wc -l

It turns out we have about 110,000 lines of Swift code (including comments and external libraries).

To determine how many obvious places there are where we could benefit from this syntax sugar, I went to Xcode and did Find / Regular Expression on our Workspace, and entered the following regular expression:

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

The search returned only one result. This regular expression is imperfect, since it will not match if let forms that bind multiple variables on separate lines. Maybe that will produce more hits?

This one is more inclusive but will also produce false positives:

if[:space:]*let[:space:]*\w+[:space:]*=[:space:]*(?s:.)*[:space:]*\{[:space:]*for

Inspecting the 8 hits from this one, I still found only one instance, the original one, where this new language syntax would be helpful.

I am a bit surprised by this low number but it is consistent with my personal experience: I've never felt this to be a problem.

Maybe I messed up the regular expression? If so, I'd love to re-try the search if someone can suggest a better one, or a show how to do a smart, syntactically-aware search.

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

No.

In fact, I worry this solution is worse than the problem. It introduces a subtle syntactic change, making the language more complex and less explicit, in order to remove what has been (in my experience) a small and infrequent piece of boilerplate.

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

No. As Matt Gallagher pointed out, the closest precedents are the as? and try? keywords, both of which introduce an optional, rather than affecting the interpretation of an optional.

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?

I read this entire thread, and searched the largest Swift codebase I know personally in order to estimate how often this language change would be useful.

11 Likes

Just take a look at the alternatives that have been suggested, and if you are really engaged, try to build a regex for all of those ;-)
The if let solution may be superior in terms of speed and transparency, but it's tedious to write, and speed doesn't matter for many developers. ?? [] is much more easy to type when you learned the pattern, and it seems to be much more popular.
FP fans might not even consider anything but forEach or reduce, but you can't see wether someone used that method because he doesn't like for in general, or because of the Optional...

Good idea. I tried this regular expression:

for[:space:]+\w+[:space:]+in[:space:]+.*\?\? \[\][:space:]\{

It will match stretches of code of the form: for something in something ?? [] {:

This also produced one hit in the codebase I'm looking at. (For what it's worth, it was the line for vc in self.window?.rootViewController?.allPresented.reversed() ?? [] {.)

I can't say why but this just doesn't seem to be a problematic idiom in our codebase (and dependencies).

2 Likes

I have the same concern , both for this and try? [SE-0230]

1 Like

That looks rather exotic indeed ;-)
But it's really hard to catch all Sequence?-loops:
I've just searched for "?? " and found many hits in an (old) Dropbox-SDK - but they used another variation:

    let urlTypes = NSBundle.mainBundle().objectForInfoDictionaryKey("CFBundleURLTypes") as? [ [String: AnyObject] ] ?? []
    for urlType in urlTypes {

It's always surprising how diverse people are:
I'm one of the (as it seems) very few with reservations against SE-0230, but I strongly support "sugar" for optional iteration... I don't care much about how many lines of C++ or Swift are added to the compiler or the stdlib, and I think this can make the language itself actually less complex. There are so many places where you can change a variable from T to T? by just adding some question marks - but for-loops have to be changed dramatically.
Consistent rules like "if you are sure use !, and if you are not so sure, try ?" can help a lot.

2 Likes

Hi Chris,

That is true, even though the sequence? approach is a far less disruptive change than for?. I might be missing something, but since the "bar" argument has come into play, I would like to note: if the following from the proposal template were true, how many implemented purely additive proposals would have had to be rejected? I am by no means questioning those decisions. My point is that apparently, there are cases when the strictness of said rules may be lowered. If that is true, I don't believe the alternative solution of this proposal is one that deserves to simply be put before the fact of the bar for syntactic additions.

Features that don't change the existing ABI are considered out of scope for Swift 4 stage 1.

The Standard Library's main sequence types do indeed conform to one the ExpressibleByLiteral protocols. But this isn't true for lots of other sequence types that are designed as wrappers and subsequences and often come up when calling different APIs, lazy for instance. The Range types count as well. The main goal of this proposal is to introduce a simplified general approach that besides everything else wouldn't require using ?? "", ?? [], ?? [:] or figuring out which literal or "empty"() API you need. Generic contexts are a good example of when coalescing with an empty instance isn't always applicable.

I wasn't trying to make an argument against the forEach method itself. This is why I am referring to the differences to show that forEach isn't a valid alternative if you are using control transfer keywords. I don't want people to get the wrong idea though, do you think the argument should be rephrased?

Thanks for taking a look. I intentionally delayed the clean up because of the alternative solution. Regarding technical problems, there are dirty workarounds for some implementation details I currently don't know how to deal with. The diagnostics part, in particular, is mentioned in the PR. The alternative solution though would most likely resolve all these hitches.

1 Like

This introduces a very interesting addition data point I had missed: ?? [] is a significant performance and correctness trap.

Not because [] creates an array unnecessarily (it doesn't, the empty array is a static singleton in the standard library via a performance hack that gives me heartburn).

It's because when the array isn't nil, the presence of ?? [] affects the type checker in ways you don't expect:

let maybeHugeRange: Range<Int>? = 0..<Int.max

if let hugeRange = maybeHugeRange {
  for i in hugeRange.reversed() {  // fine
    print(i) // prints Int.max
    break    // and then breaks
  }
}

for i in maybeHugeRange?.reversed() ?? [] { // doom occurs here
  print(i) // your app will jetsam without ever reaching here
  break
}

What's happening here? Well, Range is a collection that cannot be created via []. It's a very low-cost collection: it just stores the upper and lower bound, and iteration is just incrementing from one to the other. On bidirectional collections, reversed() just returns a lightweight wrapper over the base collection, that forwards calls on to the base collection but reversing the direction of traversal. So in this case, a ReversedCollection<Range<Int>>, just 16 bytes in size. It is also not creatable via []. All nice and efficient.

So what does maybeHugeRange?.reversed() ?? [] do? The ReversedCollection answer won't type check, because the rhs of ?? can't be one. So instead it falls back to the version on forward-only collections. That returns an array. So now, just because of that ?? [], we are attempting to allocate and fill an array of size Int.max. Which blows up.

Obviously this is an extreme example that crashes. But in a more likely case, if the optional reversed collection is only of modest size but rarely nil, you could be causing huge numbers of array allocations without realizing it.

21 Likes

Just in the standard library, there are numerous sequences that aren't expressible by [], [:] or "":

  • The various range types
  • All the lazy wrappers
  • All the type-erasing wrappers
  • Reversed collections
  • Repeating, Empty and -OfOne collections
  • Unfolding sequences
  • The string views
  • Default indices
  • Zipped sequences
  • Enumerated sequences

Some of these are exotic. Many of them are extremely common. Often users are using them without realizing it.

Plus, as Anthony points out, not even Array works in a generic context without further constraining to the literal protocol.

And as I mentioned above, we want more of these types. Expressing your API via a collection view is a very powerful technique and one that does not compose well with the ?? [] idiom.

13 Likes

Anyone who wants to be able to do this:

for e in someOptionalSequence.? {
    print(e)
}

can already do so (for any sequence, including CollectionOfOne), using eg this:

struct FlattenedOptionalSequence<Base: Sequence>: IteratorProtocol, Sequence {
    let baseNext: () -> Base.Element?
    init(_ optionalSequence: Optional<Base>) {
        switch optionalSequence {
        case .none: baseNext = { return nil }
        case .some(let s):
            var baseIterator = s.makeIterator()
            baseNext = { return baseIterator.next() }
        }
    }
    func makeIterator() -> FlattenedOptionalSequence<Base> { return self }
    mutating func next() -> Base.Element? { return baseNext() }
}
extension Optional where Wrapped: Sequence {
    func flattenedOptionalSequence() -> FlattenedOptionalSequence<Wrapped> {
        return FlattenedOptionalSequence(self)
    }
}

postfix operator .?

postfix func .?<S: Sequence>(lhs: Optional<S>) -> FlattenedOptionalSequence<S> {
    return FlattenedOptionalSequence(lhs)
}

It will also work for @Ben_Cohen's above example:

let maybeHugeRange: Range<Int>? = 0..<Int.max

for i in (maybeHugeRange?.reversed()).? {
    print(i) // Will print 9223372036854775806 and break.
    break
}

Although unfortunately, because of the way optional chaining works, we have to add those parens around everything before the postfix .? operator, since that's the only way to refer to the optional created by optional chaining rather than the non-optional "in-chain value". We could change .? into a prefix operator and get rid of the parens that way:

for i in .?maybeHugeRange?.reversed() { ... }

I don't know if the baseNext closure would impact performance negatively or if there would be a way to work around that.


EDIT: There's also the possibility to

make any optional sequence a sequence
public struct FlattenedOptionalSequence<Base: Sequence>: IteratorProtocol, Sequence {
    let baseNext: () -> Base.Element?
    init(_ optionalSequence: Optional<Base>) {
        switch optionalSequence {
        case .none: baseNext = { return nil }
        case .some(let s):
            var baseIterator = s.makeIterator()
            baseNext = { return baseIterator.next() }
        }
    }
    public func makeIterator() -> FlattenedOptionalSequence<Base> { return self }
    public mutating func next() -> Base.Element? { return baseNext() }
}

extension Optional: Sequence where Wrapped: Sequence {
    public typealias Element = Wrapped.Element
    public typealias Iterator = FlattenedOptionalSequence<Wrapped>

    public func makeIterator() -> FlattenedOptionalSequence<Wrapped> {
        return FlattenedOptionalSequence(self)
    }
}

so that for-in will "just work" with any number of optional layers around any sequence:

let oooa: [Int]??? = [1, 2, 3]
for e in oooa { print(e) } // Prints 1 2 3

let maybeHugeRange: Range<Int>? = 0..<Int.max
for i in maybeHugeRange?.reversed() {
    print(i) // Will print 9223372036854775806 and break.
    break
}
2 Likes

Imho this is not only interesting because of crashes or inefficient code - it shows how complicated the status quo actually is, which forces developers to think about solving a trivial task that could have an obvious solution:
If you have something common like a plain Array, ?? [] may be just fine, and foreach handles many cases as well - but the next Sequence? can be completely different, and require a very different tool to perform the exact same job.

There have been many people claiming that a change is superfluous because we have ?? [], yet we have just learned about a flaw that afaik no one has thought about before... wouldn't it be nicer if we had an answer to the question how to deal with Sequence? that just works perfectly in every situation?

8 Likes

Then why isn't eg the following an example of silent handling of nil?

let a: Bool? = nil
// Some lines of code later, the programmer has slipped into thinking about `a`
// as being just a `Bool`, not an `Optional<Bool>`, and writes:
if a == false {
    print("Does something assuming `a` is false")
} else {
    print("Does something assuming `a` is true (which is a mistake)")
}

I wouldn't mind if that == would have to be ==? for that code to compile.

Attempting to cover every sharp edge in bubble wrap is a self defeating goal, plus it only introduces more inconsistency.

1 Like

There is a problem with for e in optionalSequence? that imo is quite serious:

It would require parenthesizing an optional chain, at least if it aims to be symmetric/complementary to !.


In order to make the following program compile:

let optionalSequence: [Int]? = [1, 2, 3]
for e in optionalSequence { print(e) }

The assumed solution would be:

for e in optionalSequence? { print(e) }

complementing the currently working !-solution:

for e in optionalSequence! { print(e) }

And, this:

for e in optionalSequence?.reversed()? { print(e) }

would also work, just like:

for e in optionalSequence?.reversed()! { print(e) }

BUT, the above force unwrapping won't compile. Since, exactly like my ".?-postfix-operator-solution" above, we have to parenthesize the entire optional chain in order to get at the optional value of the entire expression (rather than the non-optional value of the last property):

for e in (optionalSequence?.reversed())! { print(e) }

So I guess a solution with the question mark at the end should also require parenthesizing the optional chain, or should we allow ? and ! to be inconsistent/asymmetric here?

1 Like

Here's a better way to express this force unwrap:

for e in optionalSequence!.reversed() { print(e) }

The better way to make the loop optional would therefore be this:

for e in optionalSequence?.reversed() { print(e) }

Similar to using a forEach:

optionalSequence?.reversed().forEach { print($0) }

Note there's no ? in front of the forEach in this case. The first ? short-circuits the rest of the chain if nil. It could allow short-circuiting the for loop too.

4 Likes

Can we fix the precedence of optional chaining so that postfix operators work properly?

The Equatable conformance of Optional<Bool> is certainly walking a fine line in implicit handling of nil. But it's also not true that it is completely silent. You must write a == false. You cannot just write if a { }. Personally, I feel like that's sufficient acknowledgement of the optionality, without a new operator (which would be problematic, given Equatable conformance – a really valuable feature now we have it – requires defining an == operator).

edit: if !a { }, I guess I mean

3 Likes