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

derive(Schema) introduces incorrect bounds instead of doing perfect derive #153

Open
ia0 opened this issue May 13, 2024 · 2 comments · May be fixed by #154
Open

derive(Schema) introduces incorrect bounds instead of doing perfect derive #153

ia0 opened this issue May 13, 2024 · 2 comments · May be fixed by #154

Comments

@ia0
Copy link
Contributor

ia0 commented May 13, 2024

This is a known issue in Rust that derive(Clone), derive(PartialEq), etc., don't infer the proper bounds. They will require each type parameter to implement the trait instead of looking at what is actually needed by the type definition (called perfect derive).

For some reason, serde derive macros do perfect derive. It would be nice if Schema derive macro would also do so. Here is a small reproduction example:

# Cargo.toml
[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[dependencies]
postcard = { version = "1.0.8", features = ["experimental-derive"] }
serde = { version = "1.0.201", features = ["derive"] }

[patch.crates-io.postcard]
git = "https://github.com/jamesmunns/postcard.git"
rev = "4e1104d9e2e1ef6f9b2473cf5da7d9d58fd8f6fd"
// src/lib.rs
use postcard::experimental::schema::Schema;
use serde::{Deserialize, Serialize};

pub trait View<'a> {
    type Type<T: Wire<'a>>: Wire<'a>;
}

#[derive(Serialize, Deserialize, Schema)]
pub enum Api<'a, T: View<'a>> {
    Foo(T::Type<Foo<'a>>),
}

#[derive(Serialize, Deserialize, Schema)]
pub struct Foo<'a> {
    buf: &'a [u8],
    ret: u32,
}

pub enum Id {}
impl<'a> View<'a> for Id {
    type Type<T: Wire<'a>> = T;
}

pub trait Wire<'a>: Serialize + Deserialize<'a> + Schema {}
impl<'a, T: Serialize + Deserialize<'a> + Schema> Wire<'a> for T {}

#[test]
fn api_schema() {
    let schema = Api::<Id>::SCHEMA;
}

Running cargo test gives the following error:

error[E0599]: the variant or associated item `SCHEMA` exists for enum `Api<'_, Id>`, but its trait bounds were not satisfied
   --> src/lib.rs:29:29
    |
9   | pub enum Api<'a, T: View<'a>> {
    | ----------------------------- variant or associated item `SCHEMA` not found for this enum because it doesn't satisfy `Api<'_, Id>: Schema`
...
19  | pub enum Id {}
    | ----------- doesn't satisfy `Id: Schema`
...
29  |     let schema = Api::<Id>::SCHEMA;
    |                             ^^^^^^ variant or associated item cannot be called on `Api<'_, Id>` due to unsatisfied trait bounds
    |
    = note: trait bound `Id: Schema` was not satisfied
    = note: the following trait bounds were not satisfied:
            `Api<'_, Id>: Schema`
            which is required by `&Api<'_, Id>: Schema`

In particular, it thinks that Id must implement Schema, while only Id::Type<Foo<'a>> (i.e. Foo<'a>) needs to.

When looking at the cargo expand output, we can see the difference between serde::Serialize and postcard::Schema:

    #[automatically_derived]
    impl<'a, T: View<'a>> _serde::Serialize for Api<'a, T>
    where
        T::Type<Foo<'a>>: _serde::Serialize, // This is the correct bound.
    { ... }
// versus
impl<
    'a,
    T: View<'a> + ::postcard::experimental::schema::Schema, // This is an incorrect bound.
> ::postcard::experimental::schema::Schema for Api<'a, T> { ... }

This issue is blocking #143.

@jamesmunns
Copy link
Owner

I am very open to getting this fixed, but my proc-macro skills are fairly limited. If you can find a way to do it, I'm all ears.

@ia0
Copy link
Contributor Author

ia0 commented May 14, 2024

The logic to infer perfect bounds is quite complex, but serde also provides a way to overwrite the bounds using #[serde(bound = "...")]. This is more general (it always works) but needs the user to write the bounds themselves. In my case this is simple because there are no bounds, i.e. I just need #[postcard(bound = "")] (similar to the added test in #154). Because this is simpler to implement and more general, I went this way. User convenience could be added later by guessing the perfect bounds like in serde if there is demand.

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

Successfully merging a pull request may close this issue.

2 participants