cmd/compile: fix and improve struct field reflect information

The previous logic was overly complicated, generated suboptimally
encoded struct type descriptors, and mishandled embeddings of
predeclared universal types.

Fixes #21122.
Fixes #21353.
Fixes #21696.
Fixes #21702.
Updates #21357.

Change-Id: If34761fa6dbe4af2af59dee501e7f30845320376
Reviewed-on: https://go-review.googlesource.com/60410
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>

diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go
index c436c55c6ae41e92da2b8ca817e62893d8ca8fe9..c5730dbcb80a4678e0aa729dc3b3bb14c1b98894 100644 (file)
--- a/src/cmd/compile/internal/gc/reflect.go
+++ b/src/cmd/compile/internal/gc/reflect.go
@@ -568,30 +568,12 @@ func dgopkgpathOff(s *obj.LSym, ot int, pkg *types.Pkg) int {
        return dsymptrOff(s, ot, pkg.Pathsym, 0)
 }
 
-// isExportedField reports whether a struct field is exported.
-// It also returns the package to use for PkgPath for an unexported field.
-func isExportedField(ft *types.Field) (bool, *types.Pkg) {
-       if ft.Sym != nil && ft.Embedded == 0 {
-               return exportname(ft.Sym.Name), ft.Sym.Pkg
-       }
-       if ft.Type.Sym != nil &&
-               (ft.Type.Sym.Pkg == builtinpkg || !exportname(ft.Type.Sym.Name)) {
-               return false, ft.Type.Sym.Pkg
-       }
-       return true, nil
-}
-
 // dnameField dumps a reflect.name for a struct field.
 func dnameField(lsym *obj.LSym, ot int, spkg *types.Pkg, ft *types.Field) int {
-       var name string
-       if ft.Sym != nil {
-               name = ft.Sym.Name
-       }
-       isExported, fpkg := isExportedField(ft)
-       if isExported || fpkg == spkg {
-               fpkg = nil
+       if !exportname(ft.Sym.Name) && ft.Sym.Pkg != spkg {
+               Fatalf("package mismatch for %v", ft.Sym)
        }
-       nsym := dname(name, ft.Note, fpkg, isExported)
+       nsym := dname(ft.Sym.Name, ft.Note, nil, exportname(ft.Sym.Name))
        return dsymptr(lsym, ot, nsym, 0)
 }
 
@@ -1356,21 +1338,21 @@ ok:
                        n++
                }
 
