Implement the break statement (#3320)

* Implement the 'break' statement

* Track whether we are within a loop or not

docs/grammar.txt

@@ -2,7 +2,7 @@
 file_input = { statement };
 
 # Any single statement. Must occur on its own line.
-statement = ( "pass" | "continue" | func_def | for | if | return |
+statement = ( "pass" | "continue" | "break" | func_def | for | if | return |
              assert | ident_statement | expression ) EOL;
 return = "return" [ expression { "," expression } ];
 assert = "assert" expression [ "," expression ];

src/parse/asp/grammar.go

@@ -28,6 +28,7 @@ type Statement struct {
 	Literal  *Expression
 	Pass     bool
 	Continue bool
+	Break    bool
 }
 
 // An AssertStatement implements the 'assert' statement.

src/parse/asp/grammar_parse.go

@@ -49,6 +49,7 @@ var keywords = map[string]struct{}{
 type parser struct {
 	l      *lex
 	endPos Position
+	inFor  bool
 }
 
 // parseFileInput is the only external entry point to this class, it parses a file into a FileInput structure.
@@ -153,13 +154,25 @@ func (p *parser) parseStatement() *Statement {
 		p.endPos = p.l.Next().EndPos()
 		p.next(EOL)
 	case "continue":
+		p.assert(p.inFor, tok, "'continue' outside loop")
 		s.Continue = true
 		p.endPos = p.l.Next().EndPos()
 		p.next(EOL)
+	case "break":
+		p.assert(p.inFor, tok, "'break' outside loop")
+		s.Break = true
+		p.endPos = p.l.Next().EndPos()
+		p.next(EOL)
 	case "def":
+		before := p.inFor
+		p.inFor = false
 		s.FuncDef = p.parseFuncDef()
+		p.inFor = before
 	case "for":
+		before := p.inFor
+		p.inFor = true
 		s.For = p.parseFor()
+		p.inFor = before
 	case "if":
 		s.If = p.parseIf()
 	case "return":

src/parse/asp/interpreter.go

@@ -555,6 +555,9 @@ func (s *scope) interpretStatements(statements []*Statement) pyObject {
 		} else if stmt.Continue {
 			// This is definitely awkward since we need to control a for loop that's happening in a function outside this scope.
 			return continueIteration
+		} else if stmt.Break {
+			// Similar to above, although CPython does do this the same way...
+			return stopIteration
 		} else if stmt.Pass {
 			continue // Nothing to do...
 		} else {
@@ -580,8 +583,11 @@ func (s *scope) interpretFor(stmt *ForStatement) pyObject {
 	for li := range s.iterable(&stmt.Expr) {
 		s.unpackNames(stmt.Names, li)
 		if ret := s.interpretStatements(stmt.Statements); ret != nil {
-			if s, ok := ret.(pySentinel); ok && s == continueIteration {
+			switch ret {
+			case continueIteration:
 				continue
+			case stopIteration:
+				break
 			}
 			return ret
 		}

src/parse/asp/interpreter_test.go

@@ -709,3 +709,9 @@ func TestListConcatenation(t *testing.T) {
 		pyString("haribo"),
 	}, s.Lookup("fruit_veg_canned_food_and_sweets"))
 }
+
+func TestBreakLoop(t *testing.T) {
+	s, err := parseFile("src/parse/asp/test_data/interpreter/break_loop.build")
+	assert.NoError(t, err)
+	assert.EqualValues(t, 1, s.Lookup("i"))
+}

src/parse/asp/objects.go

@@ -134,13 +134,16 @@ func (n pyNone) MarshalJSON() ([]byte, error) {
 
 // A pySentinel is an internal implementation detail used in some cases. It should never be
 // exposed to users.
-type pySentinel struct{}
+type pySentinel string
 
 // continueIteration is used to implement the "continue" statement.
-var continueIteration = pySentinel{}
+var continueIteration = pySentinel("ContinueIteration")
+
+// stopIteration is used to implement the "break" statement.
+var stopIteration = pySentinel("StopIteration")
 
 func (s pySentinel) Type() string {
-	return "sentinel"
+	return string(s)
 }
 
 func (s pySentinel) TypeTag() int32 {
@@ -152,11 +155,11 @@ func (s pySentinel) IsTruthy() bool {
 }
 
 func (s pySentinel) String() string {
-	panic("non stringable type sentinel")
+	panic("non stringable type " + string(s))
 }
 
 func (s pySentinel) MarshalJSON() ([]byte, error) {
-	panic("non serialisable type sentinel")
+	panic("non serialisable type " + string(s))
 }
 
 type pyInt int

src/parse/asp/parser_test.go

@@ -831,3 +831,29 @@ func TestFStringIncompleteError(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "Unterminated brace in fstring")
 }
+
+// Continue shouldn't be allowed outside a loop
+func TestContinueOutsideLoop(t *testing.T) {
+	_, err := newParser().parseAndHandleErrors(strings.NewReader("continue"))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "'continue' outside loop")
+}
+
+// Break shouldn't be allowed outside a loop
+func TestBreakOutsideLoop(t *testing.T) {
+	_, err := newParser().parseAndHandleErrors(strings.NewReader("break"))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "'break' outside loop")
+}
+
+// Functions have a new scope that doesn't count as within the enclosing loop
+func TestBreakWithinFunctionWithinLoop(t *testing.T) {
+	const code = `
+for i in [1,2,3]:
+    def foo():
+        break
+`
+	_, err := newParser().parseAndHandleErrors(strings.NewReader(code))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "'break' outside loop")
+}

src/parse/asp/test_data/interpreter/break_loop.build

@@ -0,0 +1,2 @@
+for i in range(1, 3):
+    break