Hi, I was wondering if you would consider adding a new option which would apply the AllowUnexported Option to all types and would not require a slice of input like the current AllowUnexported function does?

My use case is that I have a lot of tests which use the github.com/stretchr/testify package, which ultimately uses reflect.DeepEqual for comparing two objects, that break in Go 1.9 because they try to compare structs which contain time.Time fields. Admittedly, I could define an Equal method for all such structs but that doesn't seem to address the root of the problem to me, which is that I would like reflect.DeepEqual functionality (which compares all fields, both private and public) with the caveat that I would like to check if the types being compared have an Equal method for testing equality. Furthermore, while for structs I could pass the structs being compared to AllowUnexported, I would like the same behavior for composite types as well (e.g. maps and slices) which is not supported by AllowUnexported currently since it specifically checks that the types of the objects passed to it are structs.

I would be more than happy to put up a PR for such an Option, a first look at the code leads me to believe that we could define an additional field on state indicating that we want to always check private fields. Then, when set to true, we would always extract a struct's fields in tryExporting. With that being said, I got the impression that you might be opposed to such an Option given the warnings for AllowUnexported already and so I thought it better to open an issue first where we could discuss.

Thanks!

https://godoc.org/github.com/google/go-cmp/cmp#Comparer states "If T is an interface, it is possible that f is called with two values of different concrete types that both implement T."

I'm trying to compare different concrete types of an interface, but my Comparer is never called.

Running:

package main

import (
	"github.com/google/go-cmp/cmp"
)

func main() {
	var i1 I
	var i2 I
	
	i1 = &A{}
	i2 = &B{}
	
	cmp.Equal(i1, i2, cmp.Comparer(ic))
	
	panic("not compared")
}

type I interface {
  Foo()
}

type A struct {}
func(a *A) Foo() {}

type B struct {}
func(b *B) Foo() {}

func ic(i1 I, i2 I) bool {
        panic("compared")
}

Results in:

panic: not compared

goroutine 1 [running]:
main.main()
        /tmp/main.go:16 +0x111
exit status 2

$ go version
go version go1.13 linux/amd64
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/cjnosal/.cache/go-build"
GOENV="/home/cjnosal/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/mnt/d/workspace/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build444735255=/tmp/go-build -gno-record-gcc-switches"

The current implementation of cmp.Equal recursively applies any Transformers that remain after applying all the filters. For a trivial Transformer of the type func(T) T, this will be recursively apply to the output since it will always match. For this reason, the cmpopts.SortSlices is wrapped with a FilterValues that acts as a "base-case" to prevent infinite recursion. One alteration to the algorithm is to say that a given Transformer cannot be recursively applied on a value that was transformed at some point by that very Transformer.

The major advantage of non-recursive Transformers is:

• A naive use of func(T) T works as expected without any need for a "base-case" filter.

The disadvantages of non-recursive Transformer is:

• The additional documentation needed to explain the special-case where Transformers are not executed. The implementation of this is essentially a set of "spent" Transformers, where transformers are added to the set when descending down the tree, and popped when ascending up the tree.
• In some rare use-cases you may actually want Transformers to be recursive. Imagine you have a non-binary tree, where each node is essentially a set of children. When comparing this data-structure, you may want to sort each node using a transformer. You would need the property of recursive transformers to ensure each node may get transformed.
• The need for "base-case" filters to prevent recursive cycles.

Thoughts? I do see the value in non-recursive Transformers. It simplifies use of them, but prevents some very rare use-cases.

\cc @bcmills @jba

In some situations users want to compare two maps where missing entries are equal to those that have the zero value. For example, the following maps would be considered equal: x := map[string]int{"foo": 12345, "zero":0} y := map[string]int{"foo": 12345}

To help with this, we add DiscardElements to cmpopts that transforms maps and slices by stripping entries based on a user provided function. To strip zero values, the user can provide: cmpopts.DiscardElements(func(v int) bool { return v == 0 })

GopherJS does not have a complete implementation of unsafe, but it is complete enough that it can do basic pointer arithmetic, which is all we need to forcibly export an unexported field.

I am trying to stop go-cmp from calling fmt.Stringer, as this in some cases makes the output worse instead of better.

