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

cmd/compile: commit d349fa25 breaks unmarshaling public fields of embedded private structs #22546

Closed
wardn opened this issue Nov 2, 2017 · 2 comments

Comments

Projects
None yet
4 participants
@wardn
Copy link

commented Nov 2, 2017

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

commit 812b34e: good
commit d349fa2: bad

Does this issue reproduce with the latest release?

the issue reproduces on master, it is not on the 1.9 release branch

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

freebsd/amd64
linux/amd64
darwin/amd64
windows/amd64

What did you do?

https://play.golang.org/p/6ABGl7lNJA

package main

import (
	"encoding/json"
)

func main() {
	b := []byte(`{"ID": "1"}`)
	var m Manager
	json.Unmarshal(b, &m)
	println(m.ID)
}

type Manager struct {
	*employee
}

type employee struct {
	ID string
}

What did you expect to see?

1

What did you see instead?

panic: reflect: reflect.Value.Set using value obtained using unexported field [recovered]
panic: interface conversion: string is not error: missing method Error

goroutine 1 [running]:
encoding/json.(*decodeState).unmarshal.func1(0xc42003bed8)
/usr/home/user/dev/golang/src/encoding/json/decode.go:175 +0x8f
panic(0x4b20c0, 0xc420072210)
/usr/home/user/dev/golang/src/runtime/panic.go:492 +0x266
reflect.flag.mustBeAssignable(0x1d6)
/usr/home/user/dev/golang/src/reflect/value.go:224 +0x200
reflect.Value.Set(0x4aca20, 0xc42008c018, 0x1d6, 0x4aca20, 0xc420072200, 0x16)
/usr/home/user/dev/golang/src/reflect/value.go:1344 +0x2f
encoding/json.(*decodeState).object(0xc42009a000, 0x4ac9e0, 0xc42008c018, 0x16)
/usr/home/user/dev/golang/src/encoding/json/decode.go:707 +0x3fa
encoding/json.(*decodeState).value(0xc42009a000, 0x4ac9e0, 0xc42008c018, 0x16)
/usr/home/user/dev/golang/src/encoding/json/decode.go:405 +0x2d8
encoding/json.(*decodeState).unmarshal(0xc42009a000, 0x4ac9e0, 0xc42008c018, 0x0, 0x0)
/usr/home/user/dev/golang/src/encoding/json/decode.go:187 +0x201
encoding/json.Unmarshal(0xc420086050, 0xb, 0x10, 0x4ac9e0, 0xc42008c018, 0x10, 0x49f939)
/usr/home/user/dev/golang/src/encoding/json/decode.go:107 +0x148
main.main()
/home/user/dev/go/src/serializer/main.go:10 +0xa

@odeke-em

This comment has been minimized.

Copy link
Member

commented Nov 2, 2017

Thank you for the report @wardn!

/cc @mdempsky

@odeke-em odeke-em added this to the Go1.10 milestone Nov 2, 2017

@odeke-em odeke-em added the NeedsFix label Nov 2, 2017

@dsnet

This comment has been minimized.

Copy link
Member

commented Nov 2, 2017

Duplicate of #21357.

This is (sort of) working as intended. The json package was relying on a bug in the reflect package. @mdempsky fixed that bug. The work to be done is to get json.Unmarshal to not panic.

Fundamentally, it is impossible for json to unmarshal into Manager.employee because it can't assign into an unexported field. The Go language spec forbids that.

@dsnet dsnet closed this Nov 2, 2017

@golang golang locked and limited conversation to collaborators Nov 2, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.