reflect: Struct.PkgPath may be empty even if Name is unexported

[dsnet]

Since Go1.0, the following:

type x int
type S1 struct{ *x }
type S2 struct{ x }
fmt.Printf("%+v\n", reflect.TypeOf(S1{}).Field(0))
fmt.Printf("%+v\n", reflect.TypeOf(S2{}).Field(0))

Has printed the following:

{Name:x PkgPath: Type:*main.x Tag: Offset:0 Index:[0] Anonymous:true}
{Name:x PkgPath:main Type:main.x Tag: Offset:0 Index:[0] Anonymous:true}

Notice that StructField.PkgPath for an embedded unexported pointer type is empty, while it is populated for an unexported non-pointer type. This is surprising since the documentation for StructField.PkgPath says the following:

[PkgPath] is empty for upper case (exported) field names.

Although not explicitly stated, I would expect it to be non-empty for unexported field names.

We should either:

• Document the special-case that PkgPath may be empty for anonymous fields of pointer types.
• Fix StructField.PkgPath to be populated for embedded pointer types as well. However, this would change the behavior of what has been happening since Go1.0.

Given that the current behavior has persisted for 9 releases of Go, perhaps we should just document this? However, this difference in behavior has led to bugs (see #21121).

\cc @ianlancetaylor @rsc @crawshaw

[dsnet]

Some evidence in support of changing the behavior. The following all make the assumption that PkgPath=="" if and only if the field is exported.

• text/template/exec.go:572
• encoding/json/encode.go:1096
• encoding/xml/typeinfo.go:61
• encoding/asn1/asn1.go:862
• encoding/asn1/marshal.go:431

In the standard library, I did not find a single case where the current behavior is desired.

I glanced through a bunch of usages of this inside Google and it seems pervasive that people expect this property to be true.

[gopherbot]

CL https://golang.org/cl/50711 mentions this issue.

[gopherbot]

Change https://golang.org/cl/53643 mentions this issue: reflect: populate StructField.PkgPath on unexported embedded pointer types

[dsnet]

I ran my change through global testing (at Google) to detect regressions, and only 1 target failed as a result of this change.

The failure was in cloud.google.com/go/internal/fields, however I believe this is due to a different bug in reflect that is related (#21353).

Given that fixing this issue also fixes another bug in reflect, I have more confidence now that fixing this is the right action.

[gopherbot]

Change https://golang.org/cl/60410 mentions this issue: cmd/compile: fix and improve struct field reflect information

[gopherbot]

Change https://golang.org/cl/65550 mentions this issue: encoding/json: cleanup detection of unexported embedded fields