Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reflect: StructOf interpretation of input StructFields is confusing #18780

Closed
rsc opened this issue Jan 24, 2017 · 1 comment
Closed

reflect: StructOf interpretation of input StructFields is confusing #18780

rsc opened this issue Jan 24, 2017 · 1 comment

Comments

@rsc
Copy link
Contributor

rsc commented Jan 24, 2017

Normally, if you have

type T struct {
	A1
	*A2
	a1
	*a2
	F int
	f int
}

then there are six reflect.StructFields:

reflect.StructField{Name: "A1", Anonymous:true, Type:TypeOf(A1{}), PkgPath: ""}
reflect.StructField{Name: "A2", Anonymous:true, Type:TypeOf(&A2{}), PkgPath: ""}
reflect.StructField{Name: "a1", Anonymous:true, Type:TypeOf(a1{}), PkgPath: "mypkg"}
reflect.StructField{Name: "a2", Anonymous:true, Type:TypeOf(&a2{}), PkgPath: "mypkg"}
reflect.StructField{Name: "F", Anonymous:false, Type:TypeOf(int(0)), PkgPath: ""}
reflect.StructField{Name: "f", Anonymous:false, Type:TypeOf(int(0)), PkgPath: "mypkg"}

However, these appear not to be the StructFields expected by reflect.StructOf.

  1. PkgPath != "" panics unconditionally. That appears to be a limitation, which I understand.
  2. Anonymous is completely ignored.
  3. Name == "" appears to set Anonymous==true.
  4. Name == "" with an unexported type appears to set PkgPath incorrectly (and not panic).

This does not match the usual meaning of reflect.StructField, and it's undocumented.

I changed TestStructOfExportRules to print the input and output StructFields:

#0: {Name: PkgPath: Type:reflect_test.S1 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:S1 PkgPath: Type:reflect_test.S1 Tag: Offset:0 Index:[0] Anonymous:true}
#1: {Name: PkgPath: Type:*reflect_test.S1 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:S1 PkgPath: Type:*reflect_test.S1 Tag: Offset:0 Index:[0] Anonymous:true}
#2: {Name: PkgPath: Type:reflect_test.s2 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:s2 PkgPath:runtime Type:reflect_test.s2 Tag: Offset:0 Index:[0] Anonymous:true}
#3: {Name: PkgPath: Type:*reflect_test.s2 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:s2 PkgPath:runtime Type:*reflect_test.s2 Tag: Offset:0 Index:[0] Anonymous:true}
#8: {Name:S PkgPath: Type:reflect_test.S1 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:S PkgPath: Type:reflect_test.S1 Tag: Offset:0 Index:[0] Anonymous:false}
#9: {Name:S PkgPath: Type:*reflect_test.S1 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:S PkgPath: Type:*reflect_test.S1 Tag: Offset:0 Index:[0] Anonymous:false}
#10: {Name:S PkgPath: Type:reflect_test.s2 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:S PkgPath: Type:reflect_test.s2 Tag: Offset:0 Index:[0] Anonymous:false}
#11: {Name:S PkgPath: Type:*reflect_test.s2 Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:S PkgPath: Type:*reflect_test.s2 Tag: Offset:0 Index:[0] Anonymous:false}
#20: {Name: PkgPath: Type:reflect_test.ΦType Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:ΦType PkgPath: Type:reflect_test.ΦType Tag: Offset:0 Index:[0] Anonymous:true}
#21: {Name: PkgPath: Type:reflect_test.φType Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:φType PkgPath:runtime Type:reflect_test.φType Tag: Offset:0 Index:[0] Anonymous:true}
#22: {Name:Φ PkgPath: Type:int Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:Φ PkgPath: Type:int Tag: Offset:0 Index:[0] Anonymous:false}
#23: {Name:φ PkgPath: Type:int Tag: Offset:0 Index:[] Anonymous:false} =>
	{Name:φ PkgPath: Type:int Tag: Offset:0 Index:[0] Anonymous:false}

Note that in case 2, the input struct field says Anonymous false but the missing name triggers Anonymous true.

Note that in case 2, the input struct field has an empty PkgPath, but the missing name triggers setting PkgPath="runtime" (incorrectly, since the type is from package reflect_test).

I believe we should change StructOf so that if you have a struct type t, either StructOf([]StructField{t.Field(i)}) works and produces exactly the same field meaning or else it panics (if it's not implementable). If an input StructField is invalid (for example, it has no name), StructOf should panic.

I'm not sure what will break as a result of this; I hope not much, but the current situation is undocumented and contradicts the rest of the package, so a little breakage seems justifiable, and I can't - even with backwards compatibility hacks - make StructOf([]StructField{t.Field(i)}) work properly for embedded alias fields the way things are now.

For #18130.

/cc @sbinet

@rsc rsc added this to the Go1.9Early milestone Jan 24, 2017
@rsc rsc self-assigned this Jan 24, 2017
@gopherbot
Copy link
Contributor

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

@golang golang locked and limited conversation to collaborators Jan 31, 2018
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants