interp: fix the behaviour of goto, continue and break (#1392)

The logic of goto was false due to mixing break label and goto
label, despite being opposite. In case of break, jump to node (the
exit point) instead of node.start. Also always define label symbols
before their use is parsed.

* Fix continue label, detect invalid labels for break and continue

* fix multiple goto, break, continue to the same label

Fixes #1354.

_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()
+			// 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 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
-				}
+			if len(n.child) == 0 {
+				break
 			}
-
-		case labeledStmt:
+			// 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) {