From be48aa3f3a16006ab31c424487af352ca374afed Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 12 Oct 2016 19:51:02 -0700 Subject: [PATCH] cmd/cover: handle gotos If a labeled statement is the target of a goto, we must treat it as the boundary of a new basic block, but only if it is not already the boundary of a basic block such as a labeled for loop. Fixes #16624 Now reports 100% coverage for the test in the issue. Change-Id: If118bb6ff53a96c738e169d92c03cb3ce97bad0e Reviewed-on: https://go-review.googlesource.com/30977 Reviewed-by: Ian Lance Taylor Reviewed-by: Robert Griesemer --- src/cmd/cover/cover.go | 43 ++++++++++++++++++++++++++++++---- src/cmd/cover/testdata/test.go | 19 +++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/cmd/cover/cover.go b/src/cmd/cover/cover.go index e48d811ead..222737571f 100644 --- a/src/cmd/cover/cover.go +++ b/src/cmd/cover/cover.go @@ -481,10 +481,35 @@ func (f *File) addCounters(pos, blockEnd token.Pos, list []ast.Stmt, extendToClo var last int end := blockEnd for last = 0; last < len(list); last++ { - end = f.statementBoundary(list[last]) - if f.endsBasicSourceBlock(list[last]) { - extendToClosingBrace = false // Block is broken up now. + stmt := list[last] + end = f.statementBoundary(stmt) + if f.endsBasicSourceBlock(stmt) { + // If it is a labeled statement, we need to place a counter between + // the label and its statement because it may be the target of a goto + // and thus start a basic block. That is, given + // foo: stmt + // we need to create + // foo: ; stmt + // and mark the label as a block-terminating statement. + // The result will then be + // foo: COUNTER[n]++; stmt + // However, we can't do this if the labeled statement is already + // a control statement, such as a labeled for. + if label, isLabel := stmt.(*ast.LabeledStmt); isLabel && !f.isControl(label.Stmt) { + newLabel := *label + newLabel.Stmt = &ast.EmptyStmt{ + Semicolon: label.Stmt.Pos(), + Implicit: true, + } + end = label.Pos() // Previous block ends before the label. + list[last] = &newLabel + // Open a gap and drop in the old statement, now without a label. + list = append(list, nil) + copy(list[last+1:], list[last:]) + list[last+1] = label.Stmt + } last++ + extendToClosingBrace = false // Block is broken up now. break } } @@ -603,7 +628,7 @@ func (f *File) endsBasicSourceBlock(s ast.Stmt) bool { case *ast.IfStmt: return true case *ast.LabeledStmt: - return f.endsBasicSourceBlock(s.Stmt) + return true // A goto may branch here, starting a new basic block. case *ast.RangeStmt: return true case *ast.SwitchStmt: @@ -627,6 +652,16 @@ func (f *File) endsBasicSourceBlock(s ast.Stmt) bool { return found } +// isControl reports whether s is a control statement that, if labeled, cannot be +// separated from its label. +func (f *File) isControl(s ast.Stmt) bool { + switch s.(type) { + case *ast.ForStmt, *ast.RangeStmt, *ast.SwitchStmt, *ast.SelectStmt, *ast.TypeSwitchStmt: + return true + } + return false +} + // funcLitFinder implements the ast.Visitor pattern to find the location of any // function literal in a subtree. type funcLitFinder token.Pos diff --git a/src/cmd/cover/testdata/test.go b/src/cmd/cover/testdata/test.go index 095ce1d909..71cb115331 100644 --- a/src/cmd/cover/testdata/test.go +++ b/src/cmd/cover/testdata/test.go @@ -25,6 +25,7 @@ func testAll() { testPanic() testEmptySwitches() testFunctionLiteral() + testGoto() } // The indexes of the counters in testPanic are known to main.go @@ -247,6 +248,24 @@ func testFunctionLiteral() { } } +func testGoto() { + for i := 0; i < 2; i++ { + if i == 0 { + goto Label + } + check(LINE, 1) + Label: + check(LINE, 2) + } + // Now test that we don't inject empty statements + // between a label and a loop. +loop: + for { + check(LINE, 1) + break loop + } +} + // This comment shouldn't appear in generated go code. func haha() { // Needed for cover to add counter increment here. -- 2.48.1