Skip to content

Commit 01193de

Browse files
authored
Commit to not comparing NaN keys (#110)
Go 1.12 adds the ability to range over NaN keys. However, it turns out that even with this functionality, there is still no obvious way to compare the values. Thus, commit to behavior of just panicking and letting obscure maps like this be handled by a custom Comparer. Fixes #93
1 parent 8ca8745 commit 01193de

File tree

1 file changed

+15
-10
lines changed

1 file changed

+15
-10
lines changed

cmp/compare.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,6 @@ import (
3636
"github.com/google/go-cmp/cmp/internal/value"
3737
)
3838

39-
// BUG(dsnet): Maps with keys containing NaN values cannot be properly compared due to
40-
// the reflection package's inability to retrieve such entries. Equal will panic
41-
// anytime it comes across a NaN key, but this behavior may change.
42-
//
43-
// See https://golang.org/issue/11104 for more details.
44-
4539
var nothing = reflect.Value{}
4640

4741
// Equal reports whether x and y are equal by recursively applying the
@@ -504,10 +498,21 @@ func (s *state) compareMap(vx, vy reflect.Value, t reflect.Type) {
504498
s.report(false, nothing, vvy)
505499
default:
506500
// It is possible for both vvx and vvy to be invalid if the
507-
// key contained a NaN value in it. There is no way in
508-
// reflection to be able to retrieve these values.
509-
// See https://golang.org/issue/11104
510-
panic(fmt.Sprintf("%#v has map key with NaNs", s.curPath))
501+
// key contained a NaN value in it.
502+
//
503+
// Even with the ability to retrieve NaN keys in Go 1.12,
504+
// there still isn't a sensible way to compare the values since
505+
// a NaN key may map to multiple unordered values.
506+
// The most reasonable way to compare NaNs would be to compare the
507+
// set of values. However, this is impossible to do efficiently
508+
// since set equality is provably an O(n^2) operation given only
509+
// an Equal function. If we had a Less function or Hash function,
510+
// this could be done in O(n*log(n)) or O(n), respectively.
511+
//
512+
// Rather than adding complex logic to deal with NaNs, make it
513+
// the user's responsibility to compare such obscure maps.
514+
const help = "consider providing a Comparer to compare the map"
515+
panic(fmt.Sprintf("%#v has map key with NaNs\n%s", s.curPath, help))
511516
}
512517
}
513518
}

0 commit comments

Comments
 (0)