[dev.typeparams] cmd/compile/internal/types2: adjust errors in branch checking code, fix a bug

The types2.Config.IgnoreBranches flag mistakenly excluded a
set of label-unrelated branch checks. After fixing this and
also adjusting some error messages to match the existing
compiler errors, more errorcheck tests pass now with the -G
option.

Renamed IngnoreBranches to IgnoreLabels since its controlling
label checks, not all branch statement (such as continue, etc)
checks.

Change-Id: I0819f56eb132ce76c9a9628d8942af756691065a
Reviewed-on: https://go-review.googlesource.com/c/go/+/285652
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

src/cmd/compile/internal/noder/irgen.go

@@ -35,7 +35,7 @@ func check2(noders []*noder) {
 	// typechecking
 	conf := types2.Config{
 		InferFromConstraints:  true,
-		IgnoreBranches:        true, // parser already checked via syntax.CheckBranches mode
+		IgnoreLabels:          true, // parser already checked via syntax.CheckBranches mode
 		CompilerErrorMessages: true, // use error strings matching existing compiler errors
 		Error: func(err error) {
 			terr := err.(types2.Error)

src/cmd/compile/internal/types2/api.go

@@ -119,10 +119,9 @@ type Config struct {
 	//          Do not use casually!
 	FakeImportC bool
 
-	// If IgnoreBranches is set, errors related to incorrectly placed
+	// If IgnoreLabels is set, correct label use is not checked.
-	// labels, gotos, break, continue, and fallthrough statements are
+	// TODO(gri) Consolidate label checking and remove this flag.
-	// ignored.
+	IgnoreLabels bool
-	IgnoreBranches bool
 
 	// If CompilerErrorMessages is set, errors are reported using
 	// cmd/compile error strings to match $GOROOT/test errors.

src/cmd/compile/internal/types2/stmt.go

@@ -41,8 +41,7 @@ func (check *Checker) funcBody(decl *declInfo, name string, sig *Signature, body
 
 	check.stmtList(0, body.List)
 
-	if check.hasLabel {
+	if check.hasLabel && !check.conf.IgnoreLabels {
-		assert(!check.conf.IgnoreBranches)
 		check.labels(body)
 	}
 
@@ -321,7 +320,7 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {
 		check.declStmt(s.DeclList)
 
 	case *syntax.LabeledStmt:
-		check.hasLabel = !check.conf.IgnoreBranches
+		check.hasLabel = true
 		check.stmt(ctxt, s.Stmt)
 
 	case *syntax.ExprStmt:
@@ -446,22 +445,26 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {
 		}
 
 	case *syntax.BranchStmt:
-		if check.conf.IgnoreBranches {
-			break
-		}
-
 		if s.Label != nil {
 			check.hasLabel = true
-			return // checked in 2nd pass (check.labels)
+			break // checked in 2nd pass (check.labels)
 		}
 		switch s.Tok {
 		case syntax.Break:
 			if ctxt&breakOk == 0 {
-				check.error(s, "break not in for, switch, or select statement")
+				if check.conf.CompilerErrorMessages {
+					check.error(s, "break is not in a loop, switch, or select statement")
+				} else {
+					check.error(s, "break not in for, switch, or select statement")
+				}
 			}
 		case syntax.Continue:
 			if ctxt&continueOk == 0 {
-				check.error(s, "continue not in for statement")
+				if check.conf.CompilerErrorMessages {
+					check.error(s, "continue is not in a loop")
+				} else {
+					check.error(s, "continue not in for statement")
+				}
 			}
 		case syntax.Fallthrough:
 			if ctxt&fallthroughOk == 0 {

test/run.go

@@ -1937,13 +1937,11 @@ var excluded = map[string]bool{
 	"initializerr.go": true, // types2 reports extra errors
 	"linkname2.go":    true, // error reported by noder (not running for types2 errorcheck test)
 	"shift1.go":       true, // issue #42989
-	"switch4.go":      true, // error reported by noder (not running for types2 errorcheck test)
 	"typecheck.go":    true, // invalid function is not causing errors when called
 
 	"fixedbugs/bug176.go":    true, // types2 reports all errors (pref: types2)
 	"fixedbugs/bug193.go":    true, // types2 bug: shift error not reported (fixed in go/types)
 	"fixedbugs/bug195.go":    true, // types2 reports slightly different (but correct) bugs
-	"fixedbugs/bug213.go":    true, // error reported by noder (not running for types2 errorcheck test)
 	"fixedbugs/bug228.go":    true, // types2 not run after syntax errors
 	"fixedbugs/bug231.go":    true, // types2 bug? (same error reported twice)
 	"fixedbugs/bug255.go":    true, // types2 reports extra errors
@@ -1986,7 +1984,6 @@ var excluded = map[string]bool{
 	"fixedbugs/issue4232.go":   true, // types2 reports (correct) extra errors
 	"fixedbugs/issue4452.go":   true, // types2 reports (correct) extra errors
 	"fixedbugs/issue5609.go":   true, // types2 needs a better error message
-	"fixedbugs/issue6500.go":   true, // error reported by noder (not running for types2 errorcheck test)
 	"fixedbugs/issue6889.go":   true, // types2 can handle this without constant overflow
 	"fixedbugs/issue7525.go":   true, // types2 reports init cycle error on different line - ok otherwise
 	"fixedbugs/issue7525b.go":  true, // types2 reports init cycle error on different line - ok otherwise