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

Field with deserialize_with should not implement Deserialize #311

Merged
merged 1 commit into from May 11, 2016
Merged

Field with deserialize_with should not implement Deserialize #311

merged 1 commit into from May 11, 2016

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented May 9, 2016

Addresses #259.

The solution is, if a field is missing:

  • If there is a default expression, use the default expression (this includes skip_deserializing).
  • If the field has deserialize_with, fail with Visitor::Error::missing_field(name).
  • Otherwise call visitor.missing_field(name) to possibly handle the field.

This means deserialize_with no longer goes through the visitor's missing_field, instead failing directly unless there is a default. Thus fields with deserialize_with do not require Deserialize as a bound. The missing field behavior can be controlled by setting a default for the field.

@dtolnay
Copy link
Member Author

dtolnay commented May 9, 2016

The Travis failure is due to #313.

@dtolnay
Copy link
Member Author

dtolnay commented May 10, 2016

Rebased, per #315 (comment).

@oli-obk
Copy link
Member

oli-obk commented May 11, 2016

took me a while to grok it, but the implementation is sound.

I'm fine with the design, and can't think of another one until specialization stabilizes (so missing_field can handle fields that don't implement Deserialize)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants