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

Fix roundtrip of enum with skipped variant #2293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

djkoloski
Copy link

On enums with a skipped variant that is not the last one, the generated serialization and deserialization code map the variants of the enum to different variant indices. In deserialize_identifier, the variant index starts at 0 and counts up once per non-skipped variant. However, for serialization the index is always the index of the variant within the enum.

This PR corrects the deserialization code to always use the index of the variant within the enum.

This was surfaced in jamesmunns/postcard#79.

On enums with a skipped variant that is not the last one, the generated
serialization and deserialization code map the variants of the enum to
different variant indices. In `deserialize_identifier`, the variant
index starts at 0 and counts up once per non-skipped variant. However,
for serialization the index is always the index of the variant within
the enum.

This PR corrects the deserialization code to always use the index of the
variant within the enum.

This was surfaced in jamesmunns/postcard#79.
Copy link

@XBagon XBagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I patched my project with this fix and my serde test finally succeeded. Thank you! (Inoffical approve 😋)

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