Interpreted closures mixed with "binary" functions don't capture values correctly

[theclapp]

The following test case TestIssue1634 in interp/interp_eval_test.go triggers an unexpected result

func TestIssue1634(t *testing.T) {
    TLogf := t.Logf

    type CPE struct {
        E string
    }

    type CP struct {
        Es []*CPE
    }

    type Child struct {
        I int
        E string
        F func() string
    }

    FL := func(children []*Child) []string {
        s := []string{}
        for _, child := range children {
            s = append(s, child.F())
        }
        return s
    }

    R := func(i int, eE string, f func() string) *Child {
        TLogf("inside R: %d, %s", i, eE)
        return &Child{
            I: i,
            E: eE,
            F: f,
        }
    }

    var CpL func(cp *CP) []string
    // Having this on a line by itself makes it easy to copy into the interpreted
    // code below.
    CpL = func(cp *CP) []string {
        var ch []*Child
        for i, e := range cp.Es {
            i, e := i, e
            ch = append(ch,
                R(i, e.E, func() string {
                    TLogf("inside R's f: %d, %v", i, e.E)
                    return e.E
                }),
            )
        }
        return FL(ch)
    }

    var Cp *CP
    initCp := func() {
        Cp = &CP{
            Es: []*CPE{
                {E: "foo"},
                {E: "bar"},
                {E: "baz"},
            },
        }
    }
    initCp()

    // Verify that this works in standard Go
    t.Logf("Standard Go:")
    s := CpL(Cp)
    if expected, got := []string{"foo", "bar", "baz"}, s; !reflect.DeepEqual(expected, got) {
        t.Fatalf("Standard Go: CpL: Expected %v, got %v", expected, got)
    }

    i := interp.New(interp.Options{})
    err := i.Use(stdlib.Symbols)
    if err != nil {
        t.Fatalf("i.Use: Expected nil, got %v", err)
    }
    i.Use(interp.Exports{
        "pkg/pkg": {
            // vars
            "Cp":    reflect.ValueOf(&Cp).Elem(),
            "CpL":   reflect.ValueOf(&CpL).Elem(),
            "TLogf": reflect.ValueOf(&TLogf).Elem(),

            // funcs
            "FL": reflect.ValueOf(FL),
            "R":  reflect.ValueOf(R),

            // types
            "CP":    reflect.ValueOf((*CP)(nil)),
            "Child": reflect.ValueOf((*Child)(nil)),
        },
    })
    i.ImportUsed()

    initCp()
    _, err = i.Eval(`
package main

import . "pkg"

func main() {
    CpL = func(cp *CP) []string {
        var ch []*Child
        for i, e := range cp.Es {
            i, e := i, e
            ch = append(ch,
                R(i, e.E, func() string {
                    TLogf("inside R's f: %d, %v", i, e.E)
                    return e.E
                }),
            )
        }
        return FL(ch)
    }
}
    `)
    if err != nil {
        t.Fatalf("i.Eval: Expected nil, got %v", err)
    }
    t.Logf("Interpreted Go:")
    s = CpL(Cp)
    if expected, got := []string{"foo", "bar", "baz"}, s; !reflect.DeepEqual(expected, got) {
        t.Fatalf("Interpreted Go CpL: Expected %v, got %v", expected, got)
    }
}

Expected result

All tests pass; interpreted output matches compiled output

Got

% go test ./interp -run TestIssue1634
--- FAIL: TestIssue1634 (0.01s)
    interp_eval_test.go:2104: Standard Go:
    interp_eval_test.go:2066: inside R: 0, foo
    interp_eval_test.go:2066: inside R: 1, bar
    interp_eval_test.go:2066: inside R: 2, baz
    interp_eval_test.go:2083: inside R's f: 0, foo
    interp_eval_test.go:2083: inside R's f: 1, bar
    interp_eval_test.go:2083: inside R's f: 2, baz
    interp_eval_test.go:2158: Interpreted Go:
    interp_eval_test.go:2066: inside R: 0, foo
    interp_eval_test.go:2066: inside R: 1, bar
    interp_eval_test.go:2066: inside R: 2, baz
    value.go:596: inside R's f: 2, baz
    value.go:596: inside R's f: 2, baz
    value.go:596: inside R's f: 2, baz
    interp_eval_test.go:2161: Interpreted Go CpL: Expected [foo bar baz], got [baz baz baz]
FAIL
FAIL    github.com/traefik/yaegi/interp 0.365s
FAIL

Yaegi Version

381e045

Additional Notes

No response

[theclapp]

Pulling the closure that captures the loop variables out of the arglist of the "binary" function R works, if that helps:

Change this:

for i, e := range cp.Es {
    i, e := i, e
    ch = append(ch,
        // breaks
        R(i, e.E, func() string {
            TLogf("inside R's f: %d, %v", i, e.E)
            return e.E
        }),
    )
}

To this:

for i, e := range cp.Es {
    i, e := i, e
    // works
    f := func() string {
        TLogf("inside R's f: %d, %v", i, e.E)
        return e.E
    }
    ch = append(ch, R(i, e.E, f))
}

Also, I think that the fact that these are loop variables is a red herring, I think this is probably a problem with any closure being passed as a literal to a "binary" function.

Unfortunately, the code I'm playing with is awash with such things. :)

[rcoreilly]

This issue is very general -- we encountered it as well, and were also able to work around it by assigning a variable to the funLit function literal.

This suggests that the general solution is to add an exec to a funcLit that makes a new reflect.Value copy of the func lit in the frame. Ideally it would detect the outer context of being passed as an arg to another function (doable), in a for loop (probably pretty hard in general).

I just experimented with this and it doesn't seem to work:

in run.go:
func funcLitCopy(n *node) {
    next := getExec(n.tnext)
    n.exec = func(f *frame) bltn {
        ov := f.data[n.findex]
        nv := reflect.New(ov.Type()).Elem()
        nv.Set(ov)
        f.data[n.findex] = nv
        return next
    }
}
in cfg.go, post-processing:
        case funcLit:
            n.types, n.scope = sc.types, sc
            sc = sc.pop()
            err = genRun(n)
            n.gen = funcLitCopy

unfortunately, in this test:

func TestForRangeClosure(t *testing.T) {
    i := New(Options{})
    _, err := i.Eval(`
func main() {
    fs := []func()
    for i := range 3 {
        println(i, &i)
        fs = append(fs, func() { println(i, &i) })
    }
    for _, f := range fs {
        f()
    }
}
`)
    if err != nil {
        t.Error(err)
    }
}

it triggers a "reflect.Value.Call: call of nil function" -- presumably copying the reflect value doesn't do what one might have hoped it would?

it would probably make more sense to figure out what the process of defining a new var for the funcLit is actually accomplishing, and then just replicate that in the exec. apparently making a reflect clone is not it..