[Review] Remove C-style for-loops with conditions and incrementers


(Douglas Gregor) #1

Hello Swift community,

The review of "Remove C-style for-loops with conditions and incrementers” begins now and runs through Thursday, December 10th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

  Cheers,
  Doug Gregor
  Review Manager


(Paul Cantrell) #2

Review of SE-0007

* What is your evaluation of the proposal?

This is a sensible change. I am in favor of it.

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

In isolation, the problem appears too small to warrant a change — just a little cruft in the language, an unnecessary but mostly harmless remnant of Swift’s ancestry. The C-style for loop is not a common source of programmer error, and is easy for the compiler to implement.

However, in aggregate, cruft like this leads to language bloat and stagnation. In a garden, a single weed is not a problem, but never weeding is a significant problem.

We have a feature that is:

1. not widely used,
2. easily replaced with other languages features where it is used, and
3. inconsistent with the language’s general aesthetic.

Observation, theory, and taste agree. If this isn’t a feature to remove for the health of the language, then what is? If we do not pull this weed, then what weed would we ever pull?

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

Yes.

1. Swift tends to favor concise language constructs over boilerplate code for common patterns (e.g. didSet, lazy, optional unwrapping conveniences). This makes the C-style for out of place for the two common cases, counting and collection iteration, both of which already have more Swift-like alternatives.

2. The ++ and — operators are already slated for removal, which makes the C-style for much less compelling.

3. Swift tends to encourage immutability. It’s a minor detail, but the fact that the C-style for rules out immutability for the index variable is particularly compelling to me:

    for i in 0..<16 // i is immutable by default

    for let i = 0; i < 16; i++ // doesn't work

4. The few cases raised on the list where C-style for might still be preferable over other loop constructs all involve subtle interaction between the iterator clause (third part of the “for”) and break/continue/return. When I’ve seen this kind of code in the wild, at least in my own experience, it (1) usually is brittle and in need of rewriting, and (2) tends to appear only in low-level system libraries and graphics-related bit-twiddling, neither of which seem like Swift’s areas of focus. (Swift’s favoring of explicit scalar type conversion and precision safety already make it ill-suited to low-level bit twiddling.)

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

Ruby does not have a C-style for loop. I’ve used Ruby extensively since 2007, and have not once felt that I wanted a C-style for.

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

An hour or two. I’ve followed the discussion on the mailing list. I did a programmatic search of all of my own Swift code; I’ve never used the C-style for in Swift.

···

On Dec 7, 2015, at 2:44 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "Remove C-style for-loops with conditions and incrementers” begins now and runs through Thursday, December 10th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

  Cheers,
  Doug Gregor
  Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Stephen Canon) #3

What is your evaluation of the proposal?

Mild concern. I am not opposed to removing C-style for loops, but I do think that it leaves a hole in the language that should be filled by some other means, and I would prefer to figure that out before removing a useful feature.

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

Not obviously so. C-style for loops, while not particularly stylish, are “mostly harmless”. That said, some of their functionality is duplicated in for … in loops, so there is some overlap that might be cleaned up. Having a simpler language (if we can do so without compromising usability) is a good thing.

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

For … in loops are more Swifty, and we should encourage people to use them where appropriate. However, they are significantly less versatile than C-style for loops. My concerns, as a low-level library writer, are that:

- The loss of C-style for makes iterating backwards more complex than iterating forwards. This asymmetry is unnatural.

- The loss of C-style for makes it significantly more complex to use strides that vary between iteration steps.

In particular, the available replacements for the second issue force one to split the “loop control” over multiple lines in e.g. a while loop:

  var i = initialValue
  // possible intermediate stuff
  while i < terminalValue {
    // some loop contents
    i += update
  }

I have seen some proposals floated involving closures, but I don’t think any of these to pass the smell test. One of the chief virtues of the C-style loop is that it lets one gather all of the loop control in one place, which makes reading complex code much, much easier (if we keep C-style for, I would strongly support something like "for (let i = blah; … )” where i is immutable within the loop body for exactly this reason).

My biggest concern (the asymmetry of loops with stride -1) could be addressed by having a reasonable range operator for this case ("N >.. 0" or similar). I haven’t seen a really strong proposal yet for other loop structures, but I’m willing to write them off if we have a good story for -1 stride, which is the 99% case in my experience.

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

Every language I’ve used heavily either has a "c-style-for” structure, or is so utterly foreign to Swift as to offer no practical guidance here.

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

I’ve read all of the emails carefully, and spent a while looking for cases in my own code where I would really miss the current feature.

Thanks to everyone for taking time to contribute to this!
– Steve


(Michel Fortin) #4

Review for SE-0007 Remove C-style for-loops with conditions and incrementers

https://github.com/apple/swift-evolution/blob/6e121f714bcbade5860aa74bdc3523a14a9cf0b8/proposals/0007-remove-c-style-for-loops.md

# What is your evaluation of the proposal?

Even though I do not dislike the idea, I think the disappearance of the C-style for loop should at least be accompanied with decent migration measures. Nothing very convincing has been proposed in that regard. So I disagree with the proposal as it stands today.

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

One of the argument is that this loop is rarely used. I tend to disagree: this loop exists in many other mainstream languages: code using it can be found everywhere. What would be a simple transliteration of an algorithm to port it to Swift can easily become a mess if you have to refactor the loop at the same time. This makes the delicate process more painful and error-prone.

Even if in theory it would make sense to refactor all C-style for loops with for-in or while loops, this is not always easy, or convenient, and most importantly it does not always result in more readable code.

I do agree the syntax is not great. It's not very intuitive and the readability is poor. It should ideally be replaced by something better. But using for-in and while, as they exist currently, does not cut it.

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

I would really like if the C-style for loop could be replaced with something more intuitive. And if it did, I would expect Swift to provide a migration path to remove C-style for loops and replace them by something equivalent. What kind of code is a migration tool going to generate for this proposal? This isn't addressed very clearly, and I suspect the results won't be so pretty.

You can't replace C-style for loops with for-in in the general case. Using a "while" loop will always be possible, but also much more verbose. For instance, take this loop iterating downward over i and j in lockstep:

  for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
    ... loop body ...
  }

The only fully-compatible while loop that makes sense is this one:

  do {
    var i = 10
    var j = 20
    var first = true
    while true {
      if first { first = false }
      else { i -= 1; j -= 2 }
      guard i > 0 && j > 0 else { break }
      ... loop body ...
    }
  }

If the loop body contains a continue, it works. If the loop body mutates one of the loop variables, it works. If the loop throws, it works. If the outer scope has a variable with the same name, it works! There is no code duplication. But oh boy, the boilerplate!

Not all loops contain a continue, or need to mutate its loop variable, or throw. There is an abundance of simpler patterns that can apply to these particular cases. But any of these patterns based on a while loop are more fragile than a C-style for loop. Introduce one continue, or one throw, and the loop needs some refactoring to correctly maintain the loop state. The only robust loop pattern is the for-in loop, but its applicability is limited, especially when applied by an automated tool.

Therefore, to keep the same semantics, any migration tool should use as a replacement either a for-in (for simple forward iteration, the most common case), or the above while loop. Needless to say, nobody is going to be happy seeing a C-style for loop replaced by a while loop like above. There needs to be a better way to express that loop.

Don't forget that migration in this case is not only a Swift 2 to Swift 3 thing, it's also a foreign language to Swift 3 thing.

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

When I was learning programming, I used Pascal a lot. There was no equivalent to the C-style for loop. (There was no break, continue, or return either.) There was a for loop to iterate over a range of numbers, up or down, but sometime that for loop was not enough. So I learned to use variables to control the state of a while loop and express the condition for exiting the loop in term of those variables. The logic for that had to be scattered all across the loop body. This is what the equivalent while loop reminds me of.

The for loop from C is a power tool: it lets you express the loop control variables, the exit condition, and the prepare-the-next-iteration statement all together in one place, cleaning the loop body from the bookkeeping work, and properly handling special cases such as "continue". It's a useful middle ground between the strongly abstracted for-in loop and the almost unstructured "while" loop.

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

I read most messages on the mailing list, made a few proposals, found out a a couple of alternatives and figured out their limitations. I also remembered an experience with someone I worked with, who was better at math than at coding, and for whom a C-style for loop with all the inner working exposed would feel much more reassuring than a clever abstracted sequence type in a for-in loop.

···

--
Michel Fortin
michel.fortin@michelf.ca
https://michelf.ca


(David Owens II) #5

Review of SE-0007

* What is your evaluation of the proposal?

I am against this change. I’ve seen no consideration of the performance implications that this change brings about. Also, many of the proposed alternatives tend towards using defer as the mechanism to provide consistent increments/decrements. However, this is problematic around the boundary edges as break and throw are not mechanisms that can halt the execution of the defer statement. So unlike the c-style loop that would not perform that code, the alternatives would.

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

No; it offers no alternative solution to maintain the performance that a c-style for-loop ensures. There is no general pattern than can be used to satisfy a c-style loop’s behavior; each proposed alternative has different issues.

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

No, it limits the language’s ability to create concise and legible code when the situation does arise for a c-style loop.

This is the alternative to create a loop that matches the same functionality, including preventing the leaking of loop-variables.

var sum = 0
for var i = 1000, j = 2000; i > 0 && j > 0; i -= 1, j -= 2 {
    if i % 2 == 0 { continue }
    sum += 1
}
print(sum)

// versus

var sum = 0

if true { // some arbitrary scope mechanism is needed to contain i and j
    var i = 1000, j = 2000
    while i > 0 && j > 0 {
        defer {
            i -= 1
            j -= 2
        }
        
        if i % 2 == 0 { continue }
        sum += 1
    }
}

print(sum)

To me, that is not a fair trade-off; maybe if the defer pattern could be used at all times and the leaking of the loop-variables was considered ok. There are some ways to consolidate the code so that it takes up less visual space, but I don’t do that for other constructs, so I don’t think that is a valid thing to try and do here. Also, the use of defer in this context is a somewhat of a trick that I’m not sure is even a good pattern to promote as it can lead to boundary-case crashes.

This is unsafe code:

var sum = 0
var i: UInt = 1000
while i >= 0 {
    defer {
        i -= 1
    }
    if i == 0 {
        print("zero has been reached!")
        break
    }

    if i % 2 == 0 { continue }
    sum += 1
}

The defer statement is still executed, so now I need to come up with a different pattern to handle this type of problem.

* If you have you 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?

A few hours. I’ve read the proposal multiple times and followed the mail threads. I’ve also done some basic performance analysis (attached mail) on the problem for some of the “improved” versions of the loop construct.

-David

SE-0007 Remove C-style for-loops with conditions and incrementers - Performance Consideration.eml (27.2 KB)

···

On Dec 7, 2015, at 12:44 PM, Douglas Gregor <dgregor@apple.com> wrote:

Hello Swift community,

The review of "Remove C-style for-loops with conditions and incrementers” begins now and runs through Thursday, December 10th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

  Cheers,
  Doug Gregor
  Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce


(David Waite) #6

Review of SE-0007

* What is your evaluation of the proposal?

I can understand how this change is controversial - however, I’m in favor of it.

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

Many examples have been given on the list of for loops that look more complex or involve additional nesting once C-style for loops are eliminated. However, I couldn’t help thinking many of these examples are greatly complex in themselves - that using a functional syntax (even if it makes the line longer or has the sequence building moved to another function) serves to make the code intrinsically more readable and maintainable.

For example,

  for i in (0..<things.length).reverse() { … } is longer/more verbose, but is also very clear in purpose

For someone coming from C, for loop support may help them port their code quicker. However, for someone without C experience, the syntax and rules are different than the rest of the language (for instance, the for loop is the only construct which requires a semicolon for expression).

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

Yes.

With ++ and -- operators slated for removal, the main motivator for having for loop support (ease of porting C-dialect code) is greatly reduced.

There is a push to use functional concepts in Swift (avoiding terms like Monoids and Monads since the theory isn’t needed for usage). Every for loop I’ve ever seen is non-functional, and the availability of the feature pushes people toward imperative thinking and coding.

** Additional feedback on evaluation

I *wish* there was greater consistency with blocks and closures, so that for - in … {…} wasn’t required in addition to .forEach {…}.

Some of the concepts given seem like they may warrant language use guidance (such as in TSPL book):

  for i in 0..<things.length { … }

should perhaps instead be

  for i in things.indices { … }

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

The main language I’ve used without for loops is ruby. I honestly didn’t realize they were missing until another review stated so - once I moved away from using them, I’ve never been motivated to go back.

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

I’ve followed the mailing list discussion and looked at my own code.

-DW


(Douglas Gregor) #7

Thanks to everyone for participating in this review!

  - Doug

···

On Dec 7, 2015, at 12:44 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "Remove C-style for-loops with conditions and incrementers” begins now and runs through Thursday, December 10th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

  Cheers,
  Doug Gregor
  Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


#8

Can't you do the following?

    var i = 10, j = 20; while i > 0 && j > 0 { defer { i -= 1; j -= 2 }
        // ...
    }

I adjusted the formatting to more closely match the C-style for loop, so if
you were to use a more Swift-y style, it would be slightly more verbose.

Stephen

···

On Thu, Dec 10, 2015 at 12:16 PM, Michel Fortin via swift-evolution < swift-evolution@swift.org> wrote:

You can't replace C-style for loops with for-in in the general case. Using
a "while" loop will always be possible, but also much more verbose. For
instance, take this loop iterating downward over i and j in lockstep:

        for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
                ... loop body ...
        }

The only fully-compatible while loop that makes sense is this one:

        do {
                var i = 10
                var j = 20
                var first = true
                while true {
                        if first { first = false }
                        else { i -= 1; j -= 2 }
                        guard i > 0 && j > 0 else { break }
                        ... loop body ...
                }
        }

If the loop body contains a continue, it works. If the loop body mutates
one of the loop variables, it works. If the loop throws, it works. If the
outer scope has a variable with the same name, it works! There is no code
duplication. But oh boy, the boilerplate!


(thorsten@portableinnovations.de) #9

Nice review!

You can't replace C-style for loops with for-in in the general case. Using a "while" loop will always be possible, but also much more verbose. For instance, take this loop iterating downward over i and j in lockstep:

  for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
    ... loop body ...
  }

The only fully-compatible while loop that makes sense is this one:

  do {
    var i = 10
    var j = 20
    var first = true
    while true {
      if first { first = false }
      else { i -= 1; j -= 2 }
      guard i > 0 && j > 0 else { break }
      ... loop body ...
    }
  }

That is indeed quite ugly because of (a) the need to check for the first iteration in case of continue and (b) having the condition as guard within the loop and not as while-condition.

Another alternative using a while-loop might be extracting the loop body into a function (still not very nice but at least without checking for the first iteration and having the looping condition in its proper place):

do {
    func body(i: Int, _ j: Int) {
        if i % 2 == 0 { return } // simulating continue
        print (i, j)
    }
    var i = 10
    var j = 20
    while (i > 0 && j > 0) {
        body(i, j)
        i -= 1
        j -= 2
    }
}
        
A nicer alternative would actually be the for-in loop:

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
    if i % 2 == 0 { continue }
    print(i, j)
}

The problem here is that the parameter „to:“ of stride() is not named very intuitively. Its alternative „through:“ is easier to understand.
Maybe „to:“ should be renamed to „towards:“ or something like that.

-Thorsten

···

Am 10.12.2015 um 18:16 schrieb Michel Fortin via swift-evolution <swift-evolution@swift.org>:


(Joe Groff) #10

Review of SE-0007

* What is your evaluation of the proposal?

I am against this change. I’ve seen no consideration of the performance implications that this change brings about. Also, many of the proposed alternatives tend towards using defer as the mechanism to provide consistent increments/decrements. However, this is problematic around the boundary edges as break and throw are not mechanisms that can halt the execution of the defer statement. So unlike the c-style loop that would not perform that code, the alternatives would.

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

No; it offers no alternative solution to maintain the performance that a c-style for-loop ensures. There is no general pattern than can be used to satisfy a c-style loop’s behavior; each proposed alternative has different issues.

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

No, it limits the language’s ability to create concise and legible code when the situation does arise for a c-style loop.

This is the alternative to create a loop that matches the same functionality, including preventing the leaking of loop-variables.

var sum = 0
for var i = 1000, j = 2000; i > 0 && j > 0; i -= 1, j -= 2 {
    if i % 2 == 0 { continue }
    sum += 1
}
print(sum)

// versus

var sum = 0

if true { // some arbitrary scope mechanism is needed to contain i and j
    var i = 1000, j = 2000
    while i > 0 && j > 0 {
        defer {
            i -= 1
            j -= 2
        }
        
        if i % 2 == 0 { continue }
        sum += 1
    }
}

print(sum)

To me, that is not a fair trade-off; maybe if the defer pattern could be used at all times and the leaking of the loop-variables was considered ok. There are some ways to consolidate the code so that it takes up less visual space, but I don’t do that for other constructs, so I don’t think that is a valid thing to try and do here. Also, the use of defer in this context is a somewhat of a trick that I’m not sure is even a good pattern to promote as it can lead to boundary-case crashes.

This is unsafe code:

