[dev.typealias] reflect: fix StructOf use of StructField to match StructField docs

The runtime internal structField interprets name=="" as meaning anonymous,
but the exported reflect.StructField has always set Name, even for anonymous
fields, and also set Anonymous=true.

The initial implementation of StructOf confused the internal and public
meanings of the StructField, expecting the runtime representation of
anonymous fields instead of the exported reflect API representation.
It also did not document this fact, so that users had no way to know how
to create an anonymous field.

This CL changes StructOf to use the previously documented interpretation
of reflect.StructField instead of an undocumented one.

The implementation of StructOf also, in some cases, allowed creating
structs with unexported fields (if you knew how to ask) but set the
PkgPath incorrectly on those fields. Rather than try to fix that, this CL
changes StructOf to reject attempts to create unexported fields.
(I think that may be the right design choice, not just a temporary limitation.
In any event, it's not the topic for today's work.)

For #17766.
Fixes #18780.

Change-Id: I585a4e324dc5a90551f49d21ae04d2de9ea04b6c
Reviewed-on: https://go-review.googlesource.com/35731
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 033a18171de96b966bc9f2233689dd5d49191d93..e057b0cfcc7c7002a553ec00e4d88fe053633f48 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -3689,7 +3689,7 @@ func checkSameType(t *testing.T, x, y interface{}) {
 
 func TestArrayOf(t *testing.T) {
        // check construction and use of type not in binary
-       for _, table := range []struct {
+       tests := []struct {
                n          int
                value      func(i int) interface{}
                comparable bool
@@ -3767,7 +3767,9 @@ func TestArrayOf(t *testing.T) {
                        comparable: true,
                        want:       "[{0 0} {1 1} {2 2} {3 3} {4 4} {5 5} {6 6} {7 7} {8 8} {9 9}]",
                },
-       } {
+       }
+
+       for _, table := range tests {
                at := ArrayOf(table.n, TypeOf(table.value(0)))
                v := New(at).Elem()
                vok := New(at).Elem()
@@ -4133,50 +4135,58 @@ func TestStructOfExportRules(t *testing.T) {
                f()
        }
 
-       for i, test := range []struct {
+       tests := []struct {
                field     StructField
                mustPanic bool
                exported  bool
        }{
                {
-                       field:     StructField{Name: "", Type: TypeOf(S1{})},
-                       mustPanic: false,
-                       exported:  true,
+                       field:    StructField{Name: "S1", Anonymous: true, Type: TypeOf(S1{})},
+                       exported: true,
                },
                {
-                       field:     StructField{Name: "", Type: TypeOf((*S1)(nil))},
-                       mustPanic: false,
-                       exported:  true,
+                       field:    StructField{Name: "S1", Anonymous: true, Type: TypeOf((*S1)(nil))},
+                       exported: true,
                },
                {
-                       field:     StructField{Name: "", Type: TypeOf(s2{})},
-                       mustPanic: false,
-                       exported:  false,
+                       field:     StructField{Name: "s2", Anonymous: true, Type: TypeOf(s2{})},
+                       mustPanic: true,
                },
                {
-                       field:     StructField{Name: "", Type: TypeOf((*s2)(nil))},
-                       mustPanic: false,
-                       exported:  false,
+                       field:     StructField{Name: "s2", Anonymous: true, Type: TypeOf((*s2)(nil))},
+                       mustPanic: true,
                },
                {
-                       field:     StructField{Name: "", Type: TypeOf(S1{}), PkgPath: "other/pkg"},
+                       field:     StructField{Name: "Name", Type: nil, PkgPath: ""},
                        mustPanic: true,
-                       exported:  true,
                },
                {
-                       field:     StructField{Name: "", Type: TypeOf((*S1)(nil)), PkgPath: "other/pkg"},
+                       field:     StructField{Name: "", Type: TypeOf(S1{}), PkgPath: ""},
+                       mustPanic: true,
+               },
+               {
+                       field:     StructField{Name: "S1", Anonymous: true, Type: TypeOf(S1{}), PkgPath: "other/pkg"},
+                       mustPanic: true,
+               },
+               {
+                       field:     StructField{Name: "S1", Anonymous: true, Type: TypeOf((*S1)(nil)), PkgPath: "other/pkg"},
+                       mustPanic: true,
+               },
+               {
+                       field:     StructField{Name: "s2", Anonymous: true, Type: TypeOf(s2{}), PkgPath: "other/pkg"},
                        mustPanic: true,
-                       exported:  true,
                },
                {
-                       field:     StructField{Name: "", Type: TypeOf(s2{}), PkgPath: "other/pkg"},
+                       field:     StructField{Name: "s2", Anonymous: true, Type: TypeOf((*s2)(nil)), PkgPath: "other/pkg"},
                        mustPanic: true,
-                       exported:  false,
                },
                {
-                       field:     StructField{Name: "", Type: TypeOf((*s2)(nil)), PkgPath: "other/pkg"},
+                       field:     StructField{Name: "s2", Type: TypeOf(int(0)), PkgPath: "other/pkg"},
+                       mustPanic: true,
+               },
+               {
+                       field:     StructField{Name: "s2", Type: TypeOf(int(0)), PkgPath: "other/pkg"},
                        mustPanic: true,
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "S", Type: TypeOf(S1{})},
@@ -4184,81 +4194,68 @@ func TestStructOfExportRules(t *testing.T) {
                        exported:  true,
                },
                {
-                       field:     StructField{Name: "S", Type: TypeOf((*S1)(nil))},
-                       mustPanic: false,
-                       exported:  true,
+                       field:    StructField{Name: "S", Type: TypeOf((*S1)(nil))},
+                       exported: true,
                },
                {
-                       field:     StructField{Name: "S", Type: TypeOf(s2{})},
-                       mustPanic: false,
-                       exported:  true,
+                       field:    StructField{Name: "S", Type: TypeOf(s2{})},
+                       exported: true,
                },
                {
-                       field:     StructField{Name: "S", Type: TypeOf((*s2)(nil))},
-                       mustPanic: false,
-                       exported:  true,
+                       field:    StructField{Name: "S", Type: TypeOf((*s2)(nil))},
+                       exported: true,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf(S1{})},
                        mustPanic: true,
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf((*S1)(nil))},
                        mustPanic: true,
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf(s2{})},
                        mustPanic: true,
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf((*s2)(nil))},
                        mustPanic: true,
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf(S1{}), PkgPath: "other/pkg"},
                        mustPanic: true, // TODO(sbinet): creating a name with a package path
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf((*S1)(nil)), PkgPath: "other/pkg"},
                        mustPanic: true, // TODO(sbinet): creating a name with a package path
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf(s2{}), PkgPath: "other/pkg"},
                        mustPanic: true, // TODO(sbinet): creating a name with a package path
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "s", Type: TypeOf((*s2)(nil)), PkgPath: "other/pkg"},
                        mustPanic: true, // TODO(sbinet): creating a name with a package path
