bincode-org / bincode

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

Optional feature/configuration option to respect enum discriminants #632

Closed amsam0 closed 1 year ago

amsam0 commented 1 year ago

If you have this enum:

#[derive(bincode::Encode, bincode::Decode)]
enum TestEnum {
    Variant1 = 2,
    Variant2 = 5,
}

bincode will encode Variant1 as 0.

For my use case, I need it to be encoded as 2. Would it be possible for an optional feature/configuration option to have this functionality? I can implement it. (#[repr] would also need to be respected, but I don't think it would be too hard)

A workaround is to do this:

#[derive(bincode::Encode, bincode::Decode)]
enum TestEnum {
    __DoNotUse1,
    __DoNotUse2,
    Variant1,
    __DoNotUse3,
    __DoNotUse4,
    Variant2,
}

but it's very messy.

Implementing Encode and Decode by hand (after copying the generated implementations) is also an option, but it would be pain when updating the enum. If you have multiple enums, it also creates a lot of unnecessary code.

I made an extremely flawed (but works for my use case) implementation here: https://github.com/naturecodevoid/bincode/commit/4d55f4b97c0966eb1f2aa6757c477049b6b014a5 (it doesn't respect #[repr] and won't error if a variant is a negative (because u32). instead, it will produce a weird compile time error. other than that, it works, aside from breaking a couple of the tests)

VictorKoenders commented 1 year ago

What is the use case where you care which value bincode encodes/decodes the value at? It shouldn't matter to the developer how the data is stored

amsam0 commented 1 year ago

I am communicating with a proprietary unix socket. When I send/receive a message, there is a 12 byte header. If bincode doesn't respect enum discriminants, it will produce an incorrect header and the daemon will either not respond to the message or respond with an error code.

amsam0 commented 1 year ago

It should be possible to easily respect repr using a similar method to serde-repr:

impl<'de> serde::Deserialize<'de> for UsbmuxdResult {
    #[allow(clippy::use_self)]
    fn deserialize<D>(deserializer: D) -> core::result::Result<Self, D::Error>
    where
        D: serde::Deserializer<'de>,
    {
        struct discriminant;
        #[allow(non_upper_case_globals)]
        impl discriminant {
            const Ok: u32 = UsbmuxdResult::Ok as u32;
            ...
        }
        match <u32 as serde::Deserialize>::deserialize(deserializer)? {
            discriminant::Ok => core::result::Result::Ok(UsbmuxdResult::Ok),
            ...
            other => {
                core::result::Result::Err(
                    serde::de::Error::custom(
                        format_args!(
                            "invalid value: {0}, expected one of: {1}, {2}, {3}, {4}, {5}, {6}",
                            other, discriminant::Ok, ...
                        ),
                    ),
                )
            }
        }
    }
}

although this would require the enum to be represented by a u32.

we could make a new crate, similar to serde-repr for a derive macro that has the functionality I outlined earlier.

VictorKoenders commented 1 year ago

I am communicating with a proprietary unix socket. Sorry bincode only supports its own format, not other formats. If you want to use other formats you're going to have to implement custom encode/decode variants.

This is discussed at length in #565 and #566, and there is a comment about this in the readme

amsam0 commented 1 year ago

Ok, in that case I will most likely create bincode-repr crate, like serde has. I'm a bit surprised you/bincode maintainers haven't made this yet, since it seems to be a pretty common thing people want, but I understand that you only support the bincode protocol. I saw the comment in the README and I was hoping my use case would be good enough.