var sum = 0
var i: UInt = 1000
while i >= 0 {
    defer {
        i -= 1
    }
    if i == 0 {
        print("zero has been reached!")
        break
    }

    if i % 2 == 0 { continue }
    sum += 1
}

The defer statement is still executed, so now I need to come up with a different pattern to handle this type of problem.

* If you have you 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?

A few hours. I’ve read the proposal multiple times and followed the mail threads. I’ve also done some basic performance analysis (attached mail) on the problem for some of the “improved” versions of the loop construct.

Performance is definitely a consideration; we already reverted a pull request that remove C-style fors from the standard library. I believe Andy is currently looking into where the regressions come from. stride(...) performing poorly seems like something we should fix regardless, since that's arguably the idiomatic way to write such loops and ought to perform well. I agree we should investigate ways to ensure common loops over ranges or strides perform reasonably at -Onone if we move forward with this.

-Joe

···

On Dec 10, 2015, at 5:17 PM, David Owens II via swift-evolution <swift-evolution@swift.org> wrote:

-David

<SE-0007 Remove C-style for-loops with conditions and incrementers - Performance Consideration.eml>

On Dec 7, 2015, at 12:44 PM, Douglas Gregor <dgregor@apple.com <mailto:dgregor@apple.com>> wrote:

Hello Swift community,

The review of "Remove C-style for-loops with conditions and incrementers” begins now and runs through Thursday, December 10th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

  Cheers,
  Doug Gregor
  Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org <mailto:swift-evolution-announce@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Michel Fortin) #11

do {
    func body(i: Int, _ j: Int) {
        if i % 2 == 0 { return } // simulating continue
        print (i, j)
    }
    var i = 10
    var j = 20
    while (i > 0 && j > 0) {
        body(i, j)
        i -= 1
        j -= 2
    }
}

This is actually worse than the while loop I suggested. Now you can't break from the loop, nor return from the outer function, and continue has to be replaced by return.

A nicer alternative would actually be the for-in loop:

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
    if i % 2 == 0 { continue }
    print(i, j)
}

Nice. :slight_smile:

But depending on the perspective, this can be harder to read. You have to be familiar with zip and stride, as well as with sequence types in general, to understand what the above loop does. The C-style for loop version, or even the while loop version, is easier to interpret: you just look at the loop header and figure out how each value change over each iteration.

There's another reason someone could want to avoid the for-in version above: performance relies entirely on the optimiser inlining everything. There's a much higher risk of an unforeseen cost hiding somewhere when a loop is abstracted this way. Performance will be fine in most cases, but if for some reason the optimizer fails you and that loop really has to be fast, you'll need to rewrite the loop at a lower level.

···

Le 10 déc. 2015 à 13:20, thorsten@portableinnovations.de a écrit :

--
Michel Fortin
michel.fortin@michelf.ca
https://michelf.ca


(Michel Fortin) #12

In this particular case you can and it won't have any noticeable side effects. But in the general case it has a different behaviour: the increment part is called on break or throw. Just imagine a loop body like this one:

  defer { i += 1 }
  if i == Int.max { break }

I know I keep coming up with made-up examples to disqualify smart replacements. That might seem artificial, and it certainly is. But any migration tool needs to guaranty identical behaviour.

I'm also starting to wonder if this example with defer that keeps coming up (it's probably the third time already) is an indicator that people will start to depend on clever tricks like this one, failing to realize the subtle change in semantics it introduces.

···

Le 10 déc. 2015 à 15:13, Stephen Celis <stephen.celis@gmail.com> a écrit :

Can't you do the following?

    var i = 10, j = 20; while i > 0 && j > 0 { defer { i -= 1; j -= 2 }
        // ...
    }

--
Michel Fortin
michel.fortin@michelf.ca
https://michelf.ca


(thorsten@portableinnovations.de) #13

do {
   func body(i: Int, _ j: Int) {
       if i % 2 == 0 { return } // simulating continue
       print (i, j)
   }
   var i = 10
   var j = 20
   while (i > 0 && j > 0) {
       body(i, j)
       i -= 1
       j -= 2
   }
}

This is actually worse than the while loop I suggested. Now you can't break from the loop, nor return from the outer function, and continue has to be replaced by return.

You are right. That's no good replacement.

A nicer alternative would actually be the for-in loop:

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
   if i % 2 == 0 { continue }
   print(i, j)
}

Nice. :slight_smile:

