Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed derive impl on an empty enum #462

Merged
merged 2 commits into from
Jan 8, 2022
Merged

Fixed derive impl on an empty enum #462

merged 2 commits into from
Jan 8, 2022

Conversation

VictorKoenders
Copy link
Contributor

@VictorKoenders VictorKoenders commented Jan 8, 2022

bincode-derive failed on an empty enum. This PR fixes this.

Apparently if you implement a trait on an enum, an empty match statement is not sufficient. Rust seems to think the enum is populated somehow. Adding a unreachable!() fixed this

impl bincode::enc::Encode for BorrowedEmptyEnum {
    fn encode<E: bincode::enc::Encoder>(
        &self,
        mut encoder: E,
    ) -> core::result::Result<(), bincode::error::EncodeError> {
        match self {
            _ => core::unreachable!(), // new
        }
    }
}

Update: Made the Decode not read the byte and return a specific error for this situation

impl bincode::Decode for EmptyEnum {
    fn decode<D: bincode::de::Decoder>(
        mut decoder: D,
    ) -> core::result::Result<Self, bincode::error::DecodeError> {
        core::result::Result::Err(bincode::error::DecodeError::EmptyEnum {
            type_name: core::any::type_name::<Self>(),
        })
    }
}
old

Additionally the self.variants.len() - 1 would underflow if there are no variants. saturating_sub(1) sets it correctly to 0

impl bincode::Decode for EmptyEnum {
    fn decode<D: bincode::de::Decoder>(
        mut decoder: D,
    ) -> core::result::Result<Self, bincode::error::DecodeError> {
        let variant_index = <u32 as bincode::Decode>::decode(&mut decoder)?;
        match variant_index {
            variant => Err(bincode::error::DecodeError::UnexpectedVariant {
                found: variant,
                type_name: "EmptyEnum",
                allowed: bincode::error::AllowedEnumVariants::Range { min: 0, max: 0 },
            }),
        }
    }
}

@VictorKoenders VictorKoenders requested a review from ZoeyR January 8, 2022 09:01
@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2022

Codecov Report

Merging #462 (5b7f6e1) into trunk (db398ec) will decrease coverage by 0.16%.
The diff coverage is 4.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk     #462      +/-   ##
==========================================
- Coverage   66.80%   66.64%   -0.17%     
==========================================
  Files          35       35              
  Lines        2636     2647      +11     
==========================================
+ Hits         1761     1764       +3     
- Misses        875      883       +8     
Impacted Files Coverage Δ
derive/src/derive_enum.rs 0.00% <0.00%> (ø)
src/error.rs 0.00% <ø> (ø)
tests/derive.rs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db398ec...5b7f6e1. Read the comment docs.

@VictorKoenders VictorKoenders merged commit 53acac9 into trunk Jan 8, 2022
@VictorKoenders VictorKoenders deleted the fix/empty_enum branch January 8, 2022 11:39
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants