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 <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>

diff --git a/src/cmd/compile/internal/noder/irgen.go b/src/cmd/compile/internal/noder/irgen.go
index 628c0f54fc76f64d7b6c0bf83ff66038e161bba3..e45a204867d8c1df206750983cfb5b0e727a7740 100644 (file)
--- 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 c4c2db5f78a2d03fdba081786c4d1c4f0c84e790..15b1bf7b9fe51ab21f0eec58168bca99b49019b1 100644 (file)
--- 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 a739b1114dc292613423bb9f727f8867e828b44f..a22ea5d12fc1ee58dfd0cbbd72acb52c8de3e84d 100644 (file)
--- 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 e00f73ce99719fe13a6054246b1fbf646a8693fc..74d4164ba9d1ba3d0e669336b97a39b9bb1e493e 100644 (file)
--- 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 {