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

Serde generates strange bounds on generics #938

Closed
kryptan opened this issue May 19, 2017 · 5 comments
Closed

Serde generates strange bounds on generics #938

kryptan opened this issue May 19, 2017 · 5 comments
Labels

Comments

@kryptan
Copy link

kryptan commented May 19, 2017

Given the following code:

#[derive(Serialize)]
struct A<T> {
    b: B<T>,
}

serde generates the following bound: where T: _serde::Serialize while I would expect it to generate where B<T>: _serde::Serialize. Generated bound is wrong in some cases, e.g. if B is implemented as follows:

#[derive(Serialize)]
struct B<T> {
    #[serde(skip_serializing)]
    b: T,
}

I'm currently writing my own custom-derive plugin for other purposes and I'm just putting the same bound on every field type. Serde, instead, uses some complex code to figure out what bounds to use and I'm curious why is it necessary.

@dtolnay
Copy link
Member

dtolnay commented May 19, 2017

We used to have that and it didn't work. IIRC there are two main reasons.

Mutually recursive types

#[derive(Serialize)]
struct A<T> {
    b: Box<B<T>>,
}

// This generates an infinitely complicated bound.
#[derive(Serialize)]
struct B<T> {
    a: Box<A<T>>,
}

Private types

// Not allowed to use private type in public trait bound.
#[derive(Serialize)]
pub struct Public<T> {
    private: Private<T>,
}

#[derive(Serialize)]
struct Private<T> {
    t: T,
}

@dtolnay
Copy link
Member

dtolnay commented May 19, 2017

I would strongly discourage you from generating B<T>: Trait bounds. Serde's approach instead is to let you handwrite the bound in the rare case that it is necessary - see https://serde.rs/attr-bound.html.

@kryptan
Copy link
Author

kryptan commented May 19, 2017

Thanks for explanation!

The recursive case looks like something that can be fixed in the compiler although I'm not an expert.

@kryptan kryptan closed this as completed May 19, 2017
@kryptan
Copy link
Author

kryptan commented May 22, 2017

Recursive where clauses may be fixed in the future. In that case heuristic in serde might be improved by assigning B<T>: Trait bounds to public fields because they must have public type.

@dtolnay
Copy link
Member

dtolnay commented May 22, 2017

@kryptan that would have the unfortunate consequence of making it a potential breaking change to make a field public. That would be extremely surprising behavior.

In the following code if I change the b field to pub, my code continues to compile but users' code breaks because where B<T>: Serialize is never satisfied.

#[derive(Serialize)]
struct A<T> {
    t: T,
    #[serde(serialize_with = "f")]
    b: B<T>,
}

// This type may be from a different crate.
struct B<T>(T);

fn f<T, S>(b: &B<T>, serializer: S) -> Result<S::Ok, S::Error>
    where T: Serialize,
          S: Serializer
{
    unimplemented!()
}

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

No branches or pull requests

2 participants