jamesmunns / postcard

A no_std + serde compatible message library for Rust
Apache License 2.0
929 stars 88 forks source link

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

Open ia0 opened 6 months ago

ia0 commented 6 months ago

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 andpostcard::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 commented 6 months ago

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 commented 6 months ago

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.