Skip to content

reflect: StructOf isn't behaving properly with methods on embedded structs #20824

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.

Sign up for GitHub

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

Jump to bottom
Closed
jonbodner opened this issue Jun 28, 2017 · 9 comments
Closed

reflect: StructOf isn't behaving properly with methods on embedded structs #20824

jonbodner opened this issue Jun 28, 2017 · 9 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.9Maybe

Comments

@jonbodner
Copy link

jonbodner commented Jun 28, 2017 •
edited
Loading

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.8.3

What operating system and processor architecture are you using (go env)?

Visible on Go Playground and on Mac OS
$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mbh475/go_projects"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/xf/5x4cmml91xn880l6v0c9l1dxnrqmjk/T/go-build686415997=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

I wrote a small example to show the difference between delegating a method call to an embedded struct normally and doing so via reflection using reflect.StructOf. I expected that embedding via reflection would not work, and would panic at runtime when I tried to cast the Interface(). What happened instead is that I was able to not only cast successfully, I could invoke the method and it would use data in a different embedded struct field.

Links:

Two links, one with two embedded structs, where it's exposing the method from the second struct and using the value from the first struct:

https://play.golang.org/p/TWApy-JibG

The second has one embedded struct, one embedded pointer to a struct, again the method from the second is being used with the data from the first, but this time the data is a string not an int:

https://play.golang.org/p/Dq9Y8DAkvA

What did you expect to see?

In both cases, I expected a panic when I tried to cast the generated struct to the interface that's implemented by the embedded struct.

What did you see instead?

Not only did the cast work, but invoking the method worked as well. In the second case, it's multiplying a string by 2 and producing a nonsense result.
@bradfitz bradfitz changed the title reflect.StructOf isn't behaving properly with methods on embedded structs reflect: StructOf isn't behaving properly with methods on embedded structs Jun 28, 2017
@bradfitz
Copy link
Contributor

Out of curiosity, have you tried Go 1.9beta2?

@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 28, 2017
@bradfitz
Copy link
Contributor

Actually, I just tested.

In both examples, with Go 1.9beta2 I get:

panic: reflect.StructOf: field 0 has no name
        
goroutine 1 [running]:
reflect.StructOf(0xc420041e38, 0x3, 0x3, 0x0, 0x0)
        /home/bradfitz/go/src/reflect/type.go:2401 +0x3404
main.main()
        /home/bradfitz/reflect1.go:50 +0x562
exit status 2

So maybe this is fixed. I haven't read the repro code in detail yet.

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 28, 2017
@bradfitz
Copy link
Contributor

@jonbodner, is Go 1.9's behavior what you expect?

@bradfitz bradfitz added this to the Unplanned milestone Jun 28, 2017
@kashav
Copy link
Contributor

kashav commented Jun 28, 2017

From #20632 (comment):

The current situation is that it sometimes works and sometimes doesn't. Try adding another field to your StructOf call, such that the interface is not the first field. Horrible things will happen.

I was a little curious about where the value for f.A was coming from (in the bar2impl.Double call), after playing around with the order of the fields in the StructOf call, it appears that it's coming from the first field of the first element (i.e. Baz.D in your examples). What's even weirder is that attempting to print either f or f.B in the call results in a panic, but f.A can be printed just fine.

@kashav
Copy link
Contributor

kashav commented Jun 28, 2017 •
edited
Loading

@bradfitz I was able to repro this on 1.9beta2 after adding the name fields.

@bradfitz
Copy link
Contributor

@kshvmdn, thanks.

Closing as a dup of #15924. I'll drop a reference to this bug there.

@bradfitz bradfitz mentioned this issue Jun 28, 2017
Open
@ianlancetaylor
Copy link
Member

I'm going to reopen this, because I think this is a special case that is worse than #15924. #15924 is about the fact that we don't generate methods that we should. The problem here is that we are keeping some methods, but those methods work incorrectly.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.9Maybe, Unplanned Jun 28, 2017
@ianlancetaylor ianlancetaylor removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 28, 2017
@bradfitz
Copy link
Contributor

/cc @crawshaw @mdempsky @josharian

@gopherbot
Copy link
Contributor

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

@golang golang locked and limited conversation to collaborators Jul 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.9Maybe
Development

No branches or pull requests
5 participants
@bradfitz @jonbodner @ianlancetaylor @kashav @gopherbot