But depending on the perspective, this can be harder to read. You have to be familiar with zip and stride, as well as with sequence types in general, to understand what the above loop does. The C-style for loop version, or even the while loop version, is easier to interpret: you just look at the loop header and figure out how each value change over each iteration.

There's another reason someone could want to avoid the for-in version above: performance relies entirely on the optimiser inlining everything. There's a much higher risk of an unforeseen cost hiding somewhere when a loop is abstracted this way. Performance will be fine in most cases, but if for some reason the optimizer fails you and that loop really has to be fast, you'll need to rewrite the loop at a lower level.

Yes, performance is one thing neglected by the discussions and the proposal.
That just occurred to me and I'm going to revoke my support for the proposal for these reasons.

-Thorsten

···

Am 10.12.2015 um 20:57 schrieb Michel Fortin <michel.fortin@michelf.ca>:

Le 10 déc. 2015 à 13:20, thorsten@portableinnovations.de a écrit :

--
Michel Fortin
michel.fortin@michelf.ca
https://michelf.ca


(Nadav Rotem) #14

Review of SE-0007

* What is your evaluation of the proposal?

I am against this change. I’ve seen no consideration of the performance implications that this change brings about. Also, many of the proposed alternatives tend towards using defer as the mechanism to provide consistent increments/decrements. However, this is problematic around the boundary edges as break and throw are not mechanisms that can halt the execution of the defer statement. So unlike the c-style loop that would not perform that code, the alternatives would.

In the general case for-each loops are as efficient as c-style for loops. For example, the memset function below is optimized to something that is equivalent to the performance of C’s memset (no bounds checks, no overflow checks, vectorized, unrolled, etc).

I reverted the changes in the standard library because they exposed a problem in a different part of the code. The code for for-each is larger than c-style loops before it is optimized. According to Arnold, this changed the inlining decision and caused an extra retain/release pair, that slowed down String.

func memset(inout a: [Int], _ c: Int) {
  for i in 0..<a.count {
    a[i] = c
  }
}

···

On Dec 11, 2015, at 9:09 AM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 5:17 PM, David Owens II via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

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

No; it offers no alternative solution to maintain the performance that a c-style for-loop ensures. There is no general pattern than can be used to satisfy a c-style loop’s behavior; each proposed alternative has different issues.

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

No, it limits the language’s ability to create concise and legible code when the situation does arise for a c-style loop.

This is the alternative to create a loop that matches the same functionality, including preventing the leaking of loop-variables.

var sum = 0
for var i = 1000, j = 2000; i > 0 && j > 0; i -= 1, j -= 2 {
    if i % 2 == 0 { continue }
    sum += 1
}
print(sum)

// versus

var sum = 0

if true { // some arbitrary scope mechanism is needed to contain i and j
    var i = 1000, j = 2000
    while i > 0 && j > 0 {
        defer {
            i -= 1
            j -= 2
        }
        
        if i % 2 == 0 { continue }
        sum += 1
    }
}

print(sum)

To me, that is not a fair trade-off; maybe if the defer pattern could be used at all times and the leaking of the loop-variables was considered ok. There are some ways to consolidate the code so that it takes up less visual space, but I don’t do that for other constructs, so I don’t think that is a valid thing to try and do here. Also, the use of defer in this context is a somewhat of a trick that I’m not sure is even a good pattern to promote as it can lead to boundary-case crashes.

This is unsafe code:

var sum = 0
var i: UInt = 1000
while i >= 0 {
    defer {
        i -= 1
    }
    if i == 0 {
        print("zero has been reached!")
        break
    }

    if i % 2 == 0 { continue }
    sum += 1
}

The defer statement is still executed, so now I need to come up with a different pattern to handle this type of problem.

* If you have you 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?

A few hours. I’ve read the proposal multiple times and followed the mail threads. I’ve also done some basic performance analysis (attached mail) on the problem for some of the “improved” versions of the loop construct.

Performance is definitely a consideration; we already reverted a pull request that remove C-style fors from the standard library. I believe Andy is currently looking into where the regressions come from. stride(...) performing poorly seems like something we should fix regardless, since that's arguably the idiomatic way to write such loops and ought to perform well. I agree we should investigate ways to ensure common loops over ranges or strides perform reasonably at -Onone if we move forward with this.

-Joe

-David

<SE-0007 Remove C-style for-loops with conditions and incrementers - Performance Consideration.eml>

On Dec 7, 2015, at 12:44 PM, Douglas Gregor <dgregor@apple.com <mailto:dgregor@apple.com>> wrote:

Hello Swift community,

The review of "Remove C-style for-loops with conditions and incrementers” begins now and runs through Thursday, December 10th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

  Cheers,
  Doug Gregor
  Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org <mailto:swift-evolution-announce@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Brent Royal-Gordon) #15

In this particular case you can and it won't have any noticeable side effects. But in the general case it has a different behaviour: the increment part is called on break or throw

I know there’s no single replacement that compactly implements the exact behavior of a `for` loop using a `while` loop. But we don’t usually have to mimic the full behavior:

* The vast majority of for loops don’t use an iteration variable which shadows something in the enclosing scope, so the fact that the variable isn’t scoped to the loop doesn’t usually matter.
* The vast majority of for loops don’t have iteration statements which cause side effects beyond the statement itself, so the fact that `defer` will run an extra time if you exit the loop with `break` or `throw` doesn’t usually matter.

If we provide a fix-it, we should try to make sure it doesn’t write broken code. But if that means that, sometimes, we throw up our hands and don’t offer an automatic translation, I don’t think that’s the end of the world. If you’re maintaining code with bizarre, gnarly C-style for loops with no obvious translation, you probably know how to rewrite them as while loops.

···

--
Brent Royal-Gordon
Architechies


(David Owens II) #16

This is my primary objection to to this proposal; it assumes (or neglects?) that all of the types used can magically be inlined to nothing but the imperative code. This isn’t magical, someone has to implement the optimizations to do this.

Is there any guarantee that these two loops have the exact same runtime performance?

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
   if i % 2 == 0 { continue }
   print(i, j)
}

for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
   if i % 2 == 0 { continue }
   print(i, j)
}

And can you guarantee that performance is relatively the same across debug and release builds? Because historically, Swift has suffered greatly in this regard with respects to the performance of optimized versus non-optimized builds.

These types of optimizer issues are real-world things I’ve had to deal with (and have written up many blog posts about). I get the desire to simplify the constructs, but we need an escape hatch to write performant code when the optimizer isn’t up to the job.

-David

···

On Dec 10, 2015, at 1:57 PM, thorsten--- via swift-evolution <swift-evolution@swift.org> wrote:

Yes, performance is one thing neglected by the discussions and the proposal.


(David Owens II) #17

Here’s my basic test case:

let first = 10000000
let second = 20000000

class LoopPerfTests: XCTestCase {
    
    func testZipStride() {
        self.measureBlock {
            var sum = 0
            for (i, j) in zip(first.stride(to: 0, by: -1), second.stride(to: 0, by: -2)) {
                if i % 2 == 0 { continue }
                sum += 1
            }
            print(sum)
        }
    }
    
    func testCStyleFor() {
        self.measureBlock {
            var sum = 0
            for var i = first, j = second; i > 0 && j > 0; i -= 1, j -= 2 {
                if i % 2 == 0 { continue }
                sum += 1
            }
            print(sum)
        }

    }

}

Non-optimized timings:
testCStyleFor - 0.126s
testZipStride - 2.189s

Optimized timings:
testCStyleFor - 0.008s
testZipStride - 0.015s

That’s a lot worse than 34%; even in optimized builds, that’s 2x slower and in debug builds, that’s 17x slower. I think it’s unreasonable to force people to write a more verbose while-loop construct to simply get the performance they need.

Also, the readability argument is very subjective; for example, I don’t find the zip version more readability. In fact, I think it obscures what the logic of the loop is doing. But again, that’s subjective.

-David

···

On Dec 10, 2015, at 2:41 PM, Paul Cantrell <cantrell@pobox.com> wrote:

Is there any guarantee that these two loops have the exact same runtime performance?

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
   if i % 2 == 0 { continue }
   print(i, j)
}

for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
   if i % 2 == 0 { continue }
   print(i, j)
}

In a quick and dirty test, the second is approximately 34% slower.

I’d say that’s more than acceptable for the readability gain. If you’re in that rare stretch of critical code where the extra 34% actually matters, write it using a while loop instead.

P

On Dec 10, 2015, at 4:07 PM, David Owens II via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Dec 10, 2015, at 1:57 PM, thorsten--- via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Yes, performance is one thing neglected by the discussions and the proposal.

This is my primary objection to to this proposal; it assumes (or neglects?) that all of the types used can magically be inlined to nothing but the imperative code. This isn’t magical, someone has to implement the optimizations to do this.

