Skip to content

Commit

Permalink
automatic loop variables in for loops ("loop var"), consistent with g…
Browse files Browse the repository at this point in the history
…o 1.22 behavior

This builds on #1644 and adds automatic per-loop variables that are consistent with go 1.22 behavior.  See #1643 for discussion.

This is still a draft because the for7 version ends up capturing the per-loop var values when they are +1 relative to what they should be.  Maybe somehow the incrementing and conditional code is somehow capturing the within loop variables and incrementing them?  not sure how that would work.  anyway, need to investigate further before this is ready to go, but pushing it here in case there are other issues or someone might figure out this bug before I do..
  • Loading branch information
rcoreilly committed Jul 20, 2024
1 parent b6315ca commit 9c4dcfc
Show file tree
Hide file tree
Showing 18 changed files with 320 additions and 24 deletions.
6 changes: 3 additions & 3 deletions _test/closure10.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ func main() {
}

// Output:
// 3 0 0
// 3 1 1
// 3 2 2
// 0 0 0
// 1 1 1
// 2 2 2
6 changes: 3 additions & 3 deletions _test/closure11.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ func main() {
}

// Output:
// 3 0
// 3 1
// 3 2
// 0 0
// 1 1
// 2 2
6 changes: 3 additions & 3 deletions _test/closure12.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ func main() {
}

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

func main() {
foos := []func(){}

for i := range 3 {
a := i
foos = append(foos, func() { println(i, a) })
}
foos[0]()
foos[1]()
foos[2]()
}

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

func main() {
foos := []func(){}

for i := range 3 {
a, b := i, i
foos = append(foos, func() { println(i, a, b) })
}
foos[0]()
foos[1]()
foos[2]()
}

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

type T struct {
F func()
}

func main() {
foos := []T{}

for i := range 3 {
a := i
foos = append(foos, T{func() { println(i, a) }})
}
foos[0].F()
foos[1].F()
foos[2].F()
}

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

import "fmt"

type T struct {
F func()
}

func main() {
foos := []T{}

for i := range 3 {
a := i
n := fmt.Sprintf("i=%d", i)
foos = append(foos, T{func() { println(i, a, n) }})
}
foos[0].F()
foos[1].F()
foos[2].F()
}

// Output:
// 0 0 i=0
// 1 1 i=1
// 2 2 i=2
18 changes: 18 additions & 0 deletions _test/closure19.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package main

func main() {
foos := []func(){}

for i := 0; i < 3; i++ {
i := i
foos = append(foos, func() { println(i) })
}
foos[0]()
foos[1]()
foos[2]()
}

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

func main() {
foos := []func(){}

for i := range 3 {
i := i
foos = append(foos, func() { println(i) })
}
foos[0]()
foos[1]()
foos[2]()
}

// Output:
// 0
// 1
// 2
6 changes: 3 additions & 3 deletions _test/closure9.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ func main() {
}

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

func main() {
mx := 3
for i := range mx {
println(i)
}
}

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

func main() {
for i := range 3 {
println(i)
}
}

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

func main() {
for range 3 {
println("i")
}
}

// Output:
// i
// i
// i
17 changes: 16 additions & 1 deletion interp/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,22 @@ func (interp *Interpreter) ast(f ast.Node) (string, *node, error) {
st.push(addChild(&root, anc, pos, kind, act), nod)

case *ast.BlockStmt:
st.push(addChild(&root, anc, pos, blockStmt, aNop), nod)
b := addChild(&root, anc, pos, blockStmt, aNop)
st.push(b, nod)
var kind nkind
if anc.node != nil {
kind = anc.node.kind
}
switch kind {
case rangeStmt:
k := addChild(&root, astNode{b, nod}, pos, identExpr, aNop)
k.ident = "_"
v := addChild(&root, astNode{b, nod}, pos, identExpr, aNop)
v.ident = "_"
case forStmt7:
k := addChild(&root, astNode{b, nod}, pos, identExpr, aNop)
k.ident = "_"
}

case *ast.BranchStmt:
var kind nkind
Expand Down
72 changes: 66 additions & 6 deletions interp/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
}

case blockStmt:
var rangek, rangev *node
if n.anc != nil && n.anc.kind == rangeStmt {
// For range block: ensure that array or map type is propagated to iterators
// prior to process block. We cannot perform this at RangeStmt pre-order because
Expand Down Expand Up @@ -202,25 +203,66 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
sc.add(sc.getType("int")) // Add a dummy type to store array shallow copy for range
ktyp = sc.getType("int")
vtyp = o.typ.val
case intT:
n.anc.gen = rangeInt
sc.add(sc.getType("int"))
ktyp = sc.getType("int")
}

kindex := sc.add(ktyp)
sc.sym[k.ident] = &symbol{index: kindex, kind: varSym, typ: ktyp}
k.typ = ktyp
k.findex = kindex
rangek = k

if v != nil {
vindex := sc.add(vtyp)
sc.sym[v.ident] = &symbol{index: vindex, kind: varSym, typ: vtyp}
v.typ = vtyp
v.findex = vindex
rangev = v
}
}
}

