(defer would be more challenging 🤔 Anyway, the point isn't really to come up with a full proposal for this, but just to show general feasibility)

-- #47331 (reply in thread)

@Merovius Building on your example, could defers work something like this? Assuming the ?'s are replaced with code for the internal types are for defers.

for v := range x {
    if cond1 { break }
    if cond2 { continue }
    if cond3 { return a }
    if cond4 { defer z.Push(v) }
}

var (
    _tmp_ret bool
    _tmp_a A
    _tmp_defers []?
)
x.Do(func(v B) bool {
    if cond1 { return false }
    if cond2 { return true }
    if cond3 {
        _tmp_a, _tmp_ret = a, true
        return false
    }
    if cond4 {
        _tmp_defers = append(_tmp_defers, ?z.Push(v)?)
    }
})
if _tmp_ret {
    return _tmp_a
}
for i := len(_tmp_defers)-1; i >= 0; i-- {
    defer ?_tmp_defers[I]?
}

Personally, I don't really see the need to make range work with defined types. I think iterators (like sql.Rows) produce well-readable, efficient code, without a language change (whereas the Do convention is really inconvenient without this language change). And they are far more flexible - e.g. it is easy to add error-handling to them, support key/value iteration or add other, more special data (like (*sql.Rows).Columns).

So, personally, I would rather if we focus on using iterators for a generic iteration interface, gain some experience with what methods are common and what we do need from them. And then, maybe, make a subset of them usable with range - though, again, the need to do that isn't very large with iterator-types, as they work well on their own.

That being said, I'm not strongly opposed to this proposal.

One thing the proposal text doesn't answer is, what would happen in this code:

type Ranger struct {
    f func() bool
}

func (r *Ranger) Do(f func(v int) bool) {
    r.f = f
    return
}

func F() func() bool {
    r := new(Ranger)
    for _ = range r {
        defer func() { fmt.Println("Hello world") } ()
    }
    return r.f
}

func main() {
    f := F()
    f() // ???
}

This uses defer because it creates a pretty immediate problem that needs answering - but IMO this is an issue with the general Do API. By using an autogenerated closure, you have to deal with the question of what happens if the function you call retains that func. I think that's a novel problem, because we don't have any language features which call user-defined code so far, so we have far more flexibility in implementation options. Note, for example, that we specifically don't allow user-code to run between fork(2) and exec(2), because of the issues of how that would interact with the runtime.

Assuming the ?'s are replaced with code for the internal types are for defers.

I think for illustrative purposes, that is just func().

We need to consider how to handle goto statements in the range loop.

This doesn’t show what any iterators might look like, which is where the fault comes in. Here is an example of what an iterator would look like:

func (s SomeType) Do(forEach func (i int) bool) {
    var i int
    for {
        ok := forEach(i)
        if !ok {
            break
        }
    }
}

The issue with the Do method is that an improperly defined iterator (ie, one that forgets to check the ok variable) will continue calling the forEach function until iteration completes. In my example, the iteration would never complete because it’s building a sequence. This means that if you try to return inside of of one of these improperly-defined iterators, it simply won’t work because control flow is entirely in control of the Do function. This is not a problem that other iterator patterns have, as typically the caller of an iterator is in charge of control flow.

Also, as I mentioned in #43557 (comment) it would be much better to have a github discussion for this instead of multiple competing proposals.

This also only supports iterating structures, whereas I may want an iterator which is defined at the top-level, like:

func Fibonacci(forEach func(i int) bool) {
    …
}

It is also worth mentioning that @DeedleFake had already proposed (and I supported) a similar concept in #43557 (comment), which was a more-flexible version of this proposal, but hadn’t thought of an ok bool return.

The main issue that I highlighted earlier had already been brought up in the other proposal (however I translated “issue with the proposal” to “issue when we forget to check the ok variable”)

Personally, I don't really see the need to make range work with defined types. I think iterators (like sql.Rows) produce well-readable, efficient code, without a language change (whereas the Do convention is really inconvenient without this language change). And they are far more flexible - e.g. it is easy to add error-handling to them, support key/value iteration or add other, more special data (like (*sql.Rows).Columns).

@Merovius I think this is a good point. Iterators are very flexible and they address the problem of how to control the current scope well during iteration. The sql.Rows example is a little different in that it is a scanning pattern which is not quite the same as iterating over known types and values, but that is definitely evidence of that patterns flexibility.

The Do function has problems when used on its own as a closure, but I think it is easier to integrate Do into for rather than a two function Next and Scan.

If generics mean that there will be a shift towards defined types, such as container/set for collections rather than using built-in types I think there is value in considering how range can be used to iterate defined types.

By using an autogenerated closure, you have to deal with the question of what happens if the function you call retains that func.

@Merovius I think in this case the rewritten function is captured which captures the scope of the function containing the for loop like it would if the user referenced variables in that scope inside the closure. That would be odd, but storing a function inside an iterator like you demonstrated is likely to have strange side-effects in any situation. Would you expect a function stored in that case to work differently?

I think the main oddity is that defers could be lifted, and you could set state that affected gotos without necessarily exiting out if the caller of the rewritten function chose not to exit early, as in @deanveloper's example #47707 (comment).

If Go had a func type that didn't have its own scope, similar to blocks in Ruby, all of these problems would probably be alleviated because we could simple say that it is a block and not a closure, but I assume that would be a huge language change and I don't know how I feel about that to even suggest it.

We need to consider how to handle goto statements in the range loop.

@ianlancetaylor I think the go-to case is workable by setting a temp bool and exiting early. For example:

Label:
// ...
for v := range x {
    if cond1 { break }
    if cond2 { continue }
    if cond3 { return a }
    if cond4 { defer z.Push(v) }
    if cond5 { goto Label }
}

var (
    _tmp_ret bool
    _tmp_a A
    _tmp_defers []?
    _tmp_goto bool
)
x.Do(func(v B) bool {
    if cond1 { return false }
    if cond2 { return true }
    if cond3 {
        _tmp_a, _tmp_ret = a, true
        return false
    }
    if cond4 {
        _tmp_defers = append(_tmp_defers, ?z.Push(v)?)
    }
    if cond5 {
        _tmp_goto = true
        return false
    }
})
for i := len(_tmp_defers)-1; i >= 0; i-- {
    defer ?_tmp_defers[I]?
}
if _tmp_ret {
    return _tmp_a
}
if _tmp_goto {
    goto Label
}

@leighmcculloch

I think in this case the rewritten function is captured which captures the scope of the function containing the for loop like it would if the user referenced variables in that scope inside the closure.

You seem to be ignoring the defer. The problem isn't capturing over the scope, but that the control flow is ill-defined.
By definition, the defered function should be run when F returns. However, the loop-body is then re-executed, when the closure is called again in main. How is that supposed to work?

This wouldn't be a problem if defer would be scoped to the loop-body, as then it could just be translated into a defer in the generated function. But it isn't, it's scoped the encompassing function.

@ianlancetaylor brought up goto, which has very similar problems. If you do

func type Ranger struct {
    f func() bool
}

func (r *Ranger) Do(f func(v int) bool) {
    r.f = f
    return
}

func F() func() bool {
    r := new(Ranger)
    for _ = range r {
        goto Foo
    }
Foo:
    fmt.Println("Hello world")
    return r.f
}

func main() {
    f := F()
    f()
}

would the call to f jump back into F? After all, you are executing the loop-body, which contains a goto.

This isn't just an issue of closing over variables.

From the first sentence of the first comment in this issue:

The proposal is to allow any defined type to be iterated on using the for with range clause if that type loosely implements the following interface.

There are many possible designs to make this work. But this issue only explores using "Do", which has some issues.

Would proposing alternatives be appropriate here or in another proposal? (For example, I'm thinking Ranger like this:

type Iter [Index, Element any] interface {
     Next() (Index, Element, bool) 
}
type Ranger [Index, Element any] interface {
     Range() Iter[Index, Element]
}

```) 

The above at least would fit the analysis ssa model.

If that or other alternatives have been discussed, where are they?

@wsc0 A similar idea was mentioned in #43557 (and a new one was brought up as an alternative, which I personally like a lot better)

Would proposing alternatives be appropriate here or in another proposal?

I think it's easier to discuss different ideas in different issues. We can still cross reference them. Could you open a new issue if you have an alternative proposal?

type Iter [Index, Element any] interface {
Next() (Index, Element, bool)
}
type Ranger [Index, Element any] interface {
Range() Iter[Index, Element]
}

@wsc0 This interface only satisfies iterating with an index and an element. For supports iterating with other values, the spec refers to them as iteration variables and they can be different depending on the type being ranged over. For example, how would a type that operates like a channel work with it since it iterates with no index?

@leighmcculloch

I just took a look at #43557, which has a nice overview of different ideas, and made me realise having a separate Iter type is more verbose. I was indeed thinking of something similar to #43557 , so I'll not open another issue.

Of course, the single value return would need to be treated differently -- that was just a sketch. Thanks for pointing it out nonetheless.

Something else that this proposal seems to leave out is how to pass around iterators. For instance, how would I write the equivalent of Map[T, U any](Iterator[T], func(T) U) Iterator[U] under this proposal? It seems impossible without goroutines and channels. Unfortunately, goroutines and channels have very significant overhead (#43557 (comment)), being multiple orders of magnitude less efficient than function-based iterators. But if our solution is to fix these performance issues, then we may as well just use channels as our iterators instead of functions.

@deanveloper

type Collection[Elem any] interface {
    Do(func(Elem) bool)
}

type mapper[T, U any] struct {
    c Collection[T]
    f func(T) U
}

func (m mapper[T, U]) Do(f func(U) bool) {
    m.c.Do(func(t T) bool {
        return f(m.f(t))
    })
}

func Map[T, U any](c Collection[T], f func(T) U) Collection[U] {
    return mapper{c, f}
}

(I called it Collection instead of Iterator, as that seems a more appropriate name for the concept under this proposal)

@Merovius My bad - the example I was thinking of where this would have trouble wasn't a map operation, but a zip operation. The issue wasn't being able to pass around the iterator, but being able to iterate over two things at the same time. I think that it would not be possible without goroutines.

(I called it Collection instead of Iterator, as that seems a more appropriate name for the concept under this proposal)

Collection is still a bit weird considering that a mapper isn't really a Collection... Same thing for infinite "rangers" like sequences. In Java this concept is named Iterable.

@deanveloper

I agree that zipping seems hard without goroutines.

As this was just brought to my attention again:

@leighmcculloch

@Merovius Building on your example, could defers work something like this? Assuming the ?'s are replaced with code for the internal types are for defers.

for v := range x {
    if cond1 { break }
    if cond2 { continue }
    if cond3 { return a }
    if cond4 { defer z.Push(v) }
}

var (
    _tmp_ret bool
    _tmp_a A
    _tmp_defers []?
)
x.Do(func(v B) bool {
    if cond1 { return false }
    if cond2 { return true }
    if cond3 {
        _tmp_a, _tmp_ret = a, true
        return false
    }
    if cond4 {
        _tmp_defers = append(_tmp_defers, ?z.Push(v)?)
    }
})
if _tmp_ret {
    return _tmp_a
}
for i := len(_tmp_defers)-1; i >= 0; i-- {
    defer ?_tmp_defers[I]?
}

This doesn't work, no. Consider

for x := range c {
    defer fmt.Println("foo")
    panic(nil)
}

This should print "foo" for a non-empty container and then panic. But with that rewrite, it won't print anything, as it panics before the defer is pushed.

I know that I claimed that this kind of rewrite would work, but after some time and all this discussion, I really think it's at the very least extremely complex. I would go so far as to say I was mistaken to suggest it.

If you add a defer prior to x.Do to call the deferred functions, it should work.

for v := range x {
    if cond1 { break }
    if cond2 { continue }
    if cond3 { return a }
    if cond4 { defer z.Push(v) }
    if cond5 { goto Label }
}

var (
    _tmp_ret bool
    _tmp_a A
    _tmp_defers []func()
    _tmp_goto bool
)
defer func() {
    for fn := range _tmp_defers {
        defer fn()
    }
}()
x.Do(func(v B) bool {
    if cond1 { return false }
    if cond2 { return true }
    if cond3 {
        _tmp_a, _tmp_ret = a, true
        return false
    }
    if cond4 {
        _tmp_defers = append(_tmp_defers, func() { z.Push(v) })
    }
    if cond5 {
        _tmp_goto = true
        return false
    }
})
if _tmp_ret {
    return _tmp_a
}
if _tmp_goto {
    goto Label
}

Handling deferred functions in a deferred function ensures it always runs, regardless of panics/etc. Iterating over the slice normally and using defer ensures A) deferred calls are executed in the expected order (LIFO) and B) subsequent deferred calls are still executed if one call panics.

Of course the runtime could potentially do some trickery to make it more efficient.

I do not yet have a good understanding of how these suggestions interact with https://go.dev/ref/spec#Handling_panics . Specifically:

recover was not called directly by a deferred function.

How are we handling the additional indirection created by func() { z.Push(v) } + defer fn(), or a ?z.Push(v)?. The deferred functions need to "called directly by a deferred function".

Example to illustrate what I am getting at does: https://go.dev/play/p/Wa8VXAAW1ts

I’m assuming the compiler and runtime would do something smarter than what I wrote. I’m not suggesting that the compiler literally generate what I wrote, but I find it easier to reason about valid Go instead of “insert compiler magic here”.

Here is a crack at handling defers and panics. The key idea is to record the deferred functions and arguments and to re-raise the panic value if panicking.

Need a slightly more complex example to illustrate.

defer f()
Label:
//  ...
for v := range x {
    if cond1 { break }
    if cond2 { continue }
    if cond3 { return a }
    if cond4 { defer z.Push(v) }
    if cond5 { goto Label }
    if cond6 { defer func() { _ = recover() } }
    if cond7 { panic(nil) }
}
defer f()

This can be rewritten as:

defer f()
Label:
// ...
type _tmp_defer_loc1 struct{ f: func(V); arg0: V } // store the defer function value and args for first defer location
type _tmp_defer_loc2 struct{ f: func() }
var (
    _tmp_ret bool
    _tmp_a A
    _tmp_defers []any
    _tmp_exit bool // true => loop exited without a panic
    _tmp_pval any // if so with what value
)
x.Do(func(v B) bool {
    _tmp_exit = false
    defer func() { _tmp_pval = recover() }
    if cond1 {  _tmp_exit = true; return false }
    if cond2 { _tmp_exit = true; return true }
    if cond3 {
        _tmp_a, _tmp_ret, _tmp_exit = a, true, true
        return false
    }
    if cond4 {
        _tmp_defers = append(_tmp_defers, _tmp_defer_loc1{f: z.Push}, arg0: v)
    }
    if cond5 {
        _tmp_goto, _tmp_exit = true, true
        return false
    }
    if cond6 {
        _tmp_defers = append(_tmp_defers, _tmp_defer_loc2 {f : func() { _ = recover() } })
    }
    if cond7 {
       panic(nil)
    }
    _tmp_exit = true
    return true
})

for i := 0; i < len(_tmp_defers); i++ { // edit: original order was wrong.
    switch d := _tmp_defers[i].(type) {
    case _tmp_defer_loc1:
      defer d.f(d.arg0)
    case _tmp_defer_loc2:
      defer d.f()
   }
}

if !_tmp_exit {
    panic(_tmp_pval) // needs magic for the runtime to give the right stack trace.
}
if _tmp_ret {
    return _tmp_a
}
if _tmp_goto {
    goto Label
}
defer f()

I know that I claimed that this kind of rewrite would work, but after some time and all this discussion, I really think it's at the very least extremely complex. I would go so far as to say I was mistaken to suggest it.

I thought this would require continuations when I first saw thought of how to deal with defers, but maybe the save the panicing value and deferred functions is sufficient?

The problem with deferring in the for-loop after x.Do is that means none of the simulated defer calls will get hit if there's a panic in x.Do or the callback.

I don't think there's any need to rewrite panic. A panic within the callback will work exactly as expected, as long as x.Do runs the callback in the same goroutine.

The problem with deferring in the for-loop after x.Do is that means none of the simulated defer calls will get hit if there's a panic in x.Do or the callback.

Fair point about x.Do panicking. My suggestion is not correct for that case. I am not sure I see a problem about the callback yet.

I don't think there's any need to rewrite panic. A panic within the callback will work exactly as expected, as long as x.Do runs the callback in the same goroutine.

What I was trying to address with my suggestion is ordering the defer statements, such that recover can be handled correctly. Spec: "recover was not called directly by a deferred function." So it is not quite enough to re-execute the same deferred functions with the same arguments in the correct order. Which function stack matters too. This was why I was saving the deferred functions and executing them in the original function body.

Here is a variant of the previous that uses a defer before x.Do, and within this it defers the _temp_defers and then recovers and re-raises a panic if recover() != nil. This is trying to avoid explicitly re-raising the exception.

type _tmp_defer_loc1 struct{ f: func(V); arg0: V }
type _tmp_defer_loc2 struct{ f: func() }
var (
    // ...
    _tmp_defers []any
)
defer func() {
  for i := range _tmp_defers {
      switch d := _tmp_defers[i].(type) {
      case _tmp_defer_loc1:
        defer d.f(d.arg0)
      case _tmp_defer_loc2:
        defer d.f()
     }
  }
  if r := recover(); r != nil { // not quite right. will stop unwinding panic(nil) prematurely. may need help from runtime.
    panic(r)
  }
}()
x.Do(func(v B) bool {
   // ...

You can try playing around the idea of re-raising the panic in the defer https://go.dev/play/p/zmXP_MVAM5U. It seems to be needed to achieve the correct order.

This feels close, but I don't think it is not quite right yet. The panicking unwinding order does not seem correct in all cases yet.

Closing in favor of #43557.