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.

cmp/internal/value/sort.go

@@ -19,7 +19,7 @@ func SortKeys(vs []reflect.Value) []reflect.Value {
 	}
 
 	// 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 @@ func isLess(x, y reflect.Value) bool {
 	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:

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 @@ func IsZero(v reflect.Value) bool {
 	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:

cmp/internal/value/zero_test.go

@@ -6,6 +6,7 @@ package value
 
 import (
 	"archive/tar"
+	"math"
 	"reflect"
 	"testing"
 )
@@ -32,6 +33,12 @@ func TestIsZero(t *testing.T) {
 		{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 {