yaegi
yaegi copied to clipboard
traefik
Interpreted closures mixed with "binary" functions don't capture values correctly
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
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. :)
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..
finally found and fixed the issue! in run.go, see comments at the end:
func genFunctionWrapper(n *node) func(*frame) reflect.Value {
var def *node
var ok bool
if def, ok = n.val.(*node); !ok {
return genValueAsFunctionWrapper(n)
}
start := def.child[3].start
numRet := len(def.typ.ret)
var rcvr func(*frame) reflect.Value
if n.recv != nil {
rcvr = genValueRecv(n)
}
funcType := n.typ.TypeOf()
value := genValue(n)
return func(f *frame) reflect.Value {
v := value(f)
if v.Kind() == reflect.Func {
// per #1634, if v is already a func, then don't re-wrap! critically, the original wrapping
// clones the frame, whereas the one here (below) does _not_ clone the frame, so it doesn't
// generate the proper closure capture effects!
// this path is the same as genValueAsFunctionWrapper which is the path taken above if
// the value has an associated node, which happens when you do f := func() ..
return v
}
...
the tests reveal that this does break a defer test, so it needs to be further qualified somehow, but at least the crux of the issue here is clear.