reflect: prevent additional StructOf embedded method cases

The current implementation does not generate wrappers for methods of
embedded non-interface types. We can only skip the wrapper if
kindDirectIface of the generated struct type matches kindDirectIface
of the embedded type. Panic if that is not the case.

It would be better to actually generate wrappers, but that can be done
later.

Updates #15924
Fixes #24782

Change-Id: I01f5c76d9a07f44e1b04861bfe9f9916a04e65ca
Reviewed-on: https://go-review.googlesource.com/121316
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 63d068cd78c76f6e9c1f50f5e169f902efdabf1a..cf7fe3cf7ae72746a0ea6a027469e69bc3152e82 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -4817,13 +4817,29 @@ func (i StructI) Get() int { return int(i) }
 
 type StructIPtr int
 
-func (i *StructIPtr) Get() int { return int(*i) }
+func (i *StructIPtr) Get() int  { return int(*i) }
+func (i *StructIPtr) Set(v int) { *(*int)(i) = v }
+
+type SettableStruct struct {
+       SettableField int
+}
+
+func (p *SettableStruct) Set(v int) { p.SettableField = v }
+
+type SettablePointer struct {
+       SettableField *int
+}
+
+func (p *SettablePointer) Set(v int) { *p.SettableField = v }
 
 func TestStructOfWithInterface(t *testing.T) {
        const want = 42
        type Iface interface {
                Get() int
        }
+       type IfaceSet interface {
+               Set(int)
+       }
        tests := []struct {
                name string
                typ  Type
@@ -4931,6 +4947,76 @@ func TestStructOfWithInterface(t *testing.T) {
                        }
                }
        }
+
+       // Test an embedded nil pointer with pointer methods.
+       fields := []StructField{{
+               Name:      "StructIPtr",
+               Anonymous: true,
+               Type:      PtrTo(TypeOf(StructIPtr(want))),
+       }}
+       rt := StructOf(fields)
+       rv := New(rt).Elem()
+       // This should panic since the pointer is nil.
+       shouldPanic(func() {
+               rv.Interface().(IfaceSet).Set(want)
+       })
+
+       // Test an embedded nil pointer to a struct with pointer methods.
+
+       fields = []StructField{{
+               Name:      "SettableStruct",
+               Anonymous: true,
+               Type:      PtrTo(TypeOf(SettableStruct{})),
+       }}
+       rt = StructOf(fields)
+       rv = New(rt).Elem()
+       // This should panic since the pointer is nil.
+       shouldPanic(func() {
+               rv.Interface().(IfaceSet).Set(want)
+       })
+
+       // The behavior is different if there is a second field,
+       // since now an interface value holds a pointer to the struct
+       // rather than just holding a copy of the struct.
+       fields = []StructField{
+               {
+                       Name:      "SettableStruct",
+                       Anonymous: true,
+                       Type:      PtrTo(TypeOf(SettableStruct{})),
+               },
+               {
+                       Name:      "EmptyStruct",
+                       Anonymous: true,
+                       Type:      StructOf(nil),
+               },
+       }
+       // With the current implementation this is expected to panic.
+       // Ideally it should work and we should be able to see a panic
+       // if we call the Set method.
+       shouldPanic(func() {
+               StructOf(fields)
+       })
+
+       // Embed a field that can be stored directly in an interface,
+       // with a second field.
+       fields = []StructField{
+               {
+                       Name:      "SettablePointer",
+                       Anonymous: true,
+                       Type:      TypeOf(SettablePointer{}),
+               },
+               {
+                       Name:      "EmptyStruct",
+                       Anonymous: true,
+                       Type:      StructOf(nil),
+               },
+       }
+       // With the current implementation this is expected to panic.
+       // Ideally it should work and we should be able to call the
+       // Set and Get methods.
+       shouldPanic(func() {
+               StructOf(fields)
+       })
 }
 
 func TestChanOf(t *testing.T) {

diff --git a/src/reflect/type.go b/src/reflect/type.go
index 1f3b665ce4da9f61b5edaaea266903a440d87d2b..a7d660fbefd4780b6912fc7f1e0a3ea7fd51554e 100644 (file)
--- a/src/reflect/type.go
+++ b/src/reflect/type.go
@@ -2467,6 +2467,9 @@ func StructOf(fields []StructField) Type {
                                                // Issue 15924.
                                                panic("reflect: embedded type with methods not implemented if type is not first field")
                                        }
+                                       if len(fields) > 1 {
+                                               panic("reflect: embedded type with methods not implemented if there is more than one field")
+                                       }
                                        for _, m := range unt.methods() {
                                                mname := ptr.nameOff(m.name)
                                                if mname.pkgPath() != "" {
@@ -2504,6 +2507,9 @@ func StructOf(fields []StructField) Type {
                                                // Issue 15924.
                                                panic("reflect: embedded type with methods not implemented if type is not first field")
                                        }
+                                       if len(fields) > 1 && ft.kind&kindDirectIface != 0 {
+                                               panic("reflect: embedded type with methods not implemented for non-pointer type")
+                                       }
                                        for _, m := range unt.methods() {
                                                mname := ft.nameOff(m.name)
                                                if mname.pkgPath() != "" {