Skip to content

Commit

Permalink
interp: correctly init variables assigned from function call
Browse files Browse the repository at this point in the history
In the case of a Go short definition (i.e. `a, b := f()`), the new defined variables must be (re-)created in order to preserve the previous value (if in a loop) which can be still in use in the context of a closure. This must not apply to redeclared variables which simply see their value reassigned.

The problem was both occuring in callBin() for runtime functions and assignFromCall() for functions created in the interpreter.

Fixes #1497.
  • Loading branch information
mvertes authored Mar 24, 2023
1 parent ce2bb79 commit 20c8f5e
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 32 deletions.
36 changes: 36 additions & 0 deletions _test/closure13.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package main

import (
"fmt"
"strconv"
)

type monkey struct {
test func() int
}

func main() {
input := []string{"1", "2", "3"}

var monkeys []*monkey

for _, v := range input {
kong := monkey{}
divisor, err := strconv.Atoi(v)
if err != nil {
panic(err)
}
fmt.Print(divisor, " ")
kong.test = func() int {
return divisor
}
monkeys = append(monkeys, &kong)
}

for _, mk := range monkeys {
fmt.Print(mk.test(), " ")
}
}

// Output:
// 1 2 3 1 2 3
32 changes: 32 additions & 0 deletions _test/closure14.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package main

import "fmt"

type monkey struct {
test func() int
}

func getk(k int) (int, error) { return k, nil }

func main() {
input := []string{"1", "2", "3"}

var monkeys []*monkey

for k := range input {
kong := monkey{}
divisor, _ := getk(k)
fmt.Print(divisor, " ")
kong.test = func() int {
return divisor
}
monkeys = append(monkeys, &kong)
}

for _, mk := range monkeys {
fmt.Print(mk.test(), " ")
}
}

// Output:
// 0 1 2 0 1 2
3 changes: 2 additions & 1 deletion interp/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
return
}
if sc.global {
// Do not overload existing symbols (defined in GTA) in global scope
// Do not overload existing symbols (defined in GTA) in global scope.
sym, _, _ = sc.lookup(dest.ident)
}
if sym == nil {
Expand Down Expand Up @@ -2323,6 +2323,7 @@ func compDefineX(sc *scope, n *node) error {
canRedeclare := hasNewSymbol && len(symIsNew) > 1 && !symIsNew[id] && ok
if canRedeclare && level == n.child[i].level && sym.kind == varSym && sym.typ.id() == t.id() {
index = sym.index
n.child[i].redeclared = true
} else {
index = sc.add(t)
sc.sym[id] = &symbol{index: index, kind: varSym, typ: t}
Expand Down
59 changes: 30 additions & 29 deletions interp/interp.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,35 @@ import (

// Interpreter node structure for AST and CFG.
type node struct {
debug *nodeDebugData // debug info
child []*node // child subtrees (AST)
anc *node // ancestor (AST)
param []*itype // generic parameter nodes (AST)
start *node // entry point in subtree (CFG)
tnext *node // true branch successor (CFG)
fnext *node // false branch successor (CFG)
interp *Interpreter // interpreter context
frame *frame // frame pointer used for closures only (TODO: suppress this)
index int64 // node index (dot display)
findex int // index of value in frame or frame size (func def, type def)
level int // number of frame indirections to access value
nleft int // number of children in left part (assign) or indicates preceding type (compositeLit)
nright int // number of children in right part (assign)
kind nkind // kind of node
pos token.Pos // position in source code, relative to fset
sym *symbol // associated symbol
typ *itype // type of value in frame, or nil
recv *receiver // method receiver node for call, or nil
types []reflect.Type // frame types, used by function literals only
scope *scope // frame scope
action action // action
exec bltn // generated function to execute
gen bltnGenerator // generator function to produce above bltn
val interface{} // static generic value (CFG execution)
rval reflect.Value // reflection value to let runtime access interpreter (CFG)
ident string // set if node is a var or func
meta interface{} // meta stores meta information between gta runs, like errors
debug *nodeDebugData // debug info
child []*node // child subtrees (AST)
anc *node // ancestor (AST)
param []*itype // generic parameter nodes (AST)
start *node // entry point in subtree (CFG)
tnext *node // true branch successor (CFG)
fnext *node // false branch successor (CFG)
interp *Interpreter // interpreter context
frame *frame // frame pointer used for closures only (TODO: suppress this)
index int64 // node index (dot display)
findex int // index of value in frame or frame size (func def, type def)
level int // number of frame indirections to access value
nleft int // number of children in left part (assign) or indicates preceding type (compositeLit)
nright int // number of children in right part (assign)
kind nkind // kind of node
pos token.Pos // position in source code, relative to fset
sym *symbol // associated symbol
typ *itype // type of value in frame, or nil
recv *receiver // method receiver node for call, or nil
types []reflect.Type // frame types, used by function literals only
scope *scope // frame scope
action action // action
exec bltn // generated function to execute
gen bltnGenerator // generator function to produce above bltn
val interface{} // static generic value (CFG execution)
rval reflect.Value // reflection value to let runtime access interpreter (CFG)
ident string // set if node is a var or func
redeclared bool // set if node is a redeclared variable (CFG)
meta interface{} // meta stores meta information between gta runs, like errors
}

func (n *node) shouldBreak() bool {
Expand Down Expand Up @@ -105,7 +106,7 @@ type receiver struct {
type frame struct {
// id is an atomic counter used for cancellation, only accessed
// via newFrame/runid/setrunid/clone.
// Located at start of struct to ensure proper aligment.
// Located at start of struct to ensure proper alignment.
id uint64

debug *frameDebugData
Expand Down
24 changes: 22 additions & 2 deletions interp/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,15 @@ func assignFromCall(n *node) {
continue
}
s := f.data[ncall.findex+i]
c := n.child[i]
if n.kind == defineXStmt && !c.redeclared {
// Recreate destination value in case of define statement,
// to preserve previous value possibly in use by a closure.
data := getFrame(f, c.level).data
data[c.findex] = reflect.New(data[c.findex].Type()).Elem()
data[c.findex].Set(s)
continue
}
v(f).Set(s)
}
return next
Expand Down Expand Up @@ -1587,9 +1596,20 @@ func callBin(n *node) {
}
out := callFn(value(f), in)
for i, v := range rvalues {
if v != nil {
v(f).Set(out[i])
if v == nil {
continue // Skip assign "_".
}
c := n.anc.child[i]
if n.anc.kind == defineXStmt && !c.redeclared {
// In case of a define statement, the destination value in the frame
// must be recreated. This is necessary to preserve the previous value
// which may be still used in a separate closure.
data := getFrame(f, c.level).data
data[c.findex] = reflect.New(data[c.findex].Type()).Elem()
data[c.findex].Set(out[i])
continue
}
v(f).Set(out[i])
}
return tnext
}
Expand Down

0 comments on commit 20c8f5e

Please sign in to comment.