The DefaultReporter implementation is quite customizable, but this functionality is not exposed to the user.

While users can register custom exporters, they would then need to re-implement the entire DefaultExporter, even though changing a single bool in it's config would suffice.

This PR exposes the DefaultReporter implementation so it can be customized and used outside of this package

I just tried using cmpopts.SortSlices, and I know.. there are no generics in Go yet, but I think the current signature is not intuitive.

Current state of the world:

opt := cmpopts.SortSlices("yay") // can do whatever I want, it's an interface{}
cmp.Diff(someFoo, otherFoo, opt)

My desired state of the world:

// Can still mess things up, but it's more obvious.
opt := cmpopts.SortSlices(func (a, b interface{}){ return a.(Foo).Val < b.(Foo).Val })
cmp.Diff(someFoo, otherFoo, opt)

This proposal goes against Issue #67 but we could make it work like...

opt := cmpopts.SortSlices(cmpopts.DefaultSort) // renamed GenericLess to DefaultSort
cmp.Diff(someFoo, otherFoo, opt)

Go1.8 went GA on AppEngine. The lack of Go1.8 support was the primary reason for much of these backwards compatibility hacks.

Bumping to Go1.8 still ensures that we're supporting at least the latest 2 versions of Go, which are Go1.8 and Go1.9.

Now that Go 1.13 is the minimum version, we can use the reflect.Value.IsZero method instead of our own internal/value.IsZero function. Interestingly, our IsZero function pre-dates the IsZero method, but fortunately has the exact same semantics, since both are targetting semantics defined by the Go language specification.

With Go tip (to become Go 1.14) and its new checkptr mode (enabled by default in -race mode on anything but Windows for now):

=== CONT  TestDiff/EqualMethod/StructE1
panic: runtime error: unsafe pointer arithmetic [recovered]
        panic: runtime error: unsafe pointer arithmetic [recovered]
        panic: runtime error: unsafe pointer arithmetic

goroutine 142 [running]:
testing.tRunner.func1(0xc000238d00)
        /home/bradfitz/go/src/testing/testing.go:881 +0x69f
panic(0x6f3c00, 0xc00000f5e0)
        /home/bradfitz/go/src/runtime/panic.go:679 +0x1b2
github.com/google/go-cmp/cmp_test.TestDiff.func1.1.1(0xc000097eb8)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare_test.go:70 +0xfe
panic(0x6f3c00, 0xc00000f5e0)
        /home/bradfitz/go/src/runtime/panic.go:679 +0x1b2
github.com/google/go-cmp/cmp.retrieveUnexportedField(0x6e33c0, 0xc000128ad8, 0x199, 0x6ad073, 0xd, 0x6b70c3, 0x31, 0x778440, 0x6d34e0, 0x0, ...)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/export_unsafe.go:22 +0x7d
github.com/google/go-cmp/cmp.StructField.Values(0xc0002bd300, 0x8, 0x42c5e0, 0x0, 0x0, 0xc00026f6c0, 0x203000)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/path.go:190 +0x187
github.com/google/go-cmp/cmp.(*valueNode).PushStep(0xc0000a8420, 0x775540, 0xc0002bd300, 0x45bc9a)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/report_value.go:54 +0x5d
github.com/google/go-cmp/cmp.(*defaultReporter).PushStep(0xc00027d680, 0x775540, 0xc0002bd300)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/report.go:24 +0x6e
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0002be900, 0x775540, 0xc0002bd300)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare.go:214 +0x22d
github.com/google/go-cmp/cmp.(*state).compareStruct(0xc0002be900, 0x778440, 0x6e33c0, 0x6e33c0, 0xc000128ad8, 0x199, 0x6e33c0, 0xc000128ae0, 0x199)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare.go:383 +0x548
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0002be900, 0x775480, 0xc000012a00)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare.go:252 +0x1a58
github.com/google/go-cmp/cmp.(*state).comparePtr(0xc0002be900, 0x778440, 0x6d8140, 0x6d8140, 0xc000128ad8, 0x16, 0x6d8140, 0xc000128ae0, 0x16)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare.go:513 +0x526
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0002be900, 0x775280, 0xc0000129c0)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare.go:258 +0x18e6
github.com/google/go-cmp/cmp.Equal(0x6d8140, 0xc000128ad8, 0x6d8140, 0xc000128ae0, 0xc00000f580, 0x2, 0x2, 0x580c98)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare.go:107 +0x406
github.com/google/go-cmp/cmp.Diff(0x6d8140, 0xc000128ad8, 0x6d8140, 0xc000128ae0, 0xc00012ab70, 0x1, 0x1, 0xc000106000, 0xc000106070)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare.go:127 +0x1ce
github.com/google/go-cmp/cmp_test.TestDiff.func1.1(0xc000241eb8, 0xc00023c600, 0xc0002f7ec8)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare_test.go:74 +0x138
github.com/google/go-cmp/cmp_test.TestDiff.func1(0xc000238d00)
        /home/bradfitz/pkg/mod/github.com/google/[email protected]/cmp/compare_test.go:75 +0x99
testing.tRunner(0xc000238d00, 0xc0001c1bd0)
        /home/bradfitz/go/src/testing/testing.go:916 +0x19a
created by testing.(*T).Run
        /home/bradfitz/go/src/testing/testing.go:967 +0x652
FAIL    github.com/google/go-cmp/cmp    0.116s

Hi The following program crashes on an infinite recursion:

package main

import "github.com/google/go-cmp/cmp"

type Node struct {
	Next *Node
}

func main() {
	a := &Node{}
	a.Next = a

	cmp.Equal(a, a)
}

Fixing this is needed to use this package in testify.

I saw that there is already a TODO comment.

Summary

When I want to compare two Japanese words and there is diff, the returned value of Diff method is unreadable.

Detail

Here is an example.

func Echo() string {
	return "プライベート ブランド シャツ"
}

test code

func TestEcho(t *testing.T) {
	expected := "プライベート ブランド ジャケット"
	actual := Echo()
	if diff := cmp.Diff(expected, actual); diff != "" {
		t.Errorf("%s result mismatch (-want, +got):\n%s", t.Name(), diff)
	}
}

cmp.Diff produces

--- FAIL: TestEcho (0.00s)
    main_test.go:13: TestEcho result mismatch (-want, +got):
          strings.Join({
                "プライベート ブランド \xe3\x82",
        -       "\xb8ャケット",
        +       "\xb7ャツ",
          }, "")
FAIL
FAIL    cmpbug  0.347s
FAIL

the above output is not readable. I expect the following output

❯ go test ./...
--- FAIL: TestEcho (0.00s)
    main_test.go:13: TestEcho result mismatch (-want, +got):
          string(
                 "プライベート ブランド ",
        -       "ジャケット",
        +       "シャツ",
          )
FAIL
FAIL    cmpbug  0.229s
FAIL

version info

• go: 1.19.2
• cmp: v0.5.9

Is there a way to ignore all unexported fields?

I have a library where I do comparisons and I need a generic way to skip unexported fields. ie. protobuf types.

Sometimes I get output where the relevant difference between two strings is not shown because one or both get truncated with elided lines. Example:

        - 			SystemErr: Inverse(simplify, string(
        - 				"""
        - 				> Enter [BeforeEach] e2e - cleanup_test.go:54 <time>
        - 				INFO: before
        - 				< Exit [BeforeEach] e2e - cleanup_test.go:54 <time>
        - 				> Enter [BeforeEach] e2e - set up framework | framework.go:xxx <time>
        - 				STEP: Creating a kubernetes client - framework.go:xxx <time>
        - 				INFO: >>> kubeConfig: yyy/kube.config
        - 				STEP: Building a namespace api object, basename test-namespace - framework.go:xxx <time>
        - 				INFO: Skipping waiting for service account
        - 				< Exit [BeforeEach] e2e - set up framework | framework.go:xxx <time>
        - 				> Enter [BeforeEach] e2e - cleanup_test.go:98 <time>
        - 				INFO: extension before
        - 				< Exit [BeforeEach] e2e - cleanup_test.go:98 <time>
        - 				> Enter [BeforeEach] e2e - cleanup_test.go:62 <time>
        - 				INFO: before #1
        - 				< Exit [BeforeEach] e2e - cleanup_test.go:62 <time>
        - 				> Enter [BeforeEach] e2e - cleanup_test.go:66 <time>
        - 				INFO: before #2
        - 				< Exit [BeforeEach] e2e - cleanup_test.go:66 <time>
        - 				> Enter [It] works - cleanup_test.go:81 <time>
        - 				[FAILED] failure
        - 				In [It] at: cleanup_test.go:90 <time>
        - 				< Exit [It] works - cleanup_test.go:81 <time>
        - 				> Enter [AfterEach] e2e - cleanup_test.go:99 <time>
        - 				INFO: extension after
        - 				< Exit [AfterEach] e2e - cleanup_test.go:99 <time>
        - 				> Enter [AfterEach] e2e - cleanup_test.go:70 <time>
        - 				INFO: after #1
        - 				< Exit [AfterEach] e2e - cleanup_test.go:70 <time>
        - 				> Enter [AfterEach] e2e - cleanup_test.go:77 <time>
        - 				INFO: after #2
        - 				< Exit [AfterEach] e2e - cleanup_test.go:77 <time>
        - 				"""
        - 			)),
        + 			SystemErr: Inverse(simplify, string(
        + 				"""
        + 				> Enter [BeforeEach] e2e - cleanup_test.go:54 <time>
        + 				INFO: before
        + 				< Exit [BeforeEach] e2e - cleanup_test.go:54 <time>
        + 				> Enter [BeforeEach] e2e - set up framework | framework.go:xxx <time>
        + 				STEP: Creating a kubernetes client - framework.go:xxx <time>
        + 				INFO: >>> kubeConfig: yyy/kube.config
        + 				STEP: Building a namespace api object, basename test-namespace - framework.go:xxx <time>
        + 				INFO: Skipping waiting for service account
        + 				< Exit [BeforeEach] e2e - set up framework | framework.go:xxx <time>
        + 				> Enter [BeforeEach] e2e - cleanup_test.go:98 <time>
        + 				INFO: extension before
        + 				< Exit [BeforeEach] e2e - cleanup_test.go:98 <time>
        + 				> Enter [BeforeEach] e2e - cleanup_test.go:62 <time>
        + 				INFO: before #1
        + 				< Exit [BeforeEach] e2e - cleanup_test.go:62 <time>
        + 				> Enter [BeforeEach] e2e - cleanup_test.go:66 <time>
        + 				INFO: before #2
        + 				< Exit [BeforeEach] e2e - cleanup_test.go:66 <time>
        + 				> Enter [It] works - cleanup_test.go:81 <time>
        + 				[FAILED] failure
        + 				In [It] at: cleanup_test.go:90 <time>
        + 				< Exit [It] works - cleanup_test.go:81 <time>
        + 				> Enter [AfterEach] e2e - cleanup_test.go:99 <time>
        + 				INFO: extension after
        + 				< Exit [AfterEach] e2e - cleanup_test.go:99 <time>
        + 				> Enter [AfterEach] e2e - cleanup_test.go:70 <time>
        + 				INFO: after #1
        + 				< Exit [AfterEach] e2e - cleanup_test.go:70 <time>
        + 				> Enter [AfterEach] e2e - cleanup_test.go:77 <time>
        + 				INFO: after #2
        + 				< Exit [AfterEach] e2e - cleanup_test.go:77 <time>
        + 				... // 27 elided lines
        + 				"""
        + 			)),

In this example, I started with the expected string set to empty, copied as much of the actual string as possible, and then repeated. But because not all of it was shown, I am missing some lines. I was hoping to get a diff with the missing lines, but the heuristics apparently decided to just show both strings.

The simplify function does some string->string replacement to get rid of some line numbers and time stamps.

It's not entirely clear to me why this happens. I tried to reproduce it with exactly these strings and without a transformer, but then it worked as expected (showed a diff). I also tried with a transformer, with the same result.

I'm using intsets.Sparse as a field in a struct:

type T struct {
   v intsets.Sparse
   // other fields
}

When traversing T, the field v is copied, causing all methods to fail. This precludes the use of Transformer.

Ideally, there would be an Option that would take the address of the field for use with other Options. Notably, Sparse has an Equals(*Sparse) bool method, so perhaps an alternate idea is to allow different equality methods? But I'm not sure whether go-cmp will take the address of a field in order to invoke the Equal method.