Skip to content

Commit

Permalink
Explicitly convert assignable nil interface values (#49)
Browse files Browse the repository at this point in the history
The reflect package has a bug (golang/go#22143).
Let R and T both be interface types and R be assignable to T.
Let F be a function that takes T as its argument.

The Go language lets you directly call F with a nil T or a nil R
(since R is assignable to T) without any special handling.

Currently, reflect allows you to call F with a nil T,
but panics when calling F with a nil R. To avoid this panic,
we explicitly check for the condition where a value:
	* is an interface type
	* is nil
	* is not exactly same as type T
and re-create the value as a nil interface of type T.
  • Loading branch information
dsnet committed Oct 5, 2017
1 parent 47d4c62 commit 7ffe192
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
15 changes: 15 additions & 0 deletions cmp/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ func (s *state) tryMethod(vx, vy reflect.Value, t reflect.Type) bool {
}

func (s *state) callTRFunc(f, v reflect.Value) reflect.Value {
v = sanitizeValue(v, f.Type().In(0))
if !s.dynChecker.Next() {
return f.Call([]reflect.Value{v})[0]
}
Expand All @@ -345,6 +346,8 @@ func (s *state) callTRFunc(f, v reflect.Value) reflect.Value {
}

func (s *state) callTTBFunc(f, x, y reflect.Value) bool {
x = sanitizeValue(x, f.Type().In(0))
y = sanitizeValue(y, f.Type().In(1))
if !s.dynChecker.Next() {
return f.Call([]reflect.Value{x, y})[0].Bool()
}
Expand Down Expand Up @@ -372,6 +375,18 @@ func detectRaces(c chan<- reflect.Value, f reflect.Value, vs ...reflect.Value) {
ret = f.Call(vs)[0]
}

// sanitizeValue converts nil interfaces of type T to those of type R,
// assuming that T is assignable to R.
// Otherwise, it returns the input value as is.
func sanitizeValue(v reflect.Value, t reflect.Type) reflect.Value {
// TODO(dsnet): Remove this hacky workaround.
// See https://golang.org/issue/22143
if v.Kind() == reflect.Interface && v.IsNil() && v.Type() != t {
return reflect.New(t).Elem()
}
return v
}

func (s *state) compareArray(vx, vy reflect.Value, t reflect.Type) {
step := &sliceIndex{pathStep{t.Elem()}, 0, 0}
s.curPath.push(step)
Expand Down
34 changes: 34 additions & 0 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ func TestDiff(t *testing.T) {
func comparerTests() []test {
const label = "Comparer"

type Iface1 interface {
Method()
}
type Iface2 interface {
Method()
}

type tarHeader struct {
Name string
Mode int64
Expand Down Expand Up @@ -419,6 +426,33 @@ root:
label: label,
x: []*pb.Stringer{{`multi\nline\nline\nline`}},
wantDiff: ":\n\t-: []*testprotos.Stringer{s`multi\\nline\\nline\\nline`}\n\t+: <non-existent>",
}, {
label: label,
x: struct{ I Iface2 }{},
y: struct{ I Iface2 }{},
opts: []cmp.Option{
cmp.Comparer(func(x, y Iface1) bool {
return x == nil && y == nil
}),
},
}, {
label: label,
x: struct{ I Iface2 }{},
y: struct{ I Iface2 }{},
opts: []cmp.Option{
cmp.Transformer("", func(v Iface1) bool {
return v == nil
}),
},
}, {
label: label,
x: struct{ I Iface2 }{},
y: struct{ I Iface2 }{},
opts: []cmp.Option{
cmp.FilterValues(func(x, y Iface1) bool {
return x == nil && y == nil
}, cmp.Ignore()),
},
}}
}

Expand Down

0 comments on commit 7ffe192

Please sign in to comment.