cmp/internal/value: fix handling of negative zero for floats (#152)
* Fix IsZero to properly report false for IsZero(-0.0) since
we define IsZero as whether it is equal to the zero memory value.
* Add note to isLess that we don't need to handle -0.0 since
we can't possibly have both keys present in the same map.
* Use sort.SliceStable in SortedKeys for deterministic output since
it is possible to have both -0.0 and +0.0 from two different maps.
The zero key from the left left map will be taken over the right map.
diff --git a/cmp/internal/value/sort.go b/cmp/internal/value/sort.go
index 938f646..24fbae6 100644
--- a/cmp/internal/value/sort.go
+++ b/cmp/internal/value/sort.go
@@ -19,7 +19,7 @@
}
// Sort the map keys.
- sort.Slice(vs, func(i, j int) bool { return isLess(vs[i], vs[j]) })
+ sort.SliceStable(vs, func(i, j int) bool { return isLess(vs[i], vs[j]) })
// Deduplicate keys (fails for NaNs).
vs2 := vs[:1]
@@ -42,6 +42,8 @@
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
return x.Uint() < y.Uint()
case reflect.Float32, reflect.Float64:
+ // NOTE: This does not sort -0 as less than +0
+ // since Go maps treat -0 and +0 as equal keys.
fx, fy := x.Float(), y.Float()
return fx < fy || math.IsNaN(fx) && !math.IsNaN(fy)
case reflect.Complex64, reflect.Complex128:
diff --git a/cmp/internal/value/zero.go b/cmp/internal/value/zero.go
index d13a12c..06a8ffd 100644
--- a/cmp/internal/value/zero.go
+++ b/cmp/internal/value/zero.go
@@ -4,7 +4,10 @@
package value
-import "reflect"
+import (
+ "math"
+ "reflect"
+)
// IsZero reports whether v is the zero value.
// This does not rely on Interface and so can be used on unexported fields.
@@ -17,9 +20,9 @@
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
return v.Uint() == 0
case reflect.Float32, reflect.Float64:
- return v.Float() == 0
+ return math.Float64bits(v.Float()) == 0
case reflect.Complex64, reflect.Complex128:
- return v.Complex() == 0
+ return math.Float64bits(real(v.Complex())) == 0 && math.Float64bits(imag(v.Complex())) == 0
case reflect.String:
return v.String() == ""
case reflect.UnsafePointer:
diff --git a/cmp/internal/value/zero_test.go b/cmp/internal/value/zero_test.go
index 536d50b..1d6c434 100644
--- a/cmp/internal/value/zero_test.go
+++ b/cmp/internal/value/zero_test.go
@@ -6,6 +6,7 @@
import (
"archive/tar"
+ "math"
"reflect"
"testing"
)
@@ -32,6 +33,12 @@
{TestIsZero, false},
{[...]int{0, 0, 0}, true},
{[...]int{0, 1, 0}, false},
+ {math.Copysign(0, +1), true},
+ {math.Copysign(0, -1), false},
+ {complex(math.Copysign(0, +1), math.Copysign(0, +1)), true},
+ {complex(math.Copysign(0, -1), math.Copysign(0, +1)), false},
+ {complex(math.Copysign(0, +1), math.Copysign(0, -1)), false},
+ {complex(math.Copysign(0, -1), math.Copysign(0, -1)), false},
}
for _, tt := range tests {
