From c92471fb8564bcdcbad7b3e6d7acc4c0ae9aea50 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 30 Sep 2013 14:03:33 -0700 Subject: [PATCH] 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 | 50 +++++++++++++++++++----------------- go/types/testdata/labels.src | 45 +++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 27 deletions(-) diff --git a/go/types/labels.go b/go/types/labels.go index da988c86..65b39e44 100644 --- a/go/types/labels.go +++ b/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) - if t == nil { - check.errorf(s.Label.Pos(), "break label not declared: %s", name) - return + valid := false + if t := b.enclosingTarget(name); t != nil { + switch t.Stmt.(type) { + case *ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt, *ast.ForStmt, *ast.RangeStmt: + valid = true + } } - switch t.Stmt.(type) { - case *ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt, *ast.ForStmt, *ast.RangeStmt: - // ok - default: + if !valid { 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) - if t == nil { - check.errorf(s.Label.Pos(), "continue label not declared: %s", name) - return + valid := false + if t := b.enclosingTarget(name); t != nil { + switch t.Stmt.(type) { + case *ast.ForStmt, *ast.RangeStmt: + valid = true + } } - switch t.Stmt.(type) { - case *ast.ForStmt, *ast.RangeStmt: - // ok - default: + if !valid { 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 - // become forward jumps in the current block. - fwdJumps = append(fwdJumps, check.blockBranches(all, b, lstmt, s.List)...) + blockBranches(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) diff --git a/go/types/testdata/labels.src b/go/types/testdata/labels.src index f4fcdcec..53b8ed36 100644 --- a/go/types/testdata/labels.src +++ b/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 + } } \ No newline at end of file