[WIP] feat: capture transient values in loop block for closures

[ltzmaxwell]

this is a fix to #1135 .

package main

func main() {
        var fns []func()
        for _, v := range []int{1, 2, 3} {
                x := v*100 + v
                fns = append(fns, func() { println(x) })
        }
        for _, fn := range fns {
                fn()
        }
}

As discussed in #1135, the go behaviour (expected) should is:

101
202
303

while gno behavior:

303
303
303

The reason for this is the funcLit inside loop block is not capturing every transient values of v as the loop went on, It only keeps a reference to it. when this funcLit is in execution, the value of v is 3, so the result will always be 303.

This PR provides a solution to capture the transient state as the loop went on, and use these values when the closure is in execution.

The work flow is:

1. Identify the specific pattern. It's always showing up with a loop block and a nested funcLit.
2. Record transient values for captured named vars. the transient state of v is recorded as [1, 2, 3];
3. Use the recorded values to update context. when fn (the closure) is in execution, using [1, 2, 3] to update the context block(in which v == 3) to be 1, 2, 3 , and get the expected results.

Here are some examples to show in detail:

1. this is generally same with for loops maintain the same block on iteration, which is referenced in any closures generated within #1135, the fix makes it consistent go Go.

package main

// this is a fix, intuitive, and same with Go
func main() {
	var fns []func() int
	for i := 0; i < 5; i++ {
		x := i
		f := func() int {
			return x
		}
		fns = append(fns, f)
	}
	for _, fn := range fns {
		println(fn())
	}
}

// Go Output:
// 0
// 1
// 2
// 3
// 4

// Output:
// 0
// 1
// 2
// 3
// 4

2. this is a fix to make it consistent with Go's experimental feature of loopVar. It's a design decision if we want this.

package main

// this is a fix, intuitive, same with go loopVar experiment feature.
func main() {
	var fns []func() int
	for i := 0; i < 5; i++ {
		f := func() int {
			return i
		}
		fns = append(fns, f)
	}
	for _, fn := range fns {
		println(fn())
	}
}

// Go Output(without loopVar flag):
// 5
// 5
// 5
// 5
// 5

// Output:
// 0
// 1
// 2
// 3
// 4

3. this one is consistent with Go too, but from an intuitive perspective, It should capture x the var declared outside of the for block, I'd like to leave it for discussion since it's a design decision.

package main

// this still keeps consistent with go, that a variable out of loop block is not captured
// this is unintuitive(should capture something)
// TODO: leave it for discuss.

func main() {
	var fns []func() int
	var x int
	for i := 0; i < 5; i++ {
		x = i
		f := func() int {
			return x
		}
		fns = append(fns, f)
	}
	for _, fn := range fns {
		println(fn())
	}
}

// Go Output:
// 4
// 4
// 4
// 4
// 4

// Output:
// 4
// 4
// 4
// 4
// 4

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.75%. Comparing base (6452476) to head (dc61511).
Report is 124 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1585       +/-   ##
===========================================
- Coverage   55.87%   44.75%   -11.13%     
===========================================
  Files         430      438        +8     
  Lines       65618    67250     +1632     
===========================================
- Hits        36667    30100     -6567     
- Misses      26083    34637     +8554     
+ Partials     2868     2513      -355     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jaekwon]

delete?

[deelawn]

@ltzmaxwell amazing! 🤩 Thanks as always for tackling these difficult VM issues.

Some things I really like about this:

• it seems like you have a pretty clear understanding of the issue and the approach to fixing it
• the added tests are super helpful to understand behavior and guard against future regressions

I spent quite a bit of time reviewing your solution, as well as time trying to understand both the problem a bit more. After thinking about this, I arrived at a related but different way to solve the problem using fewer abstractions. Do you think that you could give it a look? Perhaps there is a combined solution that would work well; the code you wrote here certainly made it easier for me to commit my own thoughts to code. https://github.com/deelawn/gno/pull/3/files

Also, I was thinking about how a similar issue might arise that doesn't use explicit loops. Consider the following example:

package main

func main() {
	var y, counter int
	var f []func()
	defer func() {
		for _, ff := range f {
			ff()
		}
	}()
LABEL:
	if counter == 5 {
		return
	}
	x := y
	f = append(f, func() { println(x) })
	y++
	counter++
	goto LABEL
}

// Output:
// 0
// 1
// 2
// 3
// 4

The solution you've presented here does not account for this scenario. How easy do you think it would be to incorporate it?

I'm currently not set on either solution; there may be an even better way than both of us have thought of. I'm hoping some others can post their thoughts here as well, so let me know what you think 😄

[thehowl]

Just a status update: @jaekwon discussed with maxwell for an alternative approach, which fixes this issue in Preprocessing. @ltzmaxwell, please do ping us for a second review when done!

[ltzmaxwell]

Just a status update: @jaekwon discussed with maxwell for an alternative approach, which fixes this issue in Preprocessing. @ltzmaxwell, please do ping us for a second review when done!

Make it draft until done.

[thehowl]

note: your result doesn't match Go's output: https://go.dev/play/p/pbx4N-F4W89

[ltzmaxwell]

house keeping.