out-of-the-box support for atomic.Pointer about go-cmp HOT 9 OPEN

At present, cmp has no special support for any type in the Go standard library. The original reason why cmp was invented was because Go 1.9 introduced monotonic clock readings in time.Time, which broke thousands of tests that relied on t1 == t2 being semantically identical to t1.Equal(t2). To fix this, we never gave special meaning to time.Time, but rather gave special meaning to types that have an Equal method since this was more versatile. Similarly, I don't think we should give special support for the atomic types. Perhaps they should gain a Equal method instead? This would be in line with other proposed methods to add (see golang/go#54582).

Regarding the inability to properly write a transformer for this, I've stated in #310 (comment) that the inability of cmp to take the address of things by default is my greatest regret in how cmp came about. I feel strongly that we should do something to fix #310, which would at least make it possible to write a transformer to safely perform the comparison yourself.

from go-cmp.

Both of these other alternatives sound better to me, in particular adding Equal because it wouldn't need changes in apps. I might try to drive adding atomic/sync.Equal but if someone else beats me to it, then I certainly wouldn't mind 😁

Let's keep this issue open until some solution is implemented? But up to you...

from go-cmp.

After thinking about Equal a bit more, I am not sure how atomic/sync could implement it in a way that would match how cmp handles plain pointers. What sync.Pointer can compare is the address that is being pointed to (not the same as cmp, not useful). It could call Equal on the items being pointed to, but only if implemented (less flexible than cmp).

Also, if false is returned, how would cmp produce a useful diff without access to the two items?

from go-cmp.

You could imagine the following being added:

package atomic
func (x *Int64) Equal(y *Int64) bool { return x.Load() == y.Load() }
func (x *Int64) String() string      { return fmt.Sprint(x.Load()) }

from go-cmp.

I suspect fmt.Sprint will lead to very unreadable formatting of more complex types and not work well in combination with cmp.Diff - think of atomic.Pointer[SomeStruct], not atomic.Pointer[int].

from go-cmp.

Also, x.Load() == y.Load() would compare the pointers for atomic.Pointer[T any], not the T values that are being pointed to. *x.Load() == *y.Load() wouldn't compile because T is not required to be comparable.

from go-cmp.

In the case of atomic.Pointer, it is unclear whether atomic.Pointer.Equal should be a pointer comparison or deep comparison. Different use-cases are going to reasonably want different behavior. This perhaps suggests that the atomic types should not have an Equal method and further suggests that cmp should not have default comparison behavior for atomic types since the default equality behavior is ill-specified.

Regardless of what we do here, the general philosophy of cmp is to not have specialized behavior of types, but to off-load customized behavior to user-provided cmp.Transformer options.

Of course, this still requires #310 be addressed, which is a prerequisite.

from go-cmp.

Doesn't cmp.Diff already have an opinion about what equality of plain pointer fields means (compare nil vs non-nil, then what's being pointed to)? Doing the same for atomic.Pointer IMHO would make sense from a consistency perspective. The same with atomic.Int64 - I'd expect that to be treated like an int64.

But that's just my two cents. Addressing #310 is the more general solution.

from go-cmp.

Doesn't cmp.Diff already have an opinion about what equality of plain pointer fields means (compare nil vs non-nil, then what's being pointed to)?

Perhaps, but I still stand by "the general philosophy of cmp is to not have specialized behavior of types".

from go-cmp.