Skip to content

Commit

Permalink
reflect: make StructOf panic for methods that don't work
Browse files Browse the repository at this point in the history
When StructOf is used with an anonymous field that has methods, and
that anonymous field is not the first field, the methods we generate
are incorrect because they do not offset to the field as required.
If we encounter that case, panic rather than doing the wrong thing.

Fixes #20824
Updates #15924

Change-Id: I3b0901ddbc6d58af5f7e84660b5e3085a431035d
Reviewed-on: https://go-review.googlesource.com/47035
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
  • Loading branch information
ianlancetaylor committed Jul 15, 2017
1 parent 792f9c9 commit 87d5f6b
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 32 deletions.
86 changes: 56 additions & 30 deletions src/reflect/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4686,41 +4686,67 @@ func TestStructOfWithInterface(t *testing.T) {
}

for i, table := range tests {
rt := StructOf(
[]StructField{
{
Name: table.name,
Anonymous: true,
PkgPath: "",
Type: table.typ,
},
},
)
rv := New(rt).Elem()
rv.Field(0).Set(table.val)
for j := 0; j < 2; j++ {
var fields []StructField
if j == 1 {
fields = append(fields, StructField{
Name: "Dummy",
PkgPath: "",
Type: TypeOf(int(0)),
})
}
fields = append(fields, StructField{
Name: table.name,
Anonymous: true,
PkgPath: "",
Type: table.typ,
})

if _, ok := rv.Interface().(Iface); ok != table.impl {
if table.impl {
t.Errorf("test-%d: type=%v fails to implement Iface.\n", i, table.typ)
} else {
t.Errorf("test-%d: type=%v should NOT implement Iface\n", i, table.typ)
// We currently do not correctly implement methods
// for anonymous fields other than the first.
// Therefore, for now, we expect those methods
// to not exist. See issues 15924 and 20824.
// When those issues are fixed, this test of panic
// should be removed.
if j == 1 && table.impl {
func() {
defer func() {
if err := recover(); err == nil {
t.Errorf("test-%d-%d did not panic", i, j)
}
}()
_ = StructOf(fields)
}()
continue
}
continue
}

if !table.impl {
continue
}
rt := StructOf(fields)
rv := New(rt).Elem()
rv.Field(j).Set(table.val)

v := rv.Interface().(Iface).Get()
if v != want {
t.Errorf("test-%d: x.Get()=%v. want=%v\n", i, v, want)
}
if _, ok := rv.Interface().(Iface); ok != table.impl {
if table.impl {
t.Errorf("test-%d-%d: type=%v fails to implement Iface.\n", i, j, table.typ)
} else {
t.Errorf("test-%d-%d: type=%v should NOT implement Iface\n", i, j, table.typ)
}
continue
}

fct := rv.MethodByName("Get")
out := fct.Call(nil)
if !DeepEqual(out[0].Interface(), want) {
t.Errorf("test-%d: x.Get()=%v. want=%v\n", i, out[0].Interface(), want)
if !table.impl {
continue
}

v := rv.Interface().(Iface).Get()
if v != want {
t.Errorf("test-%d-%d: x.Get()=%v. want=%v\n", i, j, v, want)
}

fct := rv.MethodByName("Get")
out := fct.Call(nil)
if !DeepEqual(out[0].Interface(), want) {
t.Errorf("test-%d-%d: x.Get()=%v. want=%v\n", i, j, out[0].Interface(), want)
}
}
}
}
Expand Down
15 changes: 13 additions & 2 deletions src/reflect/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -2434,7 +2434,7 @@ func StructOf(fields []StructField) Type {
ift := (*interfaceType)(unsafe.Pointer(ft))
for im, m := range ift.methods {
if ift.nameOff(m.name).pkgPath() != "" {
// TODO(sbinet)
// TODO(sbinet). Issue 15924.
panic("reflect: embedded interface with unexported method(s) not implemented")
}

Expand Down Expand Up @@ -2492,10 +2492,15 @@ func StructOf(fields []StructField) Type {
case Ptr:
ptr := (*ptrType)(unsafe.Pointer(ft))
if unt := ptr.uncommon(); unt != nil {
if i > 0 && unt.mcount > 0 {
// Issue 15924.
panic("reflect: embedded type with methods not implemented if type is not first field")
}
for _, m := range unt.methods() {
mname := ptr.nameOff(m.name)
if mname.pkgPath() != "" {
// TODO(sbinet)
// TODO(sbinet).
// Issue 15924.
panic("reflect: embedded interface with unexported method(s) not implemented")
}
methods = append(methods, method{
Expand All @@ -2511,6 +2516,7 @@ func StructOf(fields []StructField) Type {
mname := ptr.nameOff(m.name)
if mname.pkgPath() != "" {
// TODO(sbinet)
// Issue 15924.
panic("reflect: embedded interface with unexported method(s) not implemented")
}
methods = append(methods, method{
Expand All @@ -2523,10 +2529,15 @@ func StructOf(fields []StructField) Type {
}
default:
if unt := ft.uncommon(); unt != nil {
if i > 0 && unt.mcount > 0 {
// Issue 15924.
panic("reflect: embedded type with methods not implemented if type is not first field")
}
for _, m := range unt.methods() {
mname := ft.nameOff(m.name)
if mname.pkgPath() != "" {
// TODO(sbinet)
// Issue 15924.
panic("reflect: embedded interface with unexported method(s) not implemented")
}
methods = append(methods, method{
Expand Down

0 comments on commit 87d5f6b

Please sign in to comment.