interp: fix the behaviour of goto, continue and break

_test/break0.go

@@ -0,0 +1,24 @@
+package main
+
+func main() {
+	n := 2
+	m := 2
+	foo := true
+OuterLoop:
+	println("Boo")
+	for i := 0; i < n; i++ {
+		println("I: ", i)
+		for j := 0; j < m; j++ {
+			switch foo {
+			case true:
+				println(foo)
+				break OuterLoop
+			case false:
+				println(foo)
+			}
+		}
+	}
+}
+
+// Error:
+// 15:5: invalid break label OuterLoop

_test/break1.go

@@ -0,0 +1,24 @@
+package main
+
+func main() {
+	n := 2
+	m := 2
+	foo := true
+OuterLoop:
+	for i := 0; i < n; i++ {
+		println("I: ", i)
+		for j := 0; j < m; j++ {
+			switch foo {
+			case true:
+				println(foo)
+				break OuterLoop
+			case false:
+				println(foo)
+			}
+		}
+	}
+}
+
+// Output:
+// I:  0
+// true

_test/break2.go

@@ -0,0 +1,25 @@
+package main
+
+func main() {
+	n := 2
+	m := 2
+	foo := true
+OuterLoop:
+	for i := 0; i < n; i++ {
+		println("I: ", i)
+		for j := 0; j < m; j++ {
+			switch foo {
+			case true:
+				println(foo)
+				break OuterLoop
+			case false:
+				println(foo)
+				continue OuterLoop
+			}
+		}
+	}
+}
+
+// Output:
+// I:  0
+// true

_test/break3.go

@@ -0,0 +1,27 @@
+package main
+
+func main() {
+	n := 2
+	m := 2
+	foo := true
+	goto OuterLoop
+	println("Boo")
+OuterLoop:
+	for i := 0; i < n; i++ {
+		println("I: ", i)
+		for j := 0; j < m; j++ {
+			switch foo {
+			case true:
+				println(foo)
+				break OuterLoop
+			case false:
+				println(foo)
+				goto OuterLoop
+			}
+		}
+	}
+}
+
+// Output:
+// I:  0
+// true

_test/cont2.go

@@ -0,0 +1,26 @@
+package main
+
+func main() {
+	n := 2
+	m := 2
+	foo := true
+OuterLoop:
+	for i := 0; i < n; i++ {
+		println("I: ", i)
+		for j := 0; j < m; j++ {
+			switch foo {
+			case true:
+				println(foo)
+				continue OuterLoop
+			case false:
+				println(foo)
+			}
+		}
+	}
+}
+
+// Output:
+// I:  0
+// true
+// I:  1
+// true

_test/cont3.go

@@ -0,0 +1,24 @@
+package main
+
+func main() {
+	n := 2
+	m := 2
+	foo := true
+OuterLoop:
+	println("boo")
+	for i := 0; i < n; i++ {
+		println("I: ", i)
+		for j := 0; j < m; j++ {
+			switch foo {
+			case true:
+				println(foo)
+				continue OuterLoop
+			case false:
+				println(foo)
+			}
+		}
+	}
+}
+
+// Error:
+// 15:5: invalid continue label OuterLoop

_test/issue-1354.go

@@ -0,0 +1,21 @@
+package main
+
+func main() {
+	println(test()) // Go prints true, Yaegi false
+}
+
+func test() bool {
+	if true {
+		goto label
+	}
+	goto label
+label:
+	println("Go continues here")
+	return true
+	println("Yaegi goes straight to this return (this line is never printed)")
+	return false
+}
+
+// Output:
+// Go continues here
+// true

interp/cfg.go

@@ -202,38 +202,42 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
 					}
 				}
 			}
+
 			n.findex = -1
 			n.val = nil
 			sc = sc.pushBloc()
