go.tools/go/types: check unlabeled break, continue statements

This CL temporarily removes some preliminary label checks.
They will be implemented completely in a subsequent CL.

R=adonovan
CC=golang-dev
https://golang.org/cl/14055043

go/types/check.go

@@ -49,7 +49,7 @@ type checker struct {
 	// functions
 	funcList []funcInfo // list of functions/methods with correct signatures and non-empty bodies
 	funcSig  *Signature // signature of currently type-checked function
-	labels   *Scope     // label scope of currently type-checked function; lazily allocated
+	hasLabel bool       // set if a function makes use of labels (only ~1% of functions)
 
 	// debugging
 	indent int // indentation for tracing

go/types/resolver.go

@@ -396,21 +396,16 @@ func (check *checker) resolveFiles(files []*ast.File) {
 
 		check.topScope = f.sig.scope // open function scope
 		check.funcSig = f.sig
-		check.labels = nil // lazily allocated
+		check.hasLabel = false
-		check.stmtList(f.body.List, false)
+		check.stmtList(0, f.body.List)
+
+		if check.hasLabel {
+			// TODO(gri) check label use
+		}
 
 		if f.sig.results.Len() > 0 && !check.isTerminating(f.body, "") {
 			check.errorf(f.body.Rbrace, "missing return")
 		}
-
-		// spec: "It is illegal to define a label that is never used."
-		if check.labels != nil {
-			for _, obj := range check.labels.elems {
-				if l := obj.(*Label); !l.used {
-					check.errorf(l.pos, "%s defined but not used", l.name)
-				}
-			}
-		}
 	}
 
 	// Phase 5: Check for declared but not used packages and variables.

go/types/stdlib_test.go

@@ -117,8 +117,8 @@ func testTestDir(t *testing.T, path string, ignore ...string) {
 
 func TestStdtest(t *testing.T) {
 	testTestDir(t, filepath.Join(runtime.GOROOT(), "test"),
-		"cmplxdivide.go",       // also needs file cmplxdivide1.go - ignore
+		"cmplxdivide.go",                   // also needs file cmplxdivide1.go - ignore
-		"goto.go", "label1.go", // TODO(gri) implement missing label checks
+		"goto.go", "label.go", "label1.go", // TODO(gri) implement missing label checks
 		"mapnan.go", "sigchld.go", // don't work on Windows; testTestDir should consult build tags
 		"sizeof.go", "switch.go", // TODO(gri) tone down duplicate checking in expr switches
 		"typeswitch2.go", // TODO(gri) implement duplicate checking in type switches

go/types/stmt.go

@@ -11,20 +11,32 @@ import (
 	"go/token"
 )
 
-func (check *checker) optionalStmt(s ast.Stmt) {
+// stmtContext is a bitset describing the environment
+// (outer statements) containing a statement.
+type stmtContext uint
+
+const (
+	fallthroughOk stmtContext = 1 << iota
+	inBreakable
+	inContinuable
+)
+
+func (check *checker) initStmt(s ast.Stmt) {
 	if s != nil {
-		scope := check.topScope
+		check.stmt(0, s)
-		check.stmt(s, false)
-		assert(check.topScope == scope)
 	}
 }
 
-func (check *checker) stmtList(list []ast.Stmt, fallthroughOk bool) {
+func (check *checker) stmtList(ctxt stmtContext, list []ast.Stmt) {
-	scope := check.topScope
+	ok := ctxt&fallthroughOk != 0
+	inner := ctxt &^ fallthroughOk
 	for i, s := range list {
-		check.stmt(s, fallthroughOk && i+1 == len(list))
+		inner := inner
+		if ok && i+1 == len(list) {
+			inner |= fallthroughOk
+		}
+		check.stmt(inner, s)
 	}
-	assert(check.topScope == scope)
 }
 
 func (check *checker) multipleDefaults(list []ast.Stmt) {
@@ -53,10 +65,10 @@ func (check *checker) multipleDefaults(list []ast.Stmt) {
 	}
 }
 
-func (check *checker) openScope(node ast.Node) {
+func (check *checker) openScope(s ast.Stmt) {
-	s := NewScope(check.topScope)
+	scope := NewScope(check.topScope)
-	check.recordScope(node, s)
+	check.recordScope(s, scope)
-	check.topScope = s
+	check.topScope = scope
 }
 
 func (check *checker) closeScope() {
@@ -88,11 +100,23 @@ func (check *checker) suspendedCall(keyword string, call *ast.CallExpr) {
 }
 
 // stmt typechecks statement s.
-func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
+func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
 	// statements cannot use iota in general
 	// (constant declarations set it explicitly)
 	assert(check.iota == nil)
 
+	// statements must end with the same top scope as they started with
+	if debug {
+		defer func(scope *Scope) {
+			// don't check if code is panicking
+			if p := recover(); p != nil {
+				panic(p)
+			}
+			assert(scope == check.topScope)
+		}(check.topScope)
+	}
+
+	inner := ctxt &^ fallthroughOk
 	switch s := s.(type) {
 	case *ast.BadStmt, *ast.EmptyStmt:
 		// ignore
@@ -101,18 +125,8 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 		check.declStmt(s.Decl)
 
 	case *ast.LabeledStmt:
-		scope := check.labels
+		check.hasLabel = true
-		if scope == nil {
+		check.stmt(ctxt, s.Stmt)
-			scope = NewScope(nil) // no label scope chain
-			check.labels = scope
-		}
-		label := s.Label
-		l := NewLabel(label.Pos(), label.Name)
-		// Labels are not resolved yet - mark them as used to avoid errors.
-		// TODO(gri) fix this
-		l.used = true
-		check.declareObj(scope, label, l)
-		check.stmt(s.Stmt, fallthroughOk)
 
 	case *ast.ExprStmt:
 		// spec: "With the exception of specific built-in functions,
@@ -227,19 +241,23 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 		}
 
 	case *ast.BranchStmt:
+		if s.Label != nil {
+			check.hasLabel = true
+			return // checks handled in separate pass
+		}
 		switch s.Tok {
 		case token.BREAK:
-			// TODO(gri) implement checks
+			if ctxt&inBreakable == 0 {
-		case token.CONTINUE:
+				check.errorf(s.Pos(), "break not in for, switch, or select statement")
-			// TODO(gri) implement checks
-		case token.GOTO:
-			// TODO(gri) implement checks
-		case token.FALLTHROUGH:
-			if s.Label != nil {
-				check.invalidAST(s.Label.Pos(), "fallthrough statement cannot have label")
-				// ok to continue
 			}
-			if !fallthroughOk {
+		case token.CONTINUE:
+			if ctxt&inContinuable == 0 {
+				check.errorf(s.Pos(), "continue not in for statement")
+			}
+		case token.GOTO:
+			check.invalidAST(s.Pos(), "goto without label")
+		case token.FALLTHROUGH:
+			if ctxt&fallthroughOk == 0 {
 				check.errorf(s.Pos(), "fallthrough statement out of place")
 			}
 		default:
@@ -248,24 +266,27 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 
 	case *ast.BlockStmt:
 		check.openScope(s)
-		check.stmtList(s.List, false)
+		check.stmtList(inner, s.List)
 		check.closeScope()
 
 	case *ast.IfStmt:
 		check.openScope(s)
-		check.optionalStmt(s.Init)
+		check.initStmt(s.Init)
 		var x operand
 		check.expr(&x, s.Cond)
 		if x.mode != invalid && !isBoolean(x.typ) {
 			check.errorf(s.Cond.Pos(), "non-boolean condition in if statement")
 		}
-		check.stmt(s.Body, false)
+		check.stmt(inner, s.Body)
-		check.optionalStmt(s.Else)
+		if s.Else != nil {
+			check.stmt(inner, s.Else)
+		}
 		check.closeScope()
 
 	case *ast.SwitchStmt:
+		inner |= inBreakable
 		check.openScope(s)
-		check.optionalStmt(s.Init)
+		check.initStmt(s.Init)
 		var x operand
 		tag := s.Tag
 		if tag == nil {
@@ -326,15 +347,20 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 				}
 			}
 			check.openScope(clause)
-			check.stmtList(clause.Body, i+1 < len(s.Body.List))
+			inner := inner
+			if i+1 < len(s.Body.List) {
+				inner |= fallthroughOk
+			}
+			check.stmtList(inner, clause.Body)
 			check.closeScope()
 		}
 		check.closeScope()
 
 	case *ast.TypeSwitchStmt:
+		inner |= inBreakable
 		check.openScope(s)
 		defer check.closeScope()
-		check.optionalStmt(s.Init)
+		check.initStmt(s.Init)
 
 		// A type switch guard must be of the form:
 		//
@@ -420,7 +446,7 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 				check.declareObj(check.topScope, nil, obj)
 				check.recordImplicit(clause, obj)
 			}
-			check.stmtList(clause.Body, false)
+			check.stmtList(inner, clause.Body)
 			check.closeScope()
 		}
 		// If a lhs variable was declared but there were no case clauses, make sure
@@ -434,6 +460,7 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 		}
 
 	case *ast.SelectStmt:
+		inner |= inBreakable
 		check.multipleDefaults(s.Body.List)
 		for _, s := range s.Body.List {
 			clause, _ := s.(*ast.CommClause)
@@ -441,14 +468,17 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 				continue // error reported before
 			}
 			check.openScope(clause)
-			check.optionalStmt(clause.Comm) // TODO(gri) check correctness of c.Comm (must be Send/RecvStmt)
+			if s := clause.Comm; s != nil {
-			check.stmtList(clause.Body, false)
+				check.stmt(inner, s) // TODO(gri) check correctness of c.Comm (must be Send/RecvStmt)
+			}
+			check.stmtList(inner, clause.Body)
 			check.closeScope()
 		}
 
 	case *ast.ForStmt:
+		inner |= inBreakable | inContinuable
 		check.openScope(s)
-		check.optionalStmt(s.Init)
+		check.initStmt(s.Init)
 		if s.Cond != nil {
 			var x operand
 			check.expr(&x, s.Cond)
@@ -456,11 +486,12 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 				check.errorf(s.Cond.Pos(), "non-boolean condition in for statement")
 			}
 		}
-		check.optionalStmt(s.Post)
+		check.initStmt(s.Post)
-		check.stmt(s.Body, false)
+		check.stmt(inner, s.Body)
 		check.closeScope()
 
 	case *ast.RangeStmt:
+		inner |= inBreakable | inContinuable
 		check.openScope(s)
 		defer check.closeScope()
 
@@ -472,7 +503,7 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 			// if we don't have a declaration, we can still check the loop's body
 			// (otherwise we can't because we are missing the declared variables)
 			if !decl {
-				check.stmt(s.Body, false)
+				check.stmt(inner, s.Body)
 			}
 			return
 		}
@@ -516,7 +547,7 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 			check.errorf(x.pos(), "cannot range over %s", &x)
 			// if we don't have a declaration, we can still check the loop's body
 			if !decl {
-				check.stmt(s.Body, false)
+				check.stmt(inner, s.Body)
 			}
 			return
 		}
@@ -578,7 +609,7 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 			}
 		}
 
-		check.stmt(s.Body, false)
+		check.stmt(inner, s.Body)
 
 	default:
 		check.errorf(s.Pos(), "invalid statement")

go/types/testdata/stmt0.src

@@ -180,6 +180,133 @@ func defers() {
 	defer len /* ERROR "defer discards result" */ (c)
 }
 
+func breaks() {
+	var x, y int
+
+	break /* ERROR "break" */
+	{
+		break /* ERROR "break" */
+	}
+	if x < y {
+		break /* ERROR "break" */
+	}
+
+	switch x {
+	case 0:
+		break
+	case 1:
+		if x == y {
+			break
+		}
+	default:
+		break
+		break
+	}
+
+	var z interface{}
+	switch z.(type) {
+	case int:
+		break
+	}
+
+	for {
+		break
+	}
+
+	var a []int
+	for _ = range a {
+		break
+	}
+
+	for {
+		if x == y {
+			break
+		}
+	}
+
+	var ch chan int
+	select {
+	case <-ch:
+		break
+	}
+
+	select {
+	case <-ch:
+		if x == y {
+			break
+		}
+	default:
+		break
+	}
+}
+
+func continues() {
+	var x, y int
+
+	continue /* ERROR "continue" */
+	{
+		continue /* ERROR "continue" */
+	}
+
+	if x < y {
+		continue /* ERROR "continue" */
+	}
+
+	switch x {
+	case 0:
+		continue /* ERROR "continue" */
+	}
+
+	var z interface{}
+	switch z.(type) {
+	case int:
+		continue /* ERROR "continue" */
+	}
+
+	var ch chan int
+	select {
+	case <-ch:
+		continue /* ERROR "continue" */
+	}
+
+	for i := 0; i < 10; i++ {
+		continue
+		if x < y {
+			continue
+			break
+		}
+		switch x {
+		case y:
+			continue
+		default:
+			break
+		}
+		select {
+		case <-ch:
+			continue
+		}
+	}
+
+	var a []int
+	for _ = range a {
+		continue
+		if x < y {
+			continue
+			break
+		}
+		switch x {
+		case y:
+			continue
+		default:
+			break
+		}
+		select {
+		case <-ch:
+			continue
+		}
+	}
+}
+
 func switches0() {
 	var x int
 
@@ -263,6 +390,14 @@ func switches1() {
 	default:
 		L6: L7: L8: fallthrough /* ERROR "fallthrough statement out of place" */
 	}
+
+	switch x {
+	case 0:
+		{
+			fallthrough /* ERROR "fallthrough statement out of place" */
+		}
+	default:
+	}
 }
 
 type I interface {
@@ -472,10 +607,10 @@ func rangeloops() {
 func labels0() {
 	L0:
 	L1:
-	L1 /* ERROR "redeclared" */:
+	L1:
 	if true {
 		L2:
-		L0 /* ERROR "redeclared" */:
+		L0:
 	}
 	_ = func() {
 		L0: