go.tools/go/types: improved error messages for invalid labels

In general, if a break or continue label is not found, we don't
know if a correspondingly named label was not declared, was declared
but is not visible, or will be declared (and won't be visible).
Complain about "invalid" rather than "not declared" label.

Added more tests.

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

go/types/labels.go

@@ -114,6 +114,12 @@ func (check *checker) blockBranches(all *Scope, parent *block, lstmt *ast.Labele
 		return false
 	}
 
+	blockBranches := func(lstmt *ast.LabeledStmt, list []ast.Stmt) {
+		// Unresolved forward jumps inside the nested block
+		// become forward jumps in the current block.
+		fwdJumps = append(fwdJumps, check.blockBranches(all, b, lstmt, list)...)
+	}
+
 	var stmtBranches func(ast.Stmt)
 	stmtBranches = func(s ast.Stmt) {
 		switch s := s.(type) {
@@ -172,38 +178,36 @@ func (check *checker) blockBranches(all *Scope, parent *block, lstmt *ast.Labele
 				// spec: "If there is a label, it must be that of an enclosing
 				// "for", "switch", or "select" statement, and that is the one
 				// whose execution terminates."
-				t := b.enclosingTarget(name)
+				valid := false
-				if t == nil {
+				if t := b.enclosingTarget(name); t != nil {
-					check.errorf(s.Label.Pos(), "break label not declared: %s", name)
+					switch t.Stmt.(type) {
-					return
+					case *ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt, *ast.ForStmt, *ast.RangeStmt:
+						valid = true
+					}
 				}
-				switch t.Stmt.(type) {
+				if !valid {
-				case *ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt, *ast.ForStmt, *ast.RangeStmt:
-					// ok
-				default:
 					check.errorf(s.Label.Pos(), "invalid break label %s", name)
-					// continue and mark label as used to avoid another error
+					return
 				}
 
 			case token.CONTINUE:
 				// spec: "If there is a label, it must be that of an enclosing
 				// "for" statement, and that is the one whose execution advances."
-				t := b.enclosingTarget(name)
+				valid := false
-				if t == nil {
+				if t := b.enclosingTarget(name); t != nil {
-					check.errorf(s.Label.Pos(), "continue label not declared: %s", name)
+					switch t.Stmt.(type) {
-					return
+					case *ast.ForStmt, *ast.RangeStmt:
+						valid = true
+					}
 				}
-				switch t.Stmt.(type) {
+				if !valid {
-				case *ast.ForStmt, *ast.RangeStmt:
-					// ok
-				default:
 					check.errorf(s.Label.Pos(), "invalid continue label %s", name)
-					// continue and mark label as used to avoid another error
+					return
 				}
 
 			case token.GOTO:
 				if b.gotoTarget(name) == nil {
-					// label may be declared later - add to forward jumps
+					// label may be declared later - add branch to forward jumps
 					fwdJumps = append(fwdJumps, s)
 					return
 				}
@@ -224,9 +228,7 @@ func (check *checker) blockBranches(all *Scope, parent *block, lstmt *ast.Labele
 			}
 
 		case *ast.BlockStmt:
-			// Unresolved forward jumps inside the nested block
+			blockBranches(lstmt, s.List)
-			// become forward jumps in the current block.
-			fwdJumps = append(fwdJumps, check.blockBranches(all, b, lstmt, s.List)...)
 
 		case *ast.IfStmt:
 			stmtBranches(s.Body)
@@ -235,7 +237,7 @@ func (check *checker) blockBranches(all *Scope, parent *block, lstmt *ast.Labele
 			}
 
 		case *ast.CaseClause:
-			fwdJumps = append(fwdJumps, check.blockBranches(all, b, nil, s.Body)...)
+			blockBranches(nil, s.Body)
 
 		case *ast.SwitchStmt:
 			stmtBranches(s.Body)
@@ -244,7 +246,7 @@ func (check *checker) blockBranches(all *Scope, parent *block, lstmt *ast.Labele
 			stmtBranches(s.Body)
 
 		case *ast.CommClause:
-			fwdJumps = append(fwdJumps, check.blockBranches(all, b, nil, s.Body)...)
+			blockBranches(nil, s.Body)
 
 		case *ast.SelectStmt:
 			stmtBranches(s.Body)

go/types/testdata/labels.src

@@ -35,12 +35,24 @@ L6 /* ERROR "label L6 already declared" */ :
 L7:
 	for {
 		break L7
+		break L8 /* ERROR "invalid break label L8" */
+	}
+
+// A label must be directly associated with a switch, select, or
+// for statement; it cannot be the label of a labeled statement.
+
+L7a /* ERROR "declared but not used" */ : L7b:
+	for {
+		break L7a /* ERROR "invalid break label L7a" */
+		continue L7a /* ERROR "invalid continue label L7a" */
+		continue L7b
 	}
 
 L8:
 	for {
 		if x == 21 {
 			continue L8
+			continue L7 /* ERROR "invalid continue label L7" */
 		}
 	}
 
@@ -55,6 +67,16 @@ L10:
 	select {
 	default:
 		break L10
+		break L9 /* ERROR "invalid break label L9" */
+	}
+
+	goto L10a
+L10a: L10b:
+	select {
+	default:
+		break L10a /* ERROR "invalid break label L10a" */
+		break L10b
+		continue L10b /* ERROR "invalid continue label L10b" */
 	}
 }
 
@@ -121,10 +143,10 @@ L5:
 
 	for {
 		if x == 19 {
-			break L1 /* ERROR "break label not declared: L1" */
+			break L1 /* ERROR "invalid break label L1" */
 		}
 		if x == 20 {
-			continue L1 /* ERROR "continue label not declared: L1" */
+			continue L1 /* ERROR "invalid continue label L1" */
 		}
 		if x == 21 {
 			goto L1
@@ -135,10 +157,27 @@ L5:
 // Additional tests not in the original files.
 
 func f2() {
-L1:
+L1 /* ERROR "label L1 declared but not used" */ :
 	if x == 0 {
 		for {
 			continue L1 /* ERROR "invalid continue label L1" */
 		}
 	}
+}
+
+func f3() {
+L1:
+L2:
+L3:
+	for {
+		break L1 /* ERROR "invalid break label L1" */
+		break L2 /* ERROR "invalid break label L2" */
+		break L3
+		continue L1 /* ERROR "invalid continue label L1" */
+		continue L2 /* ERROR "invalid continue label L2" */
+		continue L3
+		goto L1
+		goto L2
+		goto L3
+	}
 }