-               ot = dcommontype(lsym, ot, t)
-               pkg := localpkg
-               if t.Sym != nil {
-                       pkg = t.Sym.Pkg
-               } else {
-                       // Unnamed type. Grab the package from the first field, if any.
-                       for _, f := range t.Fields().Slice() {
-                               if f.Embedded != 0 {
-                                       continue
-                               }
-                               pkg = f.Sym.Pkg
+               // All non-exported struct field names within a struct
+               // type must originate from a single package. By
+               // identifying and recording that package within the
+               // struct type descriptor, we can omit that
+               // information from the field descriptors.
+               var spkg *types.Pkg
+               for _, f := range t.Fields().Slice() {
+                       if !exportname(f.Sym.Name) {
+                               spkg = f.Sym.Pkg
                                break
                        }
                }
-               ot = dgopkgpath(lsym, ot, pkg)
+
+               ot = dcommontype(lsym, ot, t)
+               ot = dgopkgpath(lsym, ot, spkg)
                ot = dsymptr(lsym, ot, lsym, ot+3*Widthptr+uncommonSize(t))
                ot = duintptr(lsym, ot, uint64(n))
                ot = duintptr(lsym, ot, uint64(n))
@@ -1380,7 +1362,7 @@ ok:
 
                for _, f := range t.Fields().Slice() {
                        // ../../../../runtime/type.go:/structField
-                       ot = dnameField(lsym, ot, pkg, f)
+                       ot = dnameField(lsym, ot, spkg, f)
                        ot = dsymptr(lsym, ot, dtypesym(f.Type).Linksym(), 0)
                        offsetAnon := uint64(f.Offset) << 1
                        if offsetAnon>>1 != uint64(f.Offset) {

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 962c326c036ae8ee91ff5691792055f3c4ddd6b1..7c83364a4560f812cefb59abf6d536fac81528aa 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -321,6 +321,89 @@ func TestSetValue(t *testing.T) {
        }
 }
 
+func TestCanSetField(t *testing.T) {
+       type embed struct{ x, X int }
+       type Embed struct{ x, X int }
+       type S1 struct {
+               embed
+               x, X int
+       }
+       type S2 struct {
+               *embed
+               x, X int
+       }
+       type S3 struct {
+               Embed
+               x, X int
+       }
+       type S4 struct {
+               *Embed
+               x, X int
+       }
+
+       type testCase struct {
+               index  []int
+               canSet bool
+       }
+       tests := []struct {
+               val   Value
+               cases []testCase
+       }{{
+               val: ValueOf(&S1{}),
+               cases: []testCase{
+                       {[]int{0}, false},
+                       {[]int{0, 0}, false},
+                       {[]int{0, 1}, true},
+                       {[]int{1}, false},
+                       {[]int{2}, true},
+               },
+       }, {
+               val: ValueOf(&S2{embed: &embed{}}),
+               cases: []testCase{
+                       {[]int{0}, false},
+                       {[]int{0, 0}, false},
+                       {[]int{0, 1}, true},
+                       {[]int{1}, false},
+                       {[]int{2}, true},
+               },
+       }, {
+               val: ValueOf(&S3{}),
+               cases: []testCase{
+                       {[]int{0}, true},
+                       {[]int{0, 0}, false},
+                       {[]int{0, 1}, true},
+                       {[]int{1}, false},
+                       {[]int{2}, true},
+               },
+       }, {
+               val: ValueOf(&S4{Embed: &Embed{}}),
+               cases: []testCase{
+                       {[]int{0}, true},
+                       {[]int{0, 0}, false},
+                       {[]int{0, 1}, true},
+                       {[]int{1}, false},
+                       {[]int{2}, true},
+               },
+       }}
+
+       for _, tt := range tests {
+               t.Run(tt.val.Type().Name(), func(t *testing.T) {
+                       for _, tc := range tt.cases {
+                               f := tt.val
+                               for _, i := range tc.index {
+                                       if f.Kind() == Ptr {
+                                               f = f.Elem()
+                                       }
+                                       f = f.Field(i)
+                               }
+                               if got := f.CanSet(); got != tc.canSet {
+                                       t.Errorf("CanSet() = %v, want %v", got, tc.canSet)
+                               }
+                       }
+               })
+       }
+}
+
 var _i = 7
 
 var valueToStringTests = []pair{
@@ -2357,10 +2440,13 @@ func TestImportPath(t *testing.T) {
 }
 
 func TestFieldPkgPath(t *testing.T) {
+       type x int
        typ := TypeOf(struct {
                Exported   string
                unexported string
                OtherPkgFields
+               int // issue 21702
+               *x  // issue 21122
        }{})
 
        type pkgpathTest struct {
@@ -2387,6 +2473,8 @@ func TestFieldPkgPath(t *testing.T) {
                {[]int{2}, "", true},              // OtherPkgFields
                {[]int{2, 0}, "", false},          // OtherExported
                {[]int{2, 1}, "reflect", false},   // otherUnexported
+               {[]int{3}, "reflect_test", true},  // int
+               {[]int{4}, "reflect_test", true},  // *x
        })
 
        type localOtherPkgFields OtherPkgFields

diff --git a/src/reflect/type.go b/src/reflect/type.go
index 9f02219c8e90e8895f422a63629e9532e7c125af..0ecc2b3bca8b60ae834f7bdd267862769b454a02 100644 (file)
--- a/src/reflect/type.go
+++ b/src/reflect/type.go
@@ -1216,10 +1216,7 @@ func (t *structType) Field(i int) (f StructField) {
        f.Name = p.name.name()
        f.Anonymous = p.anon()
        if !p.name.isExported() {
-               f.PkgPath = p.name.pkgPath()
-               if f.PkgPath == "" {
-                       f.PkgPath = t.pkgPath.name()
-               }
+               f.PkgPath = t.pkgPath.name()
        }
        if tag := p.name.tag(); tag != "" {
                f.Tag = StructTag(tag)
@@ -1677,6 +1674,9 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool {
                if len(t.fields) != len(v.fields) {
                        return false
                }
+               if t.pkgPath.name() != v.pkgPath.name() {
+                       return false
+               }
                for i := range t.fields {
                        tf := &t.fields[i]
                        vf := &v.fields[i]
@@ -1692,19 +1692,6 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool {
                        if tf.offsetAnon != vf.offsetAnon {
                                return false
                        }
-                       if !tf.name.isExported() {
-                               tp := tf.name.pkgPath()
-                               if tp == "" {
-                                       tp = t.pkgPath.name()
-                               }
-                               vp := vf.name.pkgPath()
-                               if vp == "" {
-                                       vp = v.pkgPath.name()
-                               }
-                               if tp != vp {
-                                       return false
-                               }
-                       }
                }
                return true
        }

diff --git a/src/runtime/type.go b/src/runtime/type.go
index bf54d54eb43cd83b052b19fc2e53cc398d806910..b3df3353cecd6c6311a73cdb8548ee585fee57ae 100644 (file)
--- a/src/runtime/type.go
+++ b/src/runtime/type.go
@@ -655,15 +655,15 @@ func typesEqual(t, v *_type, seen map[_typePair]struct{}) bool {
                if len(st.fields) != len(sv.fields) {
                        return false
                }
+               if st.pkgPath.name() != sv.pkgPath.name() {
+                       return false
+               }
                for i := range st.fields {
                        tf := &st.fields[i]
                        vf := &sv.fields[i]
                        if tf.name.name() != vf.name.name() {
                                return false
                        }
-                       if tf.name.pkgPath() != vf.name.pkgPath() {
-                               return false
-                       }
                        if !typesEqual(tf.typ, vf.typ, seen) {
                                return false
                        }