From 666d736ecb8afbe1aeb1d15f3958b70af2173510 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Fri, 24 Jun 2022 19:11:52 -0700 Subject: [PATCH] cmd/compile: do branch/label checks only once The previous change implemented the missing fallthrough checking in the parser. Therefore we can now disable the duplicate check in the type checker: - rename (types2.Config.)IngoreLabels to IgnoreBranches to more accurately reflect its functionality - now also ignore break/continue/fallthroughs, not just labels The IgnoreBranches flag only exists for types2, for use with the compiler. There's no need to port this code to go/types. Note: An alternative (and perhaps better) approach would be to not use the the parser's CheckBranches mode and instead enable (i.e. not disable) the branch/label checking in the type checker. However, this requires a bit more work because the type checker's error messages about goto's jumping over variables don't have access to the variable names, which are desired in the error messages. Fixes #51456. Change-Id: Ib2e71e811d4e84e4895b729646e879fd43b12dcd Reviewed-on: https://go-review.googlesource.com/c/go/+/414135 Reviewed-by: Ian Lance Taylor Reviewed-by: Robert Griesemer --- src/cmd/compile/internal/noder/irgen.go | 2 +- src/cmd/compile/internal/noder/noder.go | 4 +--- src/cmd/compile/internal/types2/api.go | 5 ++--- src/cmd/compile/internal/types2/stmt.go | 17 ++++++----------- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/cmd/compile/internal/noder/irgen.go b/src/cmd/compile/internal/noder/irgen.go index 628c0f54fc..e45a204867 100644 --- a/src/cmd/compile/internal/noder/irgen.go +++ b/src/cmd/compile/internal/noder/irgen.go @@ -41,7 +41,7 @@ func checkFiles(noders []*noder) (posMap, *types2.Package, *types2.Info) { conf := types2.Config{ Context: ctxt, GoVersion: base.Flag.Lang, - IgnoreLabels: true, // parser already checked via syntax.CheckBranches mode + IgnoreBranchErrors: 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) diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go index c4c2db5f78..15b1bf7b9f 100644 --- a/src/cmd/compile/internal/noder/noder.go +++ b/src/cmd/compile/internal/noder/noder.go @@ -27,8 +27,6 @@ import ( func LoadPackage(filenames []string) { base.Timer.Start("fe", "parse") - mode := syntax.CheckBranches - // Limit the number of simultaneously open files. sem := make(chan struct{}, runtime.GOMAXPROCS(0)+10) @@ -58,7 +56,7 @@ func LoadPackage(filenames []string) { } defer f.Close() - p.file, _ = syntax.Parse(fbase, f, p.error, p.pragma, mode) // errors are tracked via p.error + p.file, _ = syntax.Parse(fbase, f, p.error, p.pragma, syntax.CheckBranches) // errors are tracked via p.error }() } }() diff --git a/src/cmd/compile/internal/types2/api.go b/src/cmd/compile/internal/types2/api.go index a739b1114d..a22ea5d12f 100644 --- a/src/cmd/compile/internal/types2/api.go +++ b/src/cmd/compile/internal/types2/api.go @@ -128,9 +128,8 @@ type Config struct { // Do not use casually! FakeImportC bool - // If IgnoreLabels is set, correct label use is not checked. - // TODO(gri) Consolidate label checking and remove this flag. - IgnoreLabels bool + // If IgnoreBranchErrors is set, branch/label errors are ignored. + IgnoreBranchErrors bool // If CompilerErrorMessages is set, errors are reported using // cmd/compile error strings to match $GOROOT/test errors. diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go index e00f73ce99..74d4164ba9 100644 --- a/src/cmd/compile/internal/types2/stmt.go +++ b/src/cmd/compile/internal/types2/stmt.go @@ -41,7 +41,7 @@ func (check *Checker) funcBody(decl *declInfo, name string, sig *Signature, body check.stmtList(0, body.List) - if check.hasLabel && !check.conf.IgnoreLabels { + if check.hasLabel && !check.conf.IgnoreBranchErrors { check.labels(body) } @@ -504,22 +504,17 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) { check.hasLabel = true break // checked in 2nd pass (check.labels) } + if check.conf.IgnoreBranchErrors { + break + } switch s.Tok { case syntax.Break: if ctxt&breakOk == 0 { - 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") - } + check.error(s, "break not in for, switch, or select statement") } case syntax.Continue: if ctxt&continueOk == 0 { - if check.conf.CompilerErrorMessages { - check.error(s, "continue is not in a loop") - } else { - check.error(s, "continue not in for statement") - } + check.error(s, "continue not in for statement") } case syntax.Fallthrough: if ctxt&fallthroughOk == 0 { -- 2.48.1