-
-		case breakStmt, continueStmt, gotoStmt:
-			if len(n.child) > 0 {
-				// Handle labeled statements.
-				label := n.child[0].ident
-				if sym, _, ok := sc.lookup(label); ok {
-					if sym.kind != labelSym {
-						err = n.child[0].cfgErrorf("label %s not defined", label)
-						break
-					}
-					sym.from = append(sym.from, n)
-					n.sym = sym
-				} else {
-					n.sym = &symbol{kind: labelSym, from: []*node{n}, index: -1}
-					sc.sym[label] = n.sym
+			// 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.
+			for _, c := range n.child {
+				if c.kind != labeledStmt {
+					continue
 				}
+				label := c.child[0].ident
+				sym := &symbol{kind: labelSym, node: c, index: -1}
+				sc.sym[label] = sym
+				c.sym = sym
 			}
 
-		case labeledStmt:
+		case breakStmt, continueStmt, gotoStmt:
+			if len(n.child) == 0 {
+				break
+			}
+			// Handle labeled statements.
 			label := n.child[0].ident
-			// TODO(marc): labels must be stored outside of symbols to avoid collisions
-			// Used labels are searched in current and sub scopes, not upper ones.
-			if sym, ok := sc.lookdown(label); ok {
-				sym.node = n
+			if sym, _, ok := sc.lookup(label); ok {
+				if sym.kind != labelSym {
+					err = n.child[0].cfgErrorf("label %s not defined", label)
+					break
+				}
 				n.sym = sym
 			} else {
-				n.sym = &symbol{kind: labelSym, node: n, index: -1}
+				n.sym = &symbol{kind: labelSym, index: -1}
+				sc.sym[label] = n.sym
+			}
+			if n.kind == gotoStmt {
+				n.sym.from = append(n.sym.from, n) // To allow forward goto statements.
 			}
-			sc.sym[label] = n.sym
 
 		case caseClause:
 			sc = sc.pushBloc()
@@ -874,28 +878,43 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
 			n.rval = l.rval
 
 		case breakStmt:
-			if len(n.child) > 0 {
-				gotoLabel(n.sym)
-			} else {
+			if len(n.child) == 0 {
 				n.tnext = sc.loop
+				break
 			}
+			if !n.hasAnc(n.sym.node) {
+				err = n.cfgErrorf("invalid break label %s", n.child[0].ident)
+				break
+			}
+			n.tnext = n.sym.node
 
 		case continueStmt:
-			if len(n.child) > 0 {
-				gotoLabel(n.sym)
-			} else {
+			if len(n.child) == 0 {
 				n.tnext = sc.loopRestart
+				break
 			}
+			if !n.hasAnc(n.sym.node) {
+				err = n.cfgErrorf("invalid continue label %s", n.child[0].ident)
+				break
+			}
+			n.tnext = n.sym.node.child[1].lastChild().start
 
 		case gotoStmt:
-			gotoLabel(n.sym)
+			if n.sym.node == nil {
+				// It can be only due to a forward goto, to be resolved at labeledStmt.
+				// Invalid goto labels are catched at AST parsing.
+				break
+			}
+			n.tnext = n.sym.node.start
 
 		case labeledStmt:
 			wireChild(n)
 			if len(n.child) > 1 {
 				n.start = n.child[1].start
 			}
-			gotoLabel(n.sym)
+			for _, c := range n.sym.from {
+				c.tnext = n.start // Resolve forward goto.
+			}
 
 		case callExpr:
 			for _, c := range n.child {
@@ -2479,6 +2498,15 @@ func (n *node) fieldType(m int) *node {
 // lastChild returns the last child of a node.
 func (n *node) lastChild() *node { return n.child[len(n.child)-1] }
 
+func (n *node) hasAnc(nod *node) bool {
+	for a := n.anc; a != nil; a = a.anc {
+		if a == nod {
+			return true
+		}
+	}
+	return false
+}
+
 func isKey(n *node) bool {
 	return n.anc.kind == fileStmt ||
 		(n.anc.kind == selectorExpr && n.anc.child[0] != n) ||
@@ -2645,17 +2673,6 @@ func typeSwichAssign(n *node) bool {
 	return ts.kind == typeSwitch && ts.child[1].action == aAssign
 }
 
-func gotoLabel(s *symbol) {
-	if s.node == nil {
-		return
-	}
-	for _, c := range s.from {
-		if c.tnext == nil {
-			c.tnext = s.node.start
-		}
-	}
-}
-
 func compositeGenerator(n *node, typ *itype, rtyp reflect.Type) (gen bltnGenerator) {
 	switch typ.cat {
 	case aliasT, ptrT:

interp/interp_consistent_test.go

@@ -37,6 +37,8 @@ func TestInterpConsistencyBuild(t *testing.T) {
 			file.Name() == "assign12.go" || // expect error
 			file.Name() == "assign15.go" || // expect error
 			file.Name() == "bad0.go" || // expect error
+			file.Name() == "break0.go" || // expect error
+			file.Name() == "cont3.go" || // expect error
 			file.Name() == "const9.go" || // expect error
 			file.Name() == "export1.go" || // non-main package
 			file.Name() == "export0.go" || // non-main package
@@ -198,6 +200,16 @@ func TestInterpErrorConsistency(t *testing.T) {
 			expectedInterp: "1:1: expected 'package', found println",
 			expectedExec:   "1:1: expected 'package', found println",
 		},
+		{
+			fileName:       "break0.go",
+			expectedInterp: "15:5: invalid break label OuterLoop",
+			expectedExec:   "15:11: invalid break label OuterLoop",
+		},
+		{
+			fileName:       "cont3.go",
+			expectedInterp: "15:5: invalid continue label OuterLoop",
+			expectedExec:   "15:14: invalid continue label OuterLoop",
+		},
 		{
 			fileName:       "const9.go",
 			expectedInterp: "5:2: constant definition loop",

interp/scope.go

@@ -47,7 +47,7 @@ type symbol struct {
 	kind    sKind
 	typ     *itype        // Type of value
 	node    *node         // Node value if index is negative
-	from    []*node       // list of nodes jumping to node if kind is label, or nil
+	from    []*node       // list of goto nodes jumping to this label node, or nil
 	recv    *receiver     // receiver node value, if sym refers to a method
 	index   int           // index of value in frame or -1
 	rval    reflect.Value // default value (used for constants)
@@ -144,20 +144,6 @@ func (s *scope) lookup(ident string) (*symbol, int, bool) {
 	return nil, 0, false
 }
 
-// lookdown searches for a symbol in the current scope and included ones, recursively.
-// It returns the first found symbol and true, or nil and false.
-func (s *scope) lookdown(ident string) (*symbol, bool) {
-	if sym, ok := s.sym[ident]; ok {
-		return sym, true
-	}
-	for _, c := range s.child {
-		if sym, ok := c.lookdown(ident); ok {
-			return sym, true
-		}
-	}
-	return nil, false
-}
-
 func (s *scope) rangeChanType(n *node) *itype {
 	if sym, _, found := s.lookup(n.child[1].ident); found {
 		if t := sym.typ; len(n.child) == 3 && t != nil && (t.cat == chanT || t.cat == chanRecvT) {