math/big: nat does not play well with reflect.DeepEqual

[zamsden]

The underlying problem is that the underlying nat type doesn't allocate a backing slice for the zero value in some initialization sequences, which causes reflect.DeepEqual to return false for equal big.Int values.

Either big.nat needs to be changed to always drop backing slices when setting to zero, always allocate backing slices, or the argument that zero-length slices and zero-length nil slices are different needs more thought. The current default answer is pedantically correct but not programmer friendly.

See #12918

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

x86_64

What did you do?

https://play.golang.org/p/CUcu_oCBILj

What did you expect to see?

true

What did you see instead?

false

[ALTree]

cc @griesemer

Either big.nat needs to be changed to always drop backing slices when setting to zero [...]

It would be interesting to test what kind of effect would this have on the number of allocation of a typical math/big workload. nat wraps the backing array of Int and Float types. Dropping the backing array every time the nat goes to zero may have a negative effect on the performance of math/big types.

[zamsden]

Very true. I ran into this in the context of the bls/bn256 libraries which provide multi-signature crypto support. In this environment, dropping the backing array should probably be a win, as the only reason to reduce a Natural number element down to zero is when normalizing (usually for serialization for output).

In other situations, the performance objectives may differ quite a bit. There are some very good perf tests for bn256 IIRC, so perhaps we should give those a Go and see what the results are.

[griesemer]

Using reflect.DeepEqual looks at the internals of an implementation. It can't be the right approach to require each package to adhere to the external requirements imposed by reflect.DeepEqual. Or putting it differently, reflect.DeepEqual is fine to use in package-internal code (usually tests) that has deep knowledge of the data structures being compared. It should not be used to compare "opaque" data structures: it is the prerogative of a package to hide implementation details (that's what packages are all about!) and to choose different representations for the same abstract entities implemented by the package. The math/big package offers the correct compare functions that can handle the different representations of zero.

Instead, consider using https://github.com/google/go-cmp .

I don't believe there is anything to do here. Closing.