Skip to content

Commit

Permalink
cmd/cover: fix counting of blocks split by goto statements
Browse files Browse the repository at this point in the history
When adding coverage counters to a block, the block's statement list is
mutated. CL 77150 removed the part where the mutated list is assigned
back to its parent node; this was confusing ast.Walk, which would then
lose its place and stop walking the current block, dropping counters in
the process.

This change has addCounters make a copy of the list before mutating
it, so that the original list doesn't change under Walk's feet.

Fix #32200

Change-Id: Ia3b67d8cee860ceb7caf8748cb7a80ff9c6276e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/179581
Reviewed-by: Rob Pike <[email protected]>
  • Loading branch information
adg committed Jun 3, 2019
1 parent ce656af commit 98100c5
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/cmd/cover/cover.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,9 @@ func (f *File) addCounters(pos, insertPos, blockEnd token.Pos, list []ast.Stmt,
f.edit.Insert(f.offset(insertPos), f.newCounter(insertPos, blockEnd, 0)+";")
return
}
// Make a copy of the list, as we may mutate it and should leave the
// existing list intact.
list = append([]ast.Stmt(nil), list...)
// We have a block (statement list), but it may have several basic blocks due to the
// appearance of statements that affect the flow of control.
for {
Expand Down
4 changes: 4 additions & 0 deletions src/cmd/cover/testdata/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ func testBlockRun() {

func testSwitch() {
for i := 0; i < 5; func() { i++; check(LINE, 5) }() {
goto label2
label1:
goto label1
label2:
switch i {
case 0:
check(LINE, 1)
Expand Down

0 comments on commit 98100c5

Please sign in to comment.