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

encoding/xml: lacks support for decoding arrays #20709

Open
ITR13 opened this issue Jun 16, 2017 · 6 comments
Open

encoding/xml: lacks support for decoding arrays #20709

ITR13 opened this issue Jun 16, 2017 · 6 comments
Labels
FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@ITR13
Copy link

ITR13 commented Jun 16, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8.1 windows/amd64

What did you do?

Tried to store a struct containing [2]int as xml, then load it:
https://play.golang.org/p/TDMcHlf3fn

What did you expect to see?

{[0 255]} {[255 255]}

What did you see instead?

panic: unknown type [2]uint8

goroutine 1 [running]:
main.main()
	/tmp/sandbox393989733/main.go:22 +0x260

Additional Notes

There is no case reflect.Array: in (p *Decoder) unmarshal in "encoding/xml/read.go"
Encoding arrays works
Decoding arrays as slices works

@bradfitz bradfitz changed the title encoding/xml lacks support for decoding arrays encoding/xml: lacks support for decoding arrays Jun 16, 2017
@bradfitz bradfitz added FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 16, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jun 16, 2017
@leighmcculloch
Copy link
Contributor

As a comparison to slices, which work: https://play.golang.org/p/4Jo05Hdq-m

@leighmcculloch
Copy link
Contributor

Encoding arrays works
Decoding arrays as slices works

Given it is possible to encode arrays, I would expect it should be possible to decode arrays.

However, if the package supported decoding arrays these scenarios aren't completely obvious:

More XML elements than the array's length

Should it error, or ignore the remaining?

I would probably expect it to error since the data being decoded would not be of the same type, where the type is an array of specific length and the data won't fit into an array of that length.

Counter to that though, what if an application didn't care about additional elements?

Less XML elements than the array's length

Should it error, or leave the extra elements in the array as zero values?

I would probably expect it to leave the extra elements in the array as zero values, given that when fields in a struct are missing in data being decoded they remain as their zero value.

Counter to that though, what if an application needed to distinguish between zero values and an incomplete list?

@ianlancetaylor
Copy link
Contributor

I think that if we add support for decoding arrays, it should return an error if the number of elements is not exactly the length of the array. If you aren't sure of how exactly many elements you are getting, you should be using a slice, not an array.

@leighmcculloch
Copy link
Contributor

👍 That makes sense to me. It simplifies the story that has to be told about its behavior since all scenarios have the same requirement - all or nothing.

If this gets approved I'm happy to put together a CL for it if @ITR13 want planning to.

@ITR13
Copy link
Author

ITR13 commented Oct 29, 2017

Sounds good to me, what's a CL?

@leighmcculloch
Copy link
Contributor

A CL (change list) is a set of changes pushed to Gerrit for review, like a PR (pull request) on GitHub.

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Jun 13, 2018
@rsc rsc modified the milestones: Go1.12, Go1.13 Nov 14, 2018
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

6 participants