Serde: Advice for migrating to 1.0 from 0.9

[lawliet89]

Please let me know if this is not the right place to ask for help. I thought it might be better to write it out as an issue/post rather than ask on IRC because the question might be a bit long.

I am currently trying to port my library from 0.9 to 1.0 in lawliet89/biscuit#64 and would like some advice to see if I am doing things correctly.

'de lifetime for structs

Consider the Compact enum I have linked. It contains some generic H that should be (de)serializable. I would like Compact not to have a 'de lifetime and not care whether H contains references with 'de lifetimes or not. Am I using HRTB correctly, or is HRTB not the way to do this?

At the same time, I noticed that I cannot use 'de in my HRTB because the codegen uses it when it derives.

I also noticed that I had to add #[serde(bound(deserialize = ""))] for the header field because the codegen will add H: _serde::Deserialize<'de> otherwise.

'de lifetime for functions

For functions like this where I don't care about the lifetime as well, should I also use HRTB or just stick with having 'de in the generic type parameters?

Thanks in advance for any assistance.

[dtolnay]

For data structures

I would get rid of the bound entirely. All the bounds.

#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum Compact<T, H> { // <- no bounds needed
     #[serde(skip_serializing)]
     #[serde(skip_deserializing)]
     Decrypted {
         header: Header<H>,
         payload: T,
     },
     Encrypted(::Compact),
 }

In general, trait bounds should go on behavior and not on data structures.

impl<T> Lawliet89<T> where T: MyTrait { /* behavior, ok */ }

fn lawliet89<T>() where T: MyTrait { /* behavior, also ok */ }

struct Lawliet89<T> where T: MyTrait { /* data, in general try not to do this */ }

The one exception is if a trait bound is necessary on the data structure because it refers to associated types.

enum Cow<'a, B> where B: 'a + ToOwned + ?Sized {
    Borrowed(&'a B),
    Owned(B::Owned), // <- needs the associated type of ToOwned
}

For functions

You should use T: serde::de::DeserializeOwned which means the same thing but is more intuitive to read. It means T is a type that can deserialize without borrowing any data. This trait is automatically implemented for types that satisfy the HRTB.

[lawliet89]

Thanks.

For impl, would impl<'de, T> Compact<T> where T: Deserialize<'de> or impl<T> Compact<T> where T: for<'de> Deserialize<'de> work better?

Or just stick with T: serde::de::DeserializeOwned. But I would like the flexibility for callers to call functions whether their Deserializable borrows or not.

[dtolnay]

That depends what the code does! I haven't tried to look, but if the code wants to deserialize T from some short-lived data then T needs to be owned:

impl<T> Compact<T> where T: DeserializeOwned {
    fn somehow_get_a_compact() -> Result<Compact<T>> {
        let mut bytes = Vec::new();
        for _ in 0..1024 {
            bytes.push(random_u8());
        }
        serde_whatever::from_bytes(&bytes)
        // `bytes` goes out of scope here so T can't borrow it
    }
}

Otherwise:

impl<'de, T> Compact<T> where T: Deserialize<'de> {
    fn here_take_these_bytes(bytes: &'de [u8]) -> Result<Compact<T>> {
        serde_whatever::from_bytes(bytes)
    }
}

[lawliet89]

Thanks. I'll post back with a link to my changes and close the issue when I've implemented your suggestions for future readers.

[lawliet89]

See the referenced PR above for the changes I've made.

Thank you @dtolnay

[dtolnay]

Nicely done!