Is there any guarantee that these two loops have the exact same runtime performance?

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
   if i % 2 == 0 { continue }
   print(i, j)
}

for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
   if i % 2 == 0 { continue }
   print(i, j)
}

And can you guarantee that performance is relatively the same across debug and release builds? Because historically, Swift has suffered greatly in this regard with respects to the performance of optimized versus non-optimized builds.

These types of optimizer issues are real-world things I’ve had to deal with (and have written up many blog posts about). I get the desire to simplify the constructs, but we need an escape hatch to write performant code when the optimizer isn’t up to the job.

-David

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Paul Cantrell) #18

Is there any guarantee that these two loops have the exact same runtime performance?

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
   if i % 2 == 0 { continue }
   print(i, j)
}

for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
   if i % 2 == 0 { continue }
   print(i, j)
}

In a quick and dirty test, the second is approximately 34% slower.

I’d say that’s more than acceptable for the readability gain. If you’re in that rare stretch of critical code where the extra 34% actually matters, write it using a while loop instead.

P

···

On Dec 10, 2015, at 4:07 PM, David Owens II via swift-evolution <swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 1:57 PM, thorsten--- via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Yes, performance is one thing neglected by the discussions and the proposal.

This is my primary objection to to this proposal; it assumes (or neglects?) that all of the types used can magically be inlined to nothing but the imperative code. This isn’t magical, someone has to implement the optimizations to do this.

Is there any guarantee that these two loops have the exact same runtime performance?

for (i, j) in zip(10.stride(to: 0, by: -1), 20.stride(to: 0, by: -2)) {
   if i % 2 == 0 { continue }
   print(i, j)
}

for var i = 10, j = 20; i > 0 && j > 0; i -= 1, j -= 2 {
   if i % 2 == 0 { continue }
   print(i, j)
}

And can you guarantee that performance is relatively the same across debug and release builds? Because historically, Swift has suffered greatly in this regard with respects to the performance of optimized versus non-optimized builds.

These types of optimizer issues are real-world things I’ve had to deal with (and have written up many blog posts about). I get the desire to simplify the constructs, but we need an escape hatch to write performant code when the optimizer isn’t up to the job.

-David

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Michel Fortin) #19

In this particular case you can and it won't have any noticeable side effects. But in the general case it has a different behaviour: the increment part is called on break or throw

I know there’s no single replacement that compactly implements the exact behavior of a `for` loop using a `while` loop. But we don’t usually have to mimic the full behavior:

* The vast majority of for loops don’t use an iteration variable which shadows something in the enclosing scope, so the fact that the variable isn’t scoped to the loop doesn’t usually matter.

Note that this is actually true *because* the variable is scoped. It's very common to have two loops in a row using the same variable name. This could of course be detected somehow by the migration tool, which could do a reasonably intelligent job here. I'm not that much concerned about this, it's just a minor nuisance.

* The vast majority of for loops don’t have iteration statements which cause side effects beyond the statement itself, so the fact that `defer` will run an extra time if you exit the loop with `break` or `throw` doesn’t usually matter.

If you discount potential integer overflows, you're right that increments generally won't have side effects.

If we provide a fix-it, we should try to make sure it doesn’t write broken code. But if that means that, sometimes, we throw up our hands and don’t offer an automatic translation, I don’t think that’s the end of the world.

So, how do you propose the fix-it tool determines which code has side effects and which code doesn't? I don't think it can. It certainly could recognize some common safe patterns and offer tailored solutions to these patterns however, if you invest enough time in the tool.

There is never a reason for the fix-it tool to abort and not offer a translation. There is a simple solution that will work in all cases, which I presented in my review. It's ugly and verbose. But it's better to have ugly code that is guarantied to works than leaving the code in an uncompilable state.

···

Le 10 déc. 2015 à 16:35, Brent Royal-Gordon <brent@architechies.com> a écrit :

--
Michel Fortin
michel.fortin@michelf.ca
https://michelf.ca


(David Waite) #20

Do you mean your support is conditional based on performance?

-DW

···

On Dec 10, 2015, at 2:57 PM, thorsten--- via swift-evolution <swift-evolution@swift.org> wrote:

Yes, performance is one thing neglected by the discussions and the proposal.
That just occurred to me and I'm going to revoke my support for the proposal for these reasons.

-Thorsten