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

Support 128bit integers #236

Closed
KodrAus opened this issue May 29, 2018 · 5 comments
Closed

Support 128bit integers #236

KodrAus opened this issue May 29, 2018 · 5 comments

Comments

@KodrAus
Copy link
Contributor

KodrAus commented May 29, 2018

serde 1.0.62 1.0.60 was recently released with optional support for 128bit numbers. It would be good if we could wire them up to bincode. There's a convenient serde_if_integer128! macro that serde exposes we could use to automatically detect newer compilers, but that might not line up so well with byteorder's explicit i128 feature. Maybe we could do something like this:

In Cargo.toml:

[features]
i128 = ["byteorder/i128"]

In ser.rs or de.rs:

impl Serializer {
    serde_if_integer128! {
        #[cfg(feature = "i128")]
        fn serialize_i128(self, v: i128) -> Result<Self::Ok, Self::Error> {
            // use `byteorder` to serialize our value
        }

        #[cfg(not(feature = "i128"))]
        fn serialize_i128(self, v: i128) -> Result<Self::Ok, Self::Error> {
            let _ = v;
            Err(Error::custom("i128 is not supported. Enable the `i128` feature of `bincode`"))
        }

        ...
    }
}

so that vanilla bincode keeps working on older compilers, but has a more helpful error message if you accidentally use 128bit numbers without enabling the feature?

Or maybe we could work around it using some build.rs wizardry?

@KodrAus
Copy link
Contributor Author

KodrAus commented May 29, 2018

Hmm actually, if we wanted to do something with an explicit i128 feature on bincode maybe it would be better if it looked like:

impl Serializer {
    #[cfg(feature = "i128")]
    fn serialize_i128(self, v: i128) -> Result<Self::Ok, Self::Error> {
        // use `byteorder` to serialize our value
    }

    serde_if_integer128! {
        #[cfg(not(feature = "i128"))]
        fn serialize_i128(self, v: i128) -> Result<Self::Ok, Self::Error> {
            let _ = v;
            Err(Error::custom("i128 is not supported. Enable the `i128` feature of `bincode`"))
        }

        ...
    }
}

So that enabling it always tried to implement the method, but if it's available and not implemented then we suggest the feature.

@TyOverby
Copy link
Collaborator

This looks like a good plan!

Would you feel comfortable implementing it? If not, I can get to it later this week.

@KodrAus
Copy link
Contributor Author

KodrAus commented May 29, 2018

@TyOverby Sure! I'm happy to submit a PR for this.

@KodrAus
Copy link
Contributor Author

KodrAus commented Jun 17, 2018

Hi @TyOverby! Are you happy to push out a 1.0.1 patch with the 128bit number support?

@TyOverby
Copy link
Collaborator

Published!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants