reflect: DeepEqual fails for nested NaN values

[tgpfeiffer]

What version of Go are you using (go version)?

go version go1.4 linux/amd64

What operating system and processor architecture are you using?

Fedora Linux 20, 3.19.8-100.fc20.x86_64 #1 SMP kernel

What did you do?
What did you expect to see?
What did you see instead?

I tried to compare a map that contains NaN values for a fixed key using reflect.DeepEqual and the result was true. I expected false because two NaN values are never equal. reflect.DeepEqual(math.NaN(), math.NaN()) is also false.

package main

import "fmt"
import "math"
import "reflect"

func main() {
    fmt.Println(reflect.DeepEqual(math.NaN(), math.NaN()))

    a := map[string]float64{"a": math.NaN()}
    fmt.Println(reflect.DeepEqual(a, a))

    b := []float64{math.NaN()}
    fmt.Println(reflect.DeepEqual(b, b))
}

outputs:

false
true
true

[ianlancetaylor]

This is because of a shortcut in reflect.DeepEqual: when comparing a map with itself, DeepEqual returns true without bothering to compare the keys and values. I guess we need to take that shortcut out.

[griesemer]

Same is true when comparing slices.

Being able to avoid element-wise comparison of possibly large maps and slices that are identical seems more valuable than searching for that rare NaN. DeepEqual is often used to compare test results, and if the results contain NaNs, one would have to special-case that situation - it might actually be more useful for NaNs to follow the "usual" bitwise equality of floats. Or in other words, DeepEqual perhaps should be defined to return true when comparing two NaNs that are bitwise identical.

[ianlancetaylor]

I think it's pretty important that DeepEqual follow the same rules as the language.

[griesemer]

@iant It does where the language defines what equality means. For floats it's indirectly defined by IEEE. NaN != NaN makes sense for some floating-point applications; but even there it's very rare.

[cespare]

Seems like the documentation should mention this, at least.

It uses normal == equality where possible but will scan elements of arrays, slices, maps, and fields of structs. In maps, keys are compared with == but elements use deep equality.

[tgpfeiffer]

It does where the language defines what equality means.

Well, one issue is that the current behavior is not exactly consistent:

a := map[string]float64{"a": math.NaN()}
reflect.DeepEqual(a, map[string]float64{"a": math.NaN()}) // false
reflect.DeepEqual(a, a) // true

I wouldn't call this expected behavior. As mentioned by @griesemer this is a very special case and I am not sure that efficient map identity comparison should be sacrificed for it. However, I think the documentation should in fact mention it, as "but it will scan elements of arrays, slices, maps ..." is not correct as is, as can be seen in the example above.

[ianlancetaylor]

@griesemer Good point that map equality is not defined anyhow.

But we need to document this, so reopening.