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

Defer bitfield decoding until its first use #803

Merged
merged 6 commits into from
Nov 6, 2020

Conversation

timvermeulen
Copy link
Contributor

This PR adds the UnvalidatedBitField type that may or may not have been validated for valid RLE+ yet. The existing BitField type still represents validated bitfields only.

Also adds the bitfield::Validate trait which is useful for when a function needs to be able to work with both validated and unvalidated bitfields.

Closes #735

Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

LGTM, but can you update the branch so I can make sure there is no regression? Hard to verify specifics with a few of the cases

Comment on lines 58 to 68
impl Serialize for UnvalidatedBitField {
fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
where
S: Serializer,
{
match self {
Self::Validated(bf) => serde_bytes::serialize(&bf.to_bytes(), serializer),
Self::Unvalidated(bytes) => serde_bytes::serialize(&bytes, serializer),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

optionally, can remove impl for this with serde untagged https://serde.rs/enum-representations.html#untagged (prob best to keep the deserialize either way)

@timvermeulen timvermeulen merged commit 1a44731 into main Nov 6, 2020
@timvermeulen timvermeulen deleted the tim/unvalidated-bitfield branch November 6, 2020 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate bitfields when they're first used, not at deserialization
3 participants