bincode-org / bincode

A binary encoder / decoder implementation in Rust.
MIT License
2.63k stars 265 forks source link

Discriminant size in `fixint` serialization of `Option<T>` #700

Closed mkeeter closed 5 months ago

mkeeter commented 5 months ago

I've run into an interesting corner case in the bincode specification:

fn main() {
    let i: Option<u32> = Some(100);
    let out = bincode::serialize(&i).unwrap();
    println!("{out:?}");

    let i: Option<u32> = None;
    let out = bincode::serialize(&i).unwrap();
    println!("{out:?}");

    let i: Result<u32, u32> = Ok(101);
    let out = bincode::serialize(&i).unwrap();
    println!("{out:?}");

    let i: Result<u32, u32> = Err(102);
    let out = bincode::serialize(&i).unwrap();
    println!("{out:?}");
}
$ cargo run -q
[1, 100, 0, 0, 0]
[0]
[0, 0, 0, 0, 101, 0, 0, 0]
[1, 0, 0, 0, 102, 0, 0, 0]

Note that the Result<T, E> uses a 4-byte tag, as we'd expect from the specification.

I don't think the behavior should change at this point, but it would be good to reflect this corner case in the specification. If that makes sense, I'd be happy to submit a PR to that effect.

VictorKoenders commented 5 months ago

Option<T> is indeed encoded as 0u8 or 1u8 <data>, the spec is wrong

mkeeter commented 5 months ago

Thanks for the confirmation! Do you want me to open a PR to fix the spec?

VictorKoenders commented 5 months ago

Yes please