From 6139019efaa3faa9ec94a57ab8c15b726d516664 Mon Sep 17 00:00:00 2001 From: David Chase Date: Wed, 25 Sep 2019 15:20:10 -0400 Subject: [PATCH] cmd/compile: pick position of implicit break statements more carefully 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 Reviewed-by: Jeremy Faller --- src/cmd/compile/internal/gc/swt.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go index a97e9735da0b0..1381cdacba930 100644 --- a/src/cmd/compile/internal/gc/swt.go +++ b/src/cmd/compile/internal/gc/swt.go @@ -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() { @@ -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 } @@ -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 @@ -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