-                       exported:  false,
                },
                {
                        field:     StructField{Name: "", Type: TypeOf(ΦType{})},
-                       mustPanic: false,
-                       exported:  true,
+                       mustPanic: true,
                },
                {
                        field:     StructField{Name: "", Type: TypeOf(φType{})},
-                       mustPanic: false,
-                       exported:  false,
+                       mustPanic: true,
                },
                {
-                       field:     StructField{Name: "Φ", Type: TypeOf(0)},
-                       mustPanic: false,
-                       exported:  true,
+                       field:    StructField{Name: "Φ", Type: TypeOf(0)},
+                       exported: true,
                },
                {
-                       field:     StructField{Name: "φ", Type: TypeOf(0)},
-                       mustPanic: false,
-                       exported:  false,
+                       field:    StructField{Name: "φ", Type: TypeOf(0)},
+                       exported: false,
                },
-       } {
+       }
+
+       for i, test := range tests {
                testPanic(i, test.mustPanic, func() {
                        typ := StructOf([]StructField{test.field})
                        if typ == nil {
@@ -4346,7 +4343,7 @@ func TestStructOfGenericAlg(t *testing.T) {
                {Name: "S1", Type: st1},
        })
 
-       for _, table := range []struct {
+       tests := []struct {
                rt  Type
                idx []int
        }{
@@ -4427,7 +4424,9 @@ func TestStructOfGenericAlg(t *testing.T) {
                        ),
                        idx: []int{2},
                },
-       } {
+       }
+
+       for _, table := range tests {
                v1 := New(table.rt).Elem()
                v2 := New(table.rt).Elem()
 
@@ -4529,18 +4528,21 @@ func TestStructOfWithInterface(t *testing.T) {
        type Iface interface {
                Get() int
        }
-       for i, table := range []struct {
+       tests := []struct {
+               name string
                typ  Type
                val  Value
                impl bool
        }{
                {
+                       name: "StructI",
                        typ:  TypeOf(StructI(want)),
                        val:  ValueOf(StructI(want)),
                        impl: true,
                },
                {
-                       typ: PtrTo(TypeOf(StructI(want))),
+                       name: "StructI",
+                       typ:  PtrTo(TypeOf(StructI(want))),
                        val: ValueOf(func() interface{} {
                                v := StructI(want)
                                return &v
@@ -4548,7 +4550,8 @@ func TestStructOfWithInterface(t *testing.T) {
                        impl: true,
                },
                {
-                       typ: PtrTo(TypeOf(StructIPtr(want))),
+                       name: "StructIPtr",
+                       typ:  PtrTo(TypeOf(StructIPtr(want))),
                        val: ValueOf(func() interface{} {
                                v := StructIPtr(want)
                                return &v
@@ -4556,6 +4559,7 @@ func TestStructOfWithInterface(t *testing.T) {
                        impl: true,
                },
                {
+                       name: "StructIPtr",
                        typ:  TypeOf(StructIPtr(want)),
                        val:  ValueOf(StructIPtr(want)),
                        impl: false,
@@ -4565,13 +4569,16 @@ func TestStructOfWithInterface(t *testing.T) {
                //      val:  ValueOf(StructI(want)),
                //      impl: true,
                // },
-       } {
+       }
+
+       for i, table := range tests {
                rt := StructOf(
                        []StructField{
                                {
-                                       Name:    "",
-                                       PkgPath: "",
-                                       Type:    table.typ,
+                                       Name:      table.name,
+                                       Anonymous: true,
+                                       PkgPath:   "",
+                                       Type:      table.typ,
                                },
                        },
                )
@@ -5951,6 +5958,7 @@ func TestSwapper(t *testing.T) {
                        want: []pairPtr{{5, 6, &c}, {3, 4, &b}, {1, 2, &a}},
                },
        }
+
        for i, tt := range tests {
                inStr := fmt.Sprint(tt.in)
                Swapper(tt.in)(tt.i, tt.j)

diff --git a/src/reflect/type.go b/src/reflect/type.go
index 9d6e7a6846781f7513760da833e01b9a56bf99a4..e0be36d9705c1a77974969633ddfbf3a23916f5e 100644 (file)
--- a/src/reflect/type.go
+++ b/src/reflect/type.go
@@ -2403,6 +2403,9 @@ func StructOf(fields []StructField) Type {
        lastzero := uintptr(0)
        repr = append(repr, "struct {"...)
        for i, field := range fields {
+               if field.Name == "" {
+                       panic("reflect.StructOf: field " + strconv.Itoa(i) + " has no name")
+               }
                if field.Type == nil {
                        panic("reflect.StructOf: field " + strconv.Itoa(i) + " has no type")
                }
@@ -2794,23 +2797,25 @@ func StructOf(fields []StructField) Type {
 }
 
 func runtimeStructField(field StructField) structField {
-       exported := field.PkgPath == ""
-       if field.Name == "" {
-               t := field.Type.(*rtype)
-               if t.Kind() == Ptr {
-                       t = t.Elem().(*rtype)
-               }
-               exported = t.nameOff(t.str).isExported()
-       } else if exported {
-               b0 := field.Name[0]
-               if ('a' <= b0 && b0 <= 'z') || b0 == '_' {
-                       panic("reflect.StructOf: field \"" + field.Name + "\" is unexported but has no PkgPath")
-               }
+       if field.PkgPath != "" {
+               panic("reflect.StructOf: StructOf does not allow unexported fields")
+       }
+
+       // Best-effort check for misuse.
+       // Since PkgPath is empty, not much harm done if Unicode lowercase slips through.
+       c := field.Name[0]
+       if 'a' <= c && c <= 'z' || c == '_' {
+               panic("reflect.StructOf: field \"" + field.Name + "\" is unexported but missing PkgPath")
+       }
+
+       name := field.Name
+       if field.Anonymous {
+               name = ""
        }
 
-       _ = resolveReflectType(field.Type.common())
+       resolveReflectType(field.Type.common()) // install in runtime
        return structField{
-               name:   newName(field.Name, string(field.Tag), field.PkgPath, exported),
+               name:   newName(name, string(field.Tag), "", true),
                typ:    field.Type.common(),
                offset: 0,
        }