go/types, types2: avoid sorting all errors when matching errors
Sorting is only needed if there are multiple matching errors on
the same line. Instead, in that rare case, select the error that
is closest.
Follow-up on CL 456137.
Change-Id: Ia2056b21c629f3a42495e32de89607fbefb82fa7
Reviewed-on: https://go-review.googlesource.com/c/go/+/456335
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go
index 0f97fe9..611466b 100644
--- a/src/cmd/compile/internal/types2/check_test.go
+++ b/src/cmd/compile/internal/types2/check_test.go
@@ -31,7 +31,6 @@
"os"
"path/filepath"
"regexp"
- "sort"
"strings"
"testing"
@@ -68,8 +67,8 @@
}
}
-// delta returns the absolute difference between x and y.
-func delta(x, y uint) uint {
+// absDiff returns the absolute difference between x and y.
+func absDiff(x, y uint) uint {
if x < y {
return y - x
}
@@ -165,13 +164,6 @@
return
}
- // sort errlist in source order
- sort.Slice(errlist, func(i, j int) bool {
- pi, _ := unpackError(errlist[i])
- pj, _ := unpackError(errlist[j])
- return pi.Cmp(pj) < 0
- })
-
// collect expected errors
errmap := make(map[string]map[uint][]syntax.Error)
for _, filename := range filenames {
@@ -187,6 +179,7 @@
}
// match against found errors
+ var indices []int // list indices of matching errors, reused for each error
for _, err := range errlist {
gotPos, gotMsg := unpackError(err)
@@ -199,8 +192,8 @@
errList = filemap[line]
}
- // one of errors in errList should match the current error
- index := -1 // errList index of matching message, if any
+ // At least one of the errors in errList should match the current error.
+ indices = indices[:0]
for i, want := range errList {
pattern := strings.TrimSpace(want.Msg[len(" ERROR "):])
if n := len(pattern); n >= 2 && pattern[0] == '"' && pattern[n-1] == '"' {
@@ -212,19 +205,27 @@
continue
}
if rx.MatchString(gotMsg) {
- index = i
- break
+ indices = append(indices, i)
}
}
- if index < 0 {
+ if len(indices) == 0 {
t.Errorf("%s: no error expected: %q", gotPos, gotMsg)
continue
}
+ // len(indices) > 0
- // column position must be within expected colDelta
- want := errList[index]
- if delta(gotPos.Col(), want.Pos.Col()) > colDelta {
- t.Errorf("%s: got col = %d; want %d", gotPos, gotPos.Col(), want.Pos.Col())
+ // If there are multiple matching errors, select the one with the closest column position.
+ index := -1 // index of matching error
+ var delta uint
+ for _, i := range indices {
+ if d := absDiff(gotPos.Col(), errList[i].Pos.Col()); index < 0 || d < delta {
+ index, delta = i, d
+ }
+ }
+
+ // The closest column position must be within expected colDelta.
+ if delta > colDelta {
+ t.Errorf("%s: got col = %d; want %d", gotPos, gotPos.Col(), errList[index].Pos.Col())
}
// eliminate from errList
diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go
index 81736f6..4d27f36 100644
--- a/src/go/types/check_test.go
+++ b/src/go/types/check_test.go
@@ -36,7 +36,6 @@
"path/filepath"
"reflect"
"regexp"
- "sort"
"strings"
"testing"
@@ -82,8 +81,8 @@
panic("unreachable")
}
-// delta returns the absolute difference between x and y.
-func delta(x, y int) int {
+// absDiff returns the absolute difference between x and y.
+func absDiff(x, y int) int {
if x < y {
return y - x
}
@@ -183,20 +182,6 @@
return
}
- // sort errlist in source order
- sort.Slice(errlist, func(i, j int) bool {
- // TODO(gri) This is not correct as scanner.Errors
- // don't have a correctly set Offset. But we only
- // care about sorting when multiple equal errors
- // appear on the same line, which happens with some
- // type checker errors.
- // For now this works. Will remove need for sorting
- // in a subsequent CL.
- pi, _ := unpackError(fset, errlist[i])
- pj, _ := unpackError(fset, errlist[j])
- return pi.Offset < pj.Offset
- })
-
// collect expected errors
errmap := make(map[string]map[int][]comment)
for i, filename := range filenames {
@@ -206,6 +191,7 @@
}
// match against found errors
+ var indices []int // list indices of matching errors, reused for each error
for _, err := range errlist {
gotPos, gotMsg := unpackError(fset, err)
@@ -218,8 +204,8 @@
errList = filemap[line]
}
- // one of errors in errList should match the current error
- index := -1 // errList index of matching message, if any
+ // At least one of the errors in errList should match the current error.
+ indices = indices[:0]
for i, want := range errList {
pattern := strings.TrimSpace(want.text[len(" ERROR "):])
if n := len(pattern); n >= 2 && pattern[0] == '"' && pattern[n-1] == '"' {
@@ -231,20 +217,28 @@
continue
}
if rx.MatchString(gotMsg) {
- index = i
- break
+ indices = append(indices, i)
}
}
- if index < 0 {
+ if len(indices) == 0 {
t.Errorf("%s: no error expected: %q", gotPos, gotMsg)
continue
}
+ // len(indices) > 0
- // column position must be within expected colDelta
- const colDelta = 0
- want := errList[index]
- if delta(gotPos.Column, want.col) > colDelta {
- t.Errorf("%s: got col = %d; want %d", gotPos, gotPos.Column, want.col)
+ // If there are multiple matching errors, select the one with the closest column position.
+ index := -1 // index of matching error
+ var delta int
+ for _, i := range indices {
+ if d := absDiff(gotPos.Column, errList[i].col); index < 0 || d < delta {
+ index, delta = i, d
+ }
+ }
+
+ // The closest column position must be within expected colDelta.
+ const colDelta = 0 // go/types errors are positioned correctly
+ if delta > colDelta {
+ t.Errorf("%s: got col = %d; want %d", gotPos, gotPos.Column, errList[index].col)
}
// eliminate from errList