n.findex = -1
n.val = nil
sc = sc.pushBloc()

if n.anc != nil && n.anc.kind == rangeStmt {
lk := n.child[0]
if rangek != nil {
lk.ident = rangek.ident
lk.typ = rangek.typ
kindex := sc.add(lk.typ)
sc.sym[lk.ident] = &symbol{index: kindex, kind: varSym, typ: lk.typ}
lk.findex = kindex
lk.gen = loopVarKey
}
lv := n.child[1]
if rangev != nil {
lv.ident = rangev.ident
lv.typ = rangev.typ
vindex := sc.add(lv.typ)
sc.sym[lv.ident] = &symbol{index: vindex, kind: varSym, typ: lv.typ}
lv.findex = vindex
lv.gen = loopVarVal
}
}
if n.anc != nil && n.anc.kind == forStmt7 {
lv := n.child[0]
init := n.anc.child[0]
if init.kind == defineStmt && len(init.child) >= 2 && init.child[0].kind == identExpr {
fi := init.child[0]
lv.ident = fi.ident
lv.typ = fi.typ
vindex := sc.add(lv.typ)
sc.sym[lv.ident] = &symbol{index: vindex, kind: varSym, typ: lv.typ}
lv.findex = vindex
lv.gen = loopVarFor
}
}

// Pre-define symbols for labels defined in this block, so we are sure that
// they are already defined when met.
// TODO(marc): labels must be stored outside of symbols to avoid collisions.
Expand Down Expand Up @@ -692,6 +734,22 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
return
}
if sc.global || sc.isRedeclared(dest) {
if n.anc != nil && n.anc.anc != nil && (n.anc.anc.kind == forStmt7 || n.anc.anc.kind == rangeStmt) {
// check for redefine of for loop variables, which are now auto-defined in go1.22
init := n.anc.anc.child[0]
var fi *node // for ident
if n.anc.anc.kind == forStmt7 {
if init.kind == defineStmt && len(init.child) >= 2 && init.child[0].kind == identExpr {
fi = init.child[0]
}
} else { // range
fi = init
}
if fi != nil && dest.ident == fi.ident {
n.gen = nop
break
}
}
// Do not overload existing symbols (defined in GTA) in global scope.
sym, _, _ = sc.lookup(dest.ident)
}
Expand Down Expand Up @@ -1523,6 +1581,7 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
err = cond.cfgErrorf("non-bool used as for condition")
}
n.start = init.start
body.start = body.child[0] // loopvar
if cond.rval.IsValid() {
// Condition is known at compile time, bypass test.
if cond.rval.Bool() {
Expand Down Expand Up @@ -1755,12 +1814,13 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
} else {
k, o, body = n.child[0], n.child[1], n.child[2]
}
n.start = o.start // Get array or map object
o.tnext = k.start // then go to iterator init
k.tnext = n // then go to range function
n.tnext = body.start // then go to range body
body.tnext = n // then body go to range function (loop)
k.gen = empty // init filled later by generator
n.start = o.start // Get array or map object
o.tnext = k.start // then go to iterator init
k.tnext = n // then go to range function
body.start = body.child[0] // loopvar
n.tnext = body.start // then go to range body
body.tnext = n // then body go to range function (loop)
k.gen = empty // init filled later by generator
}

case returnStmt:
Expand Down
5 changes: 1 addition & 4 deletions interp/interp_consistent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ import (
"github.com/traefik/yaegi/stdlib/unsafe"
)

// The following tests depend on an incompatible language change in go1.22, where `for` variables are now
// defined in body (thus reallocated at each loop). We skip them until both supported versions behave the same.
// We will remove this in Go1.23.
var testsToSkipGo122 = map[string]bool{"closure9.go": true, "closure10.go": true, "closure11.go": true, "closure12.go": true}
var testsToSkipGo122 = map[string]bool{}

var go122 = strings.HasPrefix(runtime.Version(), "go1.22")

Expand Down
5 changes: 4 additions & 1 deletion interp/interp_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import (

// The following tests sometimes (not always) crash with go1.21 but not with go1.20 or go1.22.
// The reason of failure is not obvious, maybe due to the runtime itself, and will be investigated separately.
var testsToSkipGo121 = map[string]bool{"cli6.go": true, "cli7.go": true, "issue-1276.go": true, "issue-1330.go": true, "struct11.go": true}
// Also, the closure tests depend on an incompatible language change in go1.22, where `for` variables are now
// defined in body (thus reallocated at each loop). This is now the behavior in yaegi, so 1.21 produces
// different results.
var testsToSkipGo121 = map[string]bool{"cli6.go": true, "cli7.go": true, "issue-1276.go": true, "issue-1330.go": true, "struct11.go": true, "closure9.go": true, "closure10.go": true, "closure11.go": true, "closure12.go": true, "closure15.go": true, "closure16.go": true, "closure17.go": true, "closure18.go": true, "closure20.go": true, "for17.go": true, "for18.go": true, "for19.go": true}

var go121 = strings.HasPrefix(runtime.Version(), "go1.21")

Expand Down
Loading

0 comments on commit 9c4dcfc

Please sign in to comment.