Skip to content

Commit

Permalink
cmd/compile: pick position of implicit break statements more carefully
Browse files Browse the repository at this point in the history
The previous version used the position of the switch statement,
which makes for potentially jumpy stepping and introduces a large
number of statements repeating the line (tricky for inserting
breaks).  It also shared a single OBREAK node and this was not
really a syntax "tree".

This improves both the nostmt test (by 6 lines) and
reduces the total badness score from dwarf-goodness (by about 200).

Change-Id: I1f71b231a26f152bdb6ce9bc8f95828bb222f665
Reviewed-on: https://go-review.googlesource.com/c/go/+/188218
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
  • Loading branch information
dr2chase committed Oct 3, 2019
1 parent f7f85bd commit 6139019
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/cmd/compile/internal/gc/swt.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ func walkExprSwitch(sw *Node) {
exprname: cond,
}

br := nod(OBREAK, nil, nil)
var defaultGoto *Node
var body Nodes
for _, ncase := range sw.List.Slice() {
Expand All @@ -290,13 +289,17 @@ func walkExprSwitch(sw *Node) {
// Process body.
body.Append(npos(ncase.Pos, nodSym(OLABEL, nil, label)))
body.Append(ncase.Nbody.Slice()...)
if !hasFall(ncase.Nbody.Slice()) {
if fall, pos := hasFall(ncase.Nbody.Slice()); !fall {
br := nod(OBREAK, nil, nil)
br.Pos = pos
body.Append(br)
}
}
sw.List.Set(nil)

if defaultGoto == nil {
br := nod(OBREAK, nil, nil)
br.Pos = br.Pos.WithNotStmt()
defaultGoto = br
}

Expand Down Expand Up @@ -469,7 +472,7 @@ func allCaseExprsAreSideEffectFree(sw *Node) bool {
}

// hasFall reports whether stmts ends with a "fallthrough" statement.
func hasFall(stmts []*Node) bool {
func hasFall(stmts []*Node) (bool, src.XPos) {
// Search backwards for the index of the fallthrough
// statement. Do not assume it'll be in the last
// position, since in some cases (e.g. when the statement
Expand All @@ -480,7 +483,10 @@ func hasFall(stmts []*Node) bool {
for i >= 0 && stmts[i].Op == OVARKILL {
i--
}
return i >= 0 && stmts[i].Op == OFALL
if i < 0 {
return false, src.NoXPos
}
return stmts[i].Op == OFALL, stmts[i].Pos
}

// walkTypeSwitch generates an AST that implements sw, where sw is a
Expand Down

0 comments on commit 6139019

Please sign in to comment.