From a50962131acb7def8241cd5e78b99746c6e52771 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 18 Apr 2017 20:44:54 -0700 Subject: [PATCH] cmd/compile/internal/syntax: compute BranchStmt.Target statements - Add new BranchStmt.Target field: It's the destination for break, continue, or goto statements. - When parsing with CheckBranches enabled, set the BranchStmt.Target field. We get the information practically for free from the branch checker, so keep it for further use. - Fix a couple of comments. - This could use a test, but the new Target field is currently not used, and writing a test is tedious w/o a general tree visitor. Do it later. For now, visually verified output from syntax dump. Change-Id: Id691d89efab514ad885e19ac9759506106579520 Reviewed-on: https://go-review.googlesource.com/40988 Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/syntax/branches.go | 70 +++++++++++-------- .../compile/internal/syntax/dumper_test.go | 2 +- src/cmd/compile/internal/syntax/nodes.go | 6 ++ 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/cmd/compile/internal/syntax/branches.go b/src/cmd/compile/internal/syntax/branches.go index 9d21f93d5c..5fecdd6551 100644 --- a/src/cmd/compile/internal/syntax/branches.go +++ b/src/cmd/compile/internal/syntax/branches.go @@ -9,7 +9,7 @@ import ( "fmt" ) -// TODO(gri) do this while parsing instead of in a separate pass? +// TODO(gri) consider making this part of the parser code // checkBranches checks correct use of labels and branch // statements (break, continue, goto) in a function body. @@ -25,7 +25,7 @@ func checkBranches(body *BlockStmt, errh ErrorHandler) { // scope of all labels in this body ls := &labelScope{errh: errh} - fwdGotos := ls.blockBranches(nil, 0, nil, body.Pos(), body.List) + fwdGotos := ls.blockBranches(nil, targets{}, nil, body.Pos(), body.List) // If there are any forward gotos left, no matching label was // found for them. Either those labels were never defined, or @@ -91,12 +91,12 @@ func (ls *labelScope) declare(b *block, s *LabeledStmt) *label { // gotoTarget returns the labeled statement matching the given name and // declared in block b or any of its enclosing blocks. The result is nil // if the label is not defined, or doesn't match a valid labeled statement. -func (ls *labelScope) gotoTarget(b *block, name string) *label { +func (ls *labelScope) gotoTarget(b *block, name string) *LabeledStmt { if l := ls.labels[name]; l != nil { l.used = true // even if it's not a valid target for ; b != nil; b = b.parent { if l.parent == b { - return l + return l.lstmt } } } @@ -121,17 +121,18 @@ func (ls *labelScope) enclosingTarget(b *block, name string) *LabeledStmt { return nil } -// context flags -const ( - breakOk = 1 << iota - continueOk -) +// targets describes the target statements within which break +// or continue statements are valid. +type targets struct { + breaks Stmt // *ForStmt, *SwitchStmt, *SelectStmt, or nil + continues *ForStmt // or nil +} // blockBranches processes a block's body starting at start and returns the // list of unresolved (forward) gotos. parent is the immediately enclosing -// block (or nil), context provides information about the enclosing statements, +// block (or nil), ctxt provides information about the enclosing statements, // and lstmt is the labeled statement asociated with this block, or nil. -func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledStmt, start src.Pos, body []Stmt) []*BranchStmt { +func (ls *labelScope) blockBranches(parent *block, ctxt targets, lstmt *LabeledStmt, start src.Pos, body []Stmt) []*BranchStmt { b := &block{parent: parent, start: start, lstmt: lstmt} var varPos src.Pos @@ -159,8 +160,10 @@ func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledS return false } - innerBlock := func(flags uint, start src.Pos, body []Stmt) { - fwdGotos = append(fwdGotos, ls.blockBranches(b, context|flags, lstmt, start, body)...) + innerBlock := func(ctxt targets, start src.Pos, body []Stmt) { + // Unresolved forward gotos from the inner block + // become forward gotos for the current block. + fwdGotos = append(fwdGotos, ls.blockBranches(b, ctxt, lstmt, start, body)...) } for _, stmt := range body { @@ -183,6 +186,7 @@ func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledS i := 0 for _, fwd := range fwdGotos { if fwd.Label.Value == name { + fwd.Target = s l.used = true if jumpsOverVarDecl(fwd) { ls.err( @@ -209,11 +213,15 @@ func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledS if s.Label == nil { switch s.Tok { case _Break: - if context&breakOk == 0 { + if t := ctxt.breaks; t != nil { + s.Target = t + } else { ls.err(s.Pos(), "break is not in a loop, switch, or select") } case _Continue: - if context&continueOk == 0 { + if t := ctxt.continues; t != nil { + s.Target = t + } else { ls.err(s.Pos(), "continue is not in a loop") } case _Fallthrough: @@ -234,12 +242,10 @@ func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledS // "for", "switch", or "select" statement, and that is the one // whose execution terminates." if t := ls.enclosingTarget(b, name); t != nil { - valid := false - switch t.Stmt.(type) { + switch t := t.Stmt.(type) { case *SwitchStmt, *SelectStmt, *ForStmt: - valid = true - } - if !valid { + s.Target = t + default: ls.err(s.Label.Pos(), "invalid break label %s", name) } } else { @@ -250,7 +256,9 @@ func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledS // spec: "If there is a label, it must be that of an enclosing // "for" statement, and that is the one whose execution advances." if t := ls.enclosingTarget(b, name); t != nil { - if _, ok := t.Stmt.(*ForStmt); !ok { + if t, ok := t.Stmt.(*ForStmt); ok { + s.Target = t + } else { ls.err(s.Label.Pos(), "invalid continue label %s", name) } } else { @@ -258,7 +266,9 @@ func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledS } case _Goto: - if ls.gotoTarget(b, name) == nil { + if t := ls.gotoTarget(b, name); t != nil { + s.Target = t + } else { // label may be declared later - add goto to forward gotos fwdGotos = append(fwdGotos, s) } @@ -275,27 +285,27 @@ func (ls *labelScope) blockBranches(parent *block, context uint, lstmt *LabeledS } case *BlockStmt: - // Unresolved forward gotos from the nested block - // become forward gotos for the current block. - innerBlock(0, s.Pos(), s.List) + innerBlock(ctxt, s.Pos(), s.List) case *IfStmt: - innerBlock(0, s.Then.Pos(), s.Then.List) + innerBlock(ctxt, s.Then.Pos(), s.Then.List) if s.Else != nil { - innerBlock(0, s.Else.Pos(), []Stmt{s.Else}) + innerBlock(ctxt, s.Else.Pos(), []Stmt{s.Else}) } case *ForStmt: - innerBlock(breakOk|continueOk, s.Body.Pos(), s.Body.List) + innerBlock(targets{s, s}, s.Body.Pos(), s.Body.List) case *SwitchStmt: + inner := targets{s, ctxt.continues} for _, cc := range s.Body { - innerBlock(breakOk, cc.Pos(), cc.Body) + innerBlock(inner, cc.Pos(), cc.Body) } case *SelectStmt: + inner := targets{s, ctxt.continues} for _, cc := range s.Body { - innerBlock(breakOk, cc.Pos(), cc.Body) + innerBlock(inner, cc.Pos(), cc.Body) } } } diff --git a/src/cmd/compile/internal/syntax/dumper_test.go b/src/cmd/compile/internal/syntax/dumper_test.go index 1186193aba..02116f5aad 100644 --- a/src/cmd/compile/internal/syntax/dumper_test.go +++ b/src/cmd/compile/internal/syntax/dumper_test.go @@ -14,7 +14,7 @@ func TestDump(t *testing.T) { t.Skip("skipping test in short mode") } - ast, err := ParseFile(*src_, nil, nil, 0) + ast, err := ParseFile(*src_, nil, nil, CheckBranches) if err != nil { t.Fatal(err) } diff --git a/src/cmd/compile/internal/syntax/nodes.go b/src/cmd/compile/internal/syntax/nodes.go index ac9cc67451..7ab6df13c4 100644 --- a/src/cmd/compile/internal/syntax/nodes.go +++ b/src/cmd/compile/internal/syntax/nodes.go @@ -356,6 +356,12 @@ type ( BranchStmt struct { Tok token // Break, Continue, Fallthrough, or Goto Label *Name + // Target is the continuation of the control flow after executing + // the branch; it is computed by the parser if CheckBranches is set. + // Target is a *LabeledStmt for gotos, and a *SwitchStmt, *SelectStmt, + // or *ForStmt for breaks and continues, depending on the context of + // the branch. Target is not set for fallthroughs. + Target Stmt stmt } -- 2.48.1