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/gob: ignore func and chan values during encoding #6071

Closed
rsc opened this issue Aug 7, 2013 · 7 comments
Closed

encoding/gob: ignore func and chan values during encoding #6071

rsc opened this issue Aug 7, 2013 · 7 comments
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Aug 7, 2013

We have been assuming that we can add new fields to existing structs as long as the zero
value of the field means "the old behavior".

This is not strictly true for gob, because the new field may contain a type that will
cause gob to reject a struct that it formerly accepted.

I ran across this because I added a Client *http.Client to a struct that I had forgotten
was being gob-encoded, and http.Client contains a func-valued field, which caused gob to
refuse to encode any instance of that struct.

I am not sure what, if anything, we should do, but I wanted to record the fact as
something we should think about. It would be good to find some way to avoid making
existing gob-compatible structs gob-incompatible in future releases. Perhaps the API
tool should reject new additions of gob-incompatible fields, although that
determinination requires recursive traversal of the content of the type of the new
field. Perhaps gob should change to be more lax. It's unclear.
@robpike
Copy link
Contributor

@robpike robpike commented Sep 12, 2013

Comment 1:

Perhaps gob should just be more forgiving. I thought there was a bug filed about this
but there's only one about nested unmarshalable structs, which is a separate issue.
I propose that gob treat chan and func fields exactly as it now treats unexported
fields. If there is no custom encoder for that field, it is simply ignored.
It's an easy, backwards-compatible change that will take care of a large subset of the
compatibility issue.
@rsc
Copy link
Contributor Author

@rsc rsc commented Sep 13, 2013

Comment 2:

Seems like the best we can do.
@rsc
Copy link
Contributor Author

@rsc rsc commented Sep 13, 2013

Comment 3:

Rob, do you intend to do this for Go 1.2? I just wanted to make sure we made a decision.
I am happy to mark this Go1.3 now that we know what the plan is.

Labels changed: added go1.2maybe, removed go1.2.

@robpike
Copy link
Contributor

@robpike robpike commented Sep 13, 2013

Comment 5:

I'm happy with it going into 1.2. It's a small change I trust. But if you'd rather hold
it off, that's OK.
@rsc
Copy link
Contributor Author

@rsc rsc commented Sep 13, 2013

Comment 6:

Sounds good to me. I looked for but somehow hadn't seen the CL when I added
the comment here.
@rsc
Copy link
Contributor Author

@rsc rsc commented Sep 13, 2013

Comment 7:

(That is, it's fine to submit.)
@robpike
Copy link
Contributor

@robpike robpike commented Sep 16, 2013

Comment 8:

This issue was closed by revision 1fba73d.

Status changed to Fixed.

@rsc rsc added fixed labels Sep 16, 2013
@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2maybe label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.