Serde: Deserialize lifetime in generic context

[jimmycuadra]

I'm working on upgrading a library to Serde 1.0 and came to a situation I'm not sure how best to solve. There is a type that previously looked like this:

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct StrippedStateContent<C> where C: Deserialize + Serialize {
    pub content: C,
}

Now that Deserialize has a lifetime, it's unclear how to declare the lifetime parameter. The lifetimes chapter in the docs suggests _not_ to include the 'de lifetime as part of the target type, but it only shows an example of how to avoid it when implementing Deserialize for a type directly, rather than a generic type within another, like in my example above.

In my case, there is no borrowed data in C, so it seems wrong to introduce a lifetime parameter into the signature of StrippedStateContent. Just to see how it would go, I tried this:

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct StrippedStateContent<'de, C> where C: Deserialize<'de> + Serialize {
    pub content: C,
}

But that results in three copies of this error in the compiler output:

error[E0263]: lifetime name `'de` declared twice in the same scope
   |
67 | #[derive(Clone, Debug, Deserialize, Serialize)]
   |                        ^^^^^^^^^^^
   |                        |
   |                        previous declaration here
   |                        declared twice

Renaming the lifetime variable (e.g. to 'a) gets around this, but of course now StrippedStateContent must always be written with a lifetime parameter even though it doesn't borrow anything.

What's the intended approach here? Thanks!

[lawliet89]

I have the same question as you. See how I got around it in #891, although I'm not sure if that's the right way.

To add on, the three copies of the error are due to the automatically derived code. If you use cargo expand you can see where they come from.

[jimmycuadra]

Thanks, @lawliet89! I tried using an HRTB, but ran into:

error[E0283]: type annotations required: cannot resolve `C: serde::Deserialize<'de>`
   |
67 | #[derive(Clone, Debug, Deserialize, Serialize)]
   |                        ^^^^^^^^^^^
   |
   = note: required by `serde::Deserialize`

Adding #[serde(bound(deserialize = ""))] fixed that, but that feels way too magical/hacky for my taste, so I'm hoping there's another approach. Or perhaps this is a case that needs to be addressed in the next Serde release. It's certainly not ergonomic, or even clear, as it stands.

[lawliet89]

I agree. Adding the bound fixed it because the code gen automatically adds the bound mentioned in the error message for you. Only reason I realised I had to use the attribute was from digging around the output from cargo expand.

[dtolnay]

Just get rid of the bound?

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct StrippedStateContent<C> {
    pub content: C,
}

[dtolnay]

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

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

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

struct Jimmy<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
}

[dtolnay]

Thanks for raising this issue!

1. I added a section called Trait bounds to the website explaining the two main ways to write Deserialize trait bounds.

2. I opened a Clippy issue https://github.com/Manishearth/rust-clippy/issues/1689 to warn about trait bounds on structs like StrippedStateContent. (Trait bounds in general, not just Serde traits. This is never what you want.)

3. I opened #893 to follow up on the "lifetime declared twice" error.