]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/cover: don't assume duplicate positions are in order
authorIan Lance Taylor <iant@golang.org>
Wed, 13 Mar 2019 02:10:43 +0000 (19:10 -0700)
committerIan Lance Taylor <iant@golang.org>
Fri, 15 Mar 2019 04:00:43 +0000 (04:00 +0000)
Fixes #30746

Change-Id: I63f2d82f14eeaab6b14e956e21ddeec56fee025b
Reviewed-on: https://go-review.googlesource.com/c/go/+/167257
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
src/cmd/cover/cover.go
src/cmd/cover/cover_test.go

index 0348849578bb1ad0f436186b72d36964c11bbd6d..2394e5797744ebafabcf2450e53ea4437a3aa186 100644 (file)
@@ -646,20 +646,11 @@ func (f *File) addVariables(w io.Writer) {
        // - 32-bit starting line number
        // - 32-bit ending line number
        // - (16 bit ending column number << 16) | (16-bit starting column number).
-       var lastStart, lastEnd token.Position
        for i, block := range f.blocks {
                start := f.fset.Position(block.startByte)
                end := f.fset.Position(block.endByte)
 
-               // It is possible for positions to repeat when there is a
-               // line directive that does not specify column information
-               // and the input has not been passed through gofmt.
-               // See issue #27350 and TestHtmlUnformatted.
-               if samePos(start, lastStart) && samePos(end, lastEnd) {
-                       end.Column++
-               }
-               lastStart = start
-               lastEnd = end
+               start, end = dedup(start, end)
 
                fmt.Fprintf(w, "\t\t%d, %d, %#x, // [%d]\n", start.Line, end.Line, (end.Column&0xFFFF)<<16|(start.Column&0xFFFF), i)
        }
@@ -710,10 +701,39 @@ func isValidIdentifier(ident string) bool {
        return true
 }
 
-// samePos returns whether two positions have the same file/line/column.
-// We don't use p1 == p2 because token.Position also has an Offset field,
-// and when the input uses //line directives two Positions can have different
-// Offset values while having the same file/line/dolumn.
-func samePos(p1, p2 token.Position) bool {
-       return p1.Filename == p2.Filename && p1.Line == p2.Line && p1.Column == p2.Column
+// It is possible for positions to repeat when there is a line
+// directive that does not specify column information and the input
+// has not been passed through gofmt.
+// See issues #27530 and #30746.
+// Tests are TestHtmlUnformatted and TestLineDup.
+// We use a map to avoid duplicates.
+
+// pos2 is a pair of token.Position values, used as a map key type.
+type pos2 struct {
+       p1, p2 token.Position
+}
+
+// seenPos2 tracks whether we have seen a token.Position pair.
+var seenPos2 = make(map[pos2]bool)
+
+// dedup takes a token.Position pair and returns a pair that does not
+// duplicate any existing pair. The returned pair will have the Offset
+// fields cleared.
+func dedup(p1, p2 token.Position) (r1, r2 token.Position) {
+       key := pos2{
+               p1: p1,
+               p2: p2,
+       }
+
+       // We want to ignore the Offset fields in the map,
+       // since cover uses only file/line/column.
+       key.p1.Offset = 0
+       key.p2.Offset = 0
+
+       for seenPos2[key] {
+               key.p2.Column++
+       }
+       seenPos2[key] = true
+
+       return key.p1, key.p2
 }
index cf8f3d2384d8603d24883dbf4ae8a900e1006332..a53660f74434dd1f40e109900e723cd65f26069c 100644 (file)
@@ -40,16 +40,20 @@ var (
        htmlGolden = filepath.Join(testdata, "html", "html.golden")
 
        // Temporary files.
-       tmpTestMain  string
-       coverInput   string
-       coverOutput  string
-       htmlProfile  string
-       htmlHTML     string
-       htmlUDir     string
-       htmlU        string
-       htmlUTest    string
-       htmlUProfile string
-       htmlUHTML    string
+       tmpTestMain    string
+       coverInput     string
+       coverOutput    string
+       htmlProfile    string
+       htmlHTML       string
+       htmlUDir       string
+       htmlU          string
+       htmlUTest      string
+       htmlUProfile   string
+       htmlUHTML      string
+       lineDupDir     string
+       lineDupGo      string
+       lineDupTestGo  string
+       lineDupProfile string
 )
 
 var (
@@ -96,6 +100,10 @@ func TestMain(m *testing.M) {
        htmlUTest = filepath.Join(htmlUDir, "htmlunformatted_test.go")
        htmlUProfile = filepath.Join(htmlUDir, "htmlunformatted.cov")
        htmlUHTML = filepath.Join(htmlUDir, "htmlunformatted.html")
+       lineDupDir = filepath.Join(dir, "linedup")
+       lineDupGo = filepath.Join(lineDupDir, "linedup.go")
+       lineDupTestGo = filepath.Join(lineDupDir, "linedup_test.go")
+       lineDupProfile = filepath.Join(lineDupDir, "linedup.out")
 
        status := m.Run()
 
@@ -484,6 +492,72 @@ lab:
        run(cmd, t)
 }
 
+// lineDupContents becomes linedup.go in TestFuncWithDuplicateLines.
+const lineDupContents = `
+package linedup
+
+var G int
+
+func LineDup(c int) {
+       for i := 0; i < c; i++ {
+//line ld.go:100
+               if i % 2 == 0 {
+                       G++
+               }
+               if i % 3 == 0 {
+                       G++; G++
+               }
+//line ld.go:100
+               if i % 4 == 0 {
+                       G++; G++; G++
+               }
+               if i % 5 == 0 {
+                       G++; G++; G++; G++
+               }
+       }
+}
+`
+
+// lineDupTestContents becomes linedup_test.go in TestFuncWithDuplicateLines.
+const lineDupTestContents = `
+package linedup
+
+import "testing"
+
+func TestLineDup(t *testing.T) {
+       LineDup(100)
+}
+`
+
+// Test -func with duplicate //line directives with different numbers
+// of statements.
+func TestFuncWithDuplicateLines(t *testing.T) {
+       t.Parallel()
+       testenv.MustHaveGoRun(t)
+       buildCover(t)
+
+       if err := os.Mkdir(lineDupDir, 0777); err != nil {
+               t.Fatal(err)
+       }
+
+       if err := ioutil.WriteFile(lineDupGo, []byte(lineDupContents), 0444); err != nil {
+               t.Fatal(err)
+       }
+       if err := ioutil.WriteFile(lineDupTestGo, []byte(lineDupTestContents), 0444); err != nil {
+               t.Fatal(err)
+       }
+
+       // go test -cover -covermode count -coverprofile TMPDIR/linedup.out
+       cmd := exec.Command(testenv.GoToolPath(t), "test", toolexecArg, "-cover", "-covermode", "count", "-coverprofile", lineDupProfile)
+       cmd.Dir = lineDupDir
+       run(cmd, t)
+
+       // testcover -func=TMPDIR/linedup.out
+       cmd = exec.Command(testcover, "-func", lineDupProfile)
+       cmd.Dir = testTempDir
+       run(cmd, t)
+}
+
 func run(c *exec.Cmd, t *testing.T) {
        t.Helper()
        t.Log("running", c.Args)