[Review] SE-0408: Pack Iteration

There’s potential for it to be more efficient. Actual consequences for any one program are hard to predict.

Would it ever be less efficient?

The efficiency of parameter packs is a super interesting topic, but it's not really related to this evolution review except to the extent that this specific proposal encourages wider parameter pack adoption. Can we move this discussion to another thread if you'd like to dig into it further?

6 Likes

I can’t think of any reason it would, except possibly over-eager specialization causing code-size bloat in places that don’t actually improve performance.

But Holly is right, we should take this to a separate thread.

1 Like

In my limited experimentation with parameter packs I think this will nicely smooth off a rough edge with their use. So that’s a big +1 for me.

I think we should be aware of the current bugs with their use though and ensure we address those as part of any future work on this feature e.g Parameter Packs embedded types and structured concurrency - bug or holding it wrong?

1 Like

In my opinion, the for loop-like syntax is unlike other places where variadic generics can be used, where the repeated value is marked by the each keyword. With the for syntax, the repeated value has to be bound to an iteration variable before use. This has the benefit of making it more obvious what value is being repeated, and feeling intuitively similar to a for loop. But it also has the disadvantage that making one variadic value syntactically "privileged" can become arbitrary. For example, there's no reason why this:

for (left, right) in repeat (each lhs, each rhs) {
    guard left == right else { return false }
}

couldn't be written like this:

repeat {
    let (left, right) = (each lhs, each rhs)
    guard left == right else { return false }
}

Going back to what ensan-hcl said earlier in this thread, the allUnwrapOrNil could be written like this:

func allUnwrapOrNil<each Element>(over element: repeat (each Element)?) -> (repeat each Element)? {
    let result: (repeat each Element)
    repeat {
        guard let element = each element else {
            return nil
        }
        each result = element
    }
    return result
}

In this case, the repeated value (each result) is an lvalue. The for loop-like syntax where repeated values are bound at the top would only allow rvalues.

3 Likes

I agree there's a bit of an issue with regards to for() loop being syntactically sugared like this, and I feel it's a more fundamental issue. With the for-loop syntax I have a hard time understanding how you even assign a value to the correct tuple index while enumerating the types that compose that tuple without some painful mirroring or other thing like that.

Also based on the Future directions section of the proposal, it seems that adding support for applying the iteration over existing swift flow control checks will need to be added individually. I feel like it should be possible to make the syntax less sweet and a bit more specialized so it makes expansion more coherent.

I don't think that the syntax can be as simple as repeat { }, not only is this already used in repeat { } while ; which might render parsing difficult, but given how repeat is used in other context, wouldn't it make more sense to define a pack iteration block like so:

func allUnwrapOrNil<each Element>(over element: (repeat each Element)?) -> (repeat each Element)? {
    let result: (repeat each Element)
    repeat each Element {
        guard let value = each element else {
            return nil
        }
        each result = value
    }
    return result
}

The idea is that you have to specify what parameter pack you are iterating over, then inside the block every call concern one element of that pack, if you're using a tuple whose pack expansion comes from the same type pack then you use the syntax each tupleVariable as the lvalue just like proposed by @ellie20 otherwise you deal with the type as whole.

I'll reuse an example from the first proposal to show how you could get the same result as the print function on variadic tuples with this syntax:

func example<each T: CustomStringConvertible>(packElements value: repeat each T, tuple: (repeat each T)) {
  var repeatEachValueComponents: [String] = []
  var repeatEachTupleComponents: [String] = []
  repeat each T {
      repeatEachValueString.append((each value).description)
      repeatEachTupleString.append((each tuple).description)
  }
  print("(", repeatEachValueComponents.joined(", "), ")")
  print("(", repeatEachTupleComponents.joined(", "), ")")

  var repeatEachValueTupleComponents: [String] = []
  repeat each T {
      var repeatEachComponents: [String] = []
      repeat each T {
          repeatEachComponents.append((each tuple).description)
      }
      repeatEachValueTupleComponents.append("(\(each value), (\(repeatEachComponents.joined(", "))))")
  }
  print("(", repeatEachValueTupleComponents.joined(", "), ")")
  

  var repeatEachValueEachTupleComponents: [String] = []
  repeat each T {
      each repeatEachValueEachTupleComponents.append("(\(value), \(tuple))")
  }
  print("(", repeatEachValueTupleComponents.joined(", "), ")")
}

example(packElements: 1, 2, 3, tuple: (4, 5, 6))

// Prints the following output:
// (1, 2, 3)
// (4, 5, 6)
// ((1, (4, 5, 6)), (2, (4, 5, 6)), (3, (4, 5, 6)))
// ((1, 4), (2, 5), (3, 6))

I think this syntax has the advantage of not adding another sugar on the for keyword, in addition it puts the pack being iterated over at the forefront of the enumeration rather than all the way at the back of the first line, it also circumscribes the whole code where "each x" will represent a single type of the type pack also allowing assigning values to corresponding tuple expansions.

Another advantage I thought about while writing this is that it does not require you to enumerate the values of the pack expansions at the outset. If a developer needs to enumerate various types and that one of the values is expensive to get but another check would remove the need to check for that value, it's beneficial to be able to choose when the value is extracted in one iteration.

Here is another convoluted example:

protocol SomeProtocol {
    associatedtype OtherType
    
    func cheapOperation() -> Bool
    func costlyOperation() -> Element?
}

func performCostlyPackIteration<each Element: SomeProtocol>(packElements values: repeat each Element, otherElements otherValues: repeat each Element) -> (repeat each Element.OtherType?) {
    let result: (repeat each Element.OtherType?)
    repeat each Element {
        if !(each values.cheapOperation()) {
            each result = nil
            continue
        }

        let costlyResult = otherValues.costlyOperation()
        // Do something with the result like use switch { } guards and others.
        each result = costlyResult
    } 
    return result
}

The comment in there does a lot of work but the point is that the complexity could go very far and I'm not sure how this could be represented with the for loop syntax while allowing the user to delay the value extraction or even using the pack expansion syntax without making it unreadable.

Under this proposal and the newly added PR #2153, the implementations of Equatable, Comparable and Hashable would look like this:

extension Tuple: Equatable where repeat each Element: Equatable {
    static func == (lhs: repeat each Element, rhs: repeat each Element) -> Bool {
        repeat each Element {
            if each lhs == each rhs {
                continue
            }
            return false
        }
        return true
    }
}

extension Tuple: Hashable where repeat each Element: Hashable {
    func hash(into hasher: inout Hasher) {
        repeat each Element {
            hasher.combine(each hasher)
        }
    }
}

extension Tuple: Comparable where repeat each Element: Comparable {
    static func < (lhs: repeat each Element, rhs: repeat each Element) -> Bool {
        repeat each Element {
            if each lhs == each rhs {
                continue
            }
            return each lhs < each rhs
        }
    }
}

I hope I conveyed my misgivings properly and please do tell me if I'm completely off.
Thank you for your kind attention.

3 Likes

I think that the proposal is really awesome, and I'd love to have support for pack iteration when using variadic generics.

I have experimented with the supplied toolchain for macOS.

I can get the following to compile:

extension Collection {

    func sorted4<each T: Comparable>(by fs: repeat (Element) -> each T) -> [Element] {
        func isLessThan<each X: Comparable>(fs: repeat (Element) -> each X, a: Element, b: Element) -> Bool {
            for f in repeat each fs {
                if f(a) < f(b) {
                    return true
                } else if f(b) < f(a) {
                    return false
                }
            }
            return false
        }

        return self.sorted(by: { (a: Element, b: Element) -> Bool in
            isLessThan(fs: repeat each fs, a: a, b: b)
        })
    }
}

But I would also expect the following (IMO more readable) version to compile:

extension Collection {
    func sorted5<each T: Comparable>(by fs: repeat (Element) -> each T) -> [Element] {
        return self.sorted(by: { (a: Element, b: Element) -> Bool in
            for f in repeat each fs {
                if f(a) < f(b) {
                    return true
                } else if f(b) < f(a) {
                    return false
                }
            }
            return false
        })
    }
}

The compiler error I get is:

.../VariadicGenerics/VariadicGenerics/Variadics.swift:148:21: error: type of expression is ambiguous without a type annotation
        return self.sorted(by: { (a: Element, b: Element) -> Bool in

I assume that the issue has something to do with the extra scope, but I wonder if the latter example is supposed to work in the final version?

Note also that the version that compiles (sorted4) crashes at runtime on an iOS 17 simulator on the line: if f(a) < f(b) { with Thread 1: EXC_BAD_ACCESS

I do have a version without pack iteration working:

struct LessThan: Error {}
struct GreaterThan: Error {}

extension Collection {
    func sorted3<each T: Comparable>(by f: repeat (Element) -> each T) -> [Element] {

        func isLessThan<S: Comparable>(_ f: (Element) -> S, _ left: Element, _ right: Element) throws {
            if (f(left) < f(right)) {
                throw LessThan()
            } else if (f(left) > f(right)) {
                throw GreaterThan()
            }
        }

        return self.sorted { (a: Element, b: Element) in
            do {
                repeat try isLessThan(each f, a, b)
            } catch is LessThan {
                return true
            } catch {}
            return false
        }
    }
}

@Morten_Bek_Ditlevsen Thank you for catching this! We aim to add pack iteration support in closures in the final version. I've added a note in the PR about this.

2 Likes

@Morten_Bek_Ditlevsen Could you please provide an example of how you ran sorted4? I'm having trouble reproducing the crash.

Certainly - I’m building with the provided macOS toolchain from Xcode. I can package up the project in the morning.

Excellent to hear that closures will be supported too! Thanks for the awesome work on this!

1 Like

Thank you!! I'd love to take a look at the Xcode project🙂

Hi again - here's a link through WeTransfer. Let me know if it's better to share it in some other fashion.

I'm running the project on the iPad Simulator running the latest 17.0 beta - and using the toolchain for macOS shared in this thread.

1 Like

This already means something under SE-0393; it's a pack expansion expression with a closure pattern.

A similar syntax, but using for instead of repeat, was discussed in the pitch thread as sugar for what is presumably the most common use-case for pack iteration: for each element { ... } could be sugar for for element in repeat each element { ... }. It was discussed as sugar for what's proposed here because for each element { ... } or repeat each element { ... } is not sufficient in the general case of binding a name to an element of a pack expansion, because pack expansion patterns are not always just a single pack reference, and this syntax also doesn't support pattern matching within an element of the expansion. There are several examples in the proposal demonstrating this:

for (left, right) in repeat (each lhs, each rhs) {

for x in repeat Generic<each Element>() {

for case .one(let value) in repeat each element {

for value in repeat printAndReturn(each t) {

In my opinion, the most important part of each of the above examples is binding a name to elements of the pack expansion. The repeat each Element suggestion which only names a type pack is more akin to explicitly parameterizing a pack expansion expression with a type pack to expand over. That is a feature I think would be valuable for pack expansion expressions, but I don't see any reason why a loop statement should be privileged with explicit parameterization but not expressions. And I think if we were to go in that direction, we would pick a syntax that makes it more clear that each Element is a parameter to repeat, such as repeat<each Element> f(), and this syntax also makes it possible to express concrete patterns that are repeated N times where N is the length of the each Element pack. To apply this to both expressions and your suggested new repeat statement, there also needs to be some syntactic difference between a repeat statement and a repeat expression with a closure pattern.

In any case, I think this is just a different feature from what's being proposed here. A major part of the goal here is to enable iteration in a way that's more natural to Swift programmers. Inventing a fancy new explicit parameterization syntax for repeat -- though a powerful feature that I think is worth exploring! -- undermines the specific goal of this proposal.

The future direction section mentions guard let. Supporting if let or guard let in the future relies on having local variable packs, which is a separate piece of functionality that would need to be proposed and implemented.

The downside is that you cannot clearly see the value packs that are expanded as part of the iteration; those are easily lost deep within the repeat loop body. I also don't think that a new syntactic form of a for loop is inherently bad. You won't encounter it unless you're inside code using parameter packs, where you will have already encountered the repeat and each syntax. The addition of the repeat keyword in for-in repeat makes it clear that the for loop is iterating over a different kind of value, and the body of the for loop is regular Swift code that's operating over opaque types. Operating over opaque types in the body of the loop is not different than if we were to allow implicit existential opening for for-in loops over collections of existential types (another valuable feature that I think is worth pursuing!).

6 Likes

The downside though is that every single unpacked value has to be listed at the top, which I worry might get cluttered for more complex code. Specifying the pack type that's being unpacked feels, to me at least, like a nice middle ground between being too verbose (explicitly listing each pack variable that's being unpacked) and not being sufficiently clear what pack is being iterated over. In my opinion, there wouldn't be much difference between explicitly specifying the pack type to iterate over, while using the keyword each before each unpacked value, and specifying a single iteration value i and then accessing the different properties of it.

// The collection is explicitly specified here
for i in someCollection() {
    // The `i.` indicates that the value varies with each loop
    let x = i.x 
    let y = i.y
}

// The iterated pack type is explicitly specified here
repeat Element {
    // The `each ` indicates that the value varies with each loop
    let x = each x
    let y = each y
}

Although, there is still the issue where if you want to iterate over different pack types that are related with a same-length requirement, it'd be arbitrary which of the pack types you'd choose.

I also worry that the analogy to a runtime for loop conveys the wrong intuition. Pack expansion comes with a lot more compile-time guarantees, and in some ways would be more powerful; for example, multiple packs can be statically guaranteed to have the same length, so you can use different pack variables all throughout the loop body (in a way that could otherwise be too cluttered if all the different packs were bound at the top of the loop). In other ways, it would be less powerful; for example, if you want to initialize each value of a tuple within a pack iteration (like in the allUnwrapOrNil hypothetical), it would be impossible to break out of the loop. I think the two types of loops might be more easily understood as separate/orthogonal features.

Yeah, that would be nice. If we were to add a repeat Element or repeat each Element syntax (which I'm not entirely sure of myself) then perhaps for consistency it could be repeat Element in f() or something like that, in an analogy to closure syntax.

Can "for expression" be a future direction?

As the length is statically known, I believe it makes a lot of sense to have something like

let sum = for left, right in repeat(each left, each right) { left + right }

If we adopt then statement (Introduce `then` statements by hamishknight · Pull Request #67454 · apple/swift · GitHub) officially, we will be able to write this.

func allUnwrapOrNil<each Element>(over element: repeat (each Element)?) -> (repeat each Element)? {
    let unwrappedValues = for value in repeat each element {
        if let value {
            then value
        }
        return nil
    }
    return unwrappedValues
}

Thank you for such detailed feedback! However, this post is intended to be a place where people can experiment with the feature. The syntax and the direction of this feature were discussed in a [Pitch] Enable pack iteration post.
As per what this proposal suggests, we are aiming to allow Swift users to iterate over packs and explicitly bind an element to a local variable. As @hborla mentioned, one of the important purposes of this proposal (and something that your syntax doesn't support) is to allow pattern matching with an element of the expansion. Also, the proposed syntax follows the progressive disclosure paradigm, where we let users use the familiar for-each syntax for pack iteration. Since you mention that your syntax can be equivalent to the normal for-each with a Sequence, it is debatable to add a new syntax for essentially the same logic as the existing for loop conveys.
Since the feature you are proposing seems to be orthogonal to this proposal, I would encourage you to move this discussion to a separate thread where you can explicitly propose the changes you'd like to make :slightly_smiling_face:

Actually, the length of a pack is not statically known since it conveys generic context. In your sum example, we cannot make any assumptions about the length of a pack.

As for enabling for expressions, I think it is worth considering it as a general future direction, not for this proposal itself.

To be clear, design feedback is definitely still on-topic during the review. If you agree that the feedback is suggesting a better design, you're welcome to make changes to your proposal; similarly, the Language Steering Group might decide that we agree with the feedback and ask you to make those changes. But of course both you and the LSG are also entitled to disagree with the feedback and not make any changes at all.

5 Likes

I'm confused, isn't this particular example the same as let sum = repeat ((each left) + (each right))?

It does support it though, albeit more verbosely:

repeat Element {
    guard case .one(let value) = each element else {
        continue
    }
    ...
}

I was only intending to show how they would have a similar level of clarity with regards to what is being iterated over and what values are updated with each iteration. There are still other important differences in my opinion.

But the syntax feels like a special case that clashes with the rest of the language design. There's no other place in the language where packs and tuples are treated analogously to runtime collections like arrays; even the indexing syntax is different (x.0 vs x[0]).

The value of progressive disclosure, in my opinion, is extending concepts you already know in more advanced ways. In my opinion, the for-each syntax doesn't really expand or generalize to anything, it's kind of just a "dead end" in being a special case. People who use value and type packs would be familiar with the repeat keyword, and the statement form would extend that to code blocks rather than just expressions. The repeat statement would then naturally extend to using unpacked values as lvalues, unpacking different value packs throughout the code block, etc.

1 Like