From 87d5f6b9f65bc0f4a68d8712b500615df5df309c Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 28 Jun 2017 12:47:08 -0700 Subject: reflect: make StructOf panic for methods that don't work 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/reflect/all_test.go | 86 ++++++++++++++++++++++++++++++++----------------- src/reflect/type.go | 15 +++++++-- 2 files changed, 69 insertions(+), 32 deletions(-) (limited to 'src/reflect') diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index 308cb77a8f..5a5c91b751 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -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) + } } } } diff --git a/src/reflect/type.go b/src/reflect/type.go index 14c16fc832..cbf4f73bf4 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -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") } @@ -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{ @@ -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{ @@ -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{ -- cgit v1.2.1