math/big: zero big int is not deep equal

[islishude]

Go version

go version go1.23.2 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/sudoless/Library/Caches/go-build'
GOENV='/Users/sudoless/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/sudoless/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/sudoless/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.2/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/sudoless/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/sudoless/codespace/metis/goat-network/goat-geth/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/ns/ft0h42296vbct3jtq5rpq71h0000gn/T/go-build558577267=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

https://go.dev/play/p/f17Hau9xQYi

What did you see happen?

print false

What did you expect to see?

they are should be equal with another

[gabyhelp]

Related Issues and Documentation

• math/big: DeepEqual return false when comparing Rat with SetString("0") and SetInt64(0) #50944 (closed)
• reflect.DeepEqual does not function correctly for Int values. #56991 (closed)
• math: Pow(negativeZero, number_that_overflows_int64) produces incorrect result on arm64 #57465 (closed)
• math/big: Int64() conversion incorrect despite Exact accuracy #17305 (closed)
• math/big: document that shallow copies of big.Int/Rat/Float are not supported #28423 (closed)
• cmd/compile: miscompilation of simple arithmetic program targeting darwin/arm64 #62723 (closed)
• go/types, types2: wrong result of float add float #48315 (closed)
• math/big: Int returned by Float.Int(Inf) is not usable #33798 (closed)
• float computation: Mac M1 result changes by adding a print #54175 (closed)
• cmd/compile: unstable arithmetic #52293 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[randall77]

These two Ints probably differ in that one has a nil slice, the other has a non-nil length 0 slice.

[Jorropo]

I'm not aware of any promise regarding reflect.DeepEqual and DeepEqual is by design not safe to use for things like this as it traverse private fields.

[islishude]

the thing is how to handle it in unit testing.

the test wasted me 10 minutes searching for a bug. :)

[Jorropo]

@islishude https://go.dev/play/p/X_PPKXEzMlI ¿

[islishude]

yes but I have the big.Int in a struct.

I'm going to close this since it works as expected.

[randall77]

I think we might want to fix this. doing reflect.DeepEqual on big.Int almost works. I think it's only the zero length nil vs non-nil slices that break it. That's a nasty footgun, as it isn't obvious that it isn't working until you happen to compare two zeros of different "kinds". If it always or mostly didn't work it wouldn't be as bad a problem as tests would notice.

The fix would be then just to nil out zero-length slices (in (nat).norm, probably). Hm, but that might interfere with allocation behavior.

@gri

[randall77]

@griesemer

[adonovan]

I think it's a slippery slope. Most data types don't come with any guarantee that DeepEqual works on them, and in general a maintainer has a lot more room to maneuver if they don't have to preserve that invariant. It is the responsibility of the caller of DeepEqual to arrange for the arguments to be of suitable form, and to know that most are not.

[thepudds]

Part of the issue might be that reflect.DeepEqual is often not the right tool for the job, but because it is conveniently at hand, it ends up being used more often than it should.

Maybe some type of caution or warning at the start of the DeepEqual documentation might help?

Separately, perhaps someone will have interest in resuscitating a slimmed down version of #45200 to add a new testing/cmp package or similar.

[randall77]

One option is to make big.Int definitely not work with reflect.DeepEqual. Like:

type Int struct {
	neg bool
	randomByteToDefeatReflectDeepEqual uint8
	abs nat
}
func NewInt(x int64) *Int {
	...
	return &Int{neg: x < 0, abs: abs, randomByteToDefeatReflectDeepEqual: getARandomByte()}
}

[zigo101]

For the "definitely not work with reflect.DeepEqual" purpose, the neg field can be of a function type:

func neg() bool {return true}
func pos() bool {return false}

type Word uint
type nat []Word
type T struct {
	neg func() bool
	abs nat 
}

func main() {
  var t = T{neg: neg}
  println(reflect.DeepEqual(t, t)) // false
}

[adonovan]

Both of those solutions have costs disproportionate to the problem:

• the first means that a zero value is no longer a valid Int, and all the constructors must call rand();
• the second changes a boolean field access into a dynamic function call.

Ideally there would be some zero-sized value that reflect.DeepEqual considers not equal to itself, but only func types (and floating-point NaN values) have this property today.

Crazy idea: add a new zero-sized defined type to the reflect package that causes DeepEqual to panic. (I suspect that could lead to all kinds of trouble when values of that type are mischievously sprinkled into, say, gob-encoded data...)

[zigo101]

Or a noCopy alike vet rule?

[adonovan]

Or a noCopy alike vet rule?

I don't see how a static analysis could detect it in general, since DeepEqual traverses the entire type, including interface values.

[griesemer]

Adding an extra field with a random value would work only if all big.Ints are initialized explicitly. But we allow the zero value to be usable on purpose. So it would only "fix" DeepEqual for code that explicitly initializes the big.Int. This is not good enough.

Resetting the abs field to nil whenever a big.Int becomes 0 would work but it may have an (possibly significant) impact on performance in some ccde because existing memory may not be re-used. Hard to assess how big of an issue that really is. It also requires a thorough review of the big.Int code to make sure we truly catch all cases where abs needs to be set to nil (it may be more than just in norm, but not sure). But that of course should be doable.

Finally, using a func type for the neg field is an interesting idea. Because the zero value of a big.Int is usable, we would need to use a nil func() to indicate a positive value, and a non-nil func() type to indicate a negative value. No calls would be needed so there wouldn't be much of a performance issue here when testing for >0 or <0. But it also doesn't work because if we have two negative big.Ints of the same value, DeepEqual would judge the two non-nil func() fields as different.

But DeepEqual already compares slices element-by-element. Maybe it should do this more consistently w/o looking at whether the slice is nil or not. That is, a nil slice and a zero-element non-nil slice should be considered equal. But that may not be a change we can make either w/o breaking existing code.

I think, as @alandonovan already said, the costs here are disproportionate to the problem. Using DeepEqual is simply problematic whenever one is comparing data structures with hidden fields. In general we can't assume it is working correctly because it would require that people write such data structures with DeepEqual in mind.

I think the issue here is on the client side (don't use DeepEqual in this case, or not exlusively), not in math/big.

[randall77]

Ok, I'll close. It does seem that any "fix" is probably worse than the problem.