dtolnay / typetag

Serde serializable and deserializable trait objects
Apache License 2.0
1.15k stars 36 forks source link

Deserialization of generic impls is not supported yet #1

Open maplant opened 5 years ago

maplant commented 5 years ago

I have a pretty simple example of a generic impl that I am very excited to use typetag to deserialize eventually, so thus I would like to open this issue as a friendly reminder of the limitation :-) Here is my example:

#[typetag::serde(tag = "collider")]
pub trait SerializableCollider {
    fn as_collider(&self) -> &dyn ComponentCollider;
}

#[typetag::serde]
impl<'a, T: 'static + ComponentCollider + Serialize + Deserialize<'a>> SerializableCollider for T {
    fn as_collider(&self) -> &dyn ComponentCollider {
        self
    }
}
dtolnay commented 5 years ago

What would the tags be?

{"collider":"???"}

Ideally it would be the name of the concrete type of T. I think this will require an additional trait bound on T that gives the generated code the ability to identify a tag for T.

maplant commented 5 years ago

You could add a trait bound Any, and then have the tag be the TypeId of T

edulix commented 5 years ago

What about the deserialization of traits with lifetimes? I'm getting this:

error: deserialization of generic impls is not supported yet; use #[typetag::serialize] to generate serialization only
   --> src/step/form.rs:134:5
    |
134 | impl<'w> Step<'w> for Form {

AFAIK, that is not a generic impl right?

maplant commented 5 years ago

@edulix how would you deserialize anything that has a lifetime?

jiangliu commented 5 years ago

Any progress on this feature requirement? We run into the same situation to deserialize a struct with generic types.

zombiepigdragon commented 3 years ago

It's been over a year since the last comment, so I'd like to ask if there's been any progress on this that happened not to appear on the issue. If not, exactly what is needed to add support for this?

dtolnay commented 3 years ago

@zombiepigdragon, someone just needs to send a PR.

Diggsey commented 2 years ago

@maplant TypeId is not consistent across compiles, so would be dangerous to use as the tag here.

Also, any solution needs to explicitly list the instantiations which can be deserialized, since otherwise the compiler won't know to generate code for them.

I think a reasonable approach would be for the #[typetag::serde] attribute to generate a macro definition when applied to a generic impl. It would then be used like this:

#[typetag::serde]
impl<T> Foo for Bar<T> { ... }

bar::instantiate! {
    <i32>,
    <f32>,
};

(And the macro can be exported to allow additional instantiations to be defined by downstream crates)

soufianexmx commented 1 year ago

JFI encountered the same limitation

baiguoname commented 1 year ago

Any update on this?

Pscheidl commented 2 months ago

I was trying to figure out the solution to this. For traits with generic parameters (not generic impls), creating an inventory of all implementations would be the challenge.

#[typetag::serde(tag = "type")]
trait Bar{}

#[typetag::serde(tag = "type")]
trait Foo<R> where R: Bar{
    fn foo(&self) -> R;
}

would require a single registry (inventory) for all generic variants. AFAIK inventory can't do that. Perhaps std::any::Any could be useful here: impl ::inventory::Collect for TypetagRegistration<TypetagFn<&dyn Any>>. The result could look something like this:

Typetag example ```rust type TypetagStrictest = as typetag::__private::Strictest>::Object; type TypetagFn = typetag::__private::DeserializeFn>; struct TypetagRegistration { name: &'static str, deserializer: T, } impl ::inventory::Collect for TypetagRegistration> { #[inline] fn registry() -> &'static ::inventory::Registry { static REGISTRY: ::inventory::Registry = ::inventory::Registry::new(); ®ISTRY } } impl dyn Foo { #[doc(hidden)] const fn typetag_register( name: &'static str, deserializer: T, ) -> TypetagRegistration { TypetagRegistration { name, deserializer, } } } ```

Even with Any used, one can't submit an instance as &'static dyn Foo<dyn Any> to inventory. Works only without generics.

Inventory example ```rust use std::{any::Any, fmt::{Debug, Display}}; trait Foo: Sync + Send + 'static{ fn foo(&self) -> R; } struct English; inventory::collect!(&'static dyn Foo); impl Foo for English { fn foo(&self) -> String { "Hello".to_string() } } inventory::submit! { &English{} as &'static dyn Foo } ```

And that looks like a dead end to me.

Dampfwalze commented 2 months ago

I have a use case similar to @Pscheidl. I have a trait MyTrait that cannot be made into a trait object, so I have a wrapper trait DynMyTrait that wraps it and can be made into a trait object. This trait however is automatically implemented using a generic parameter T: MyTrait:

struct MyType( ... );

trait MyTrait: 'static {
    type MyType: Into<MyType> + From<MyType>; 

    fn some_method(&self, f: impl Fn());
}

trait DynMyTrait: 'static {
    fn some_method(&self, f: Box<dyn Fn()>);
}

impl<T: MyTrait> DynMyTrait for T {
    fn some_method(&self, f: Box<dyn Fn()>) {
        self.some_method(f);
    }
}

struct MyImpl;

impl MyTrait for MyImpl {
    type MyType = ...;

    fn some_method(&self, f: impl Fn()) {
        f();
    }
}

The way I solved this is that I annotated the DynMyTrait with #[typetag::serde(tag = "type")] and, since #[typetag::serde] does not work on impl<T: MyTrait> DynMyTrait for T, I implemented both required methods myself. I needed to additionally add the bound MyTrait: erased_serde::Serialize and derive serde::Serialize and serde::Deserialize on all concrete types. With that, serialization worked already. To make deserialization work too, I had to add them to the crate internal inventory. This is where I had to resort to some extreme measures...

struct MyType( ... );

trait MyTrait: 'static + erased_serde::Serialize {
    type MyType: Into<MyType> + From<MyType>; 

    fn some_method(&self, f: impl Fn());

    fn typetag_name() -> &'static str;
}

#[typetag::serde(tag = "type")]
trait DynMyTrait: 'static {
    fn some_method(&self, f: Box<dyn Fn()>);
}

impl<T: MyTrait> DynMyTrait for T {
    fn some_method(&self, f: Box<dyn Fn()>) {
        self.some_method(f);
    }

    #[doc(hidden)]
    fn typetag_name(&self) -> &'static str {
        Self::typetag_name()
    }

    #[doc(hidden)]
    fn typetag_deserialize(&self) {}
}

#[derive(serde::Serialize, serde::Deserialize)]
struct MyImpl;

deserialize_object!(MyImpl, "my_impl");

impl MyTrait for MyImpl {
    type MyType = ...;

    fn some_method(&self, f: impl Fn()) {
        f();
    }

    fn typetag_name() -> &'static str {
        "my_impl"
    }
}

The deserialize_object! macro looks like this:

macro_rules! deserialize_object {
    ($ty:ty, $slug:expr) => {
        typetag::__private::inventory::submit! {
            <dyn DynMyTrait>::typetag_register(
                $slug,
                (|deserializer| typetag::__private::Result::Ok(
                    typetag::__private::Box::new(
                        typetag::__private::erased_serde::deserialize::<$ty>(deserializer)?
                    ),
                )) as typetag::__private::DeserializeFn<<dyn DynMyTrait as typetag::__private::Strictest>::Object>
            )
        }
    };
}

I pretty much recreated what the crate does at:

https://github.com/dtolnay/typetag/blob/dc3f6d4ec6672945a024c105c91958c1e8fcacd5/impl/src/tagged_impl.rs#L33-L46

The difference is just that it uses the DynMyTrait instead of MyTrait, where you would annotade the MyTrait impl.

I think this specific use case can be very well be implemented in this crate. You just need to somehow specify that the inventory should use the DynMyTrait instead of MyTrait.

contagon commented 2 months ago

@Dampfwalze Thanks for posting this workaround! This was almost the exact predicament I had as well, nice to know I can keep my blanket implementation. I think this boils down to more-or-less what @Diggsey had proposed as well, just a little more hands on.

One thing I noticed while using it in my project, and correct me if I'm wrong, I believe the method fn typetag_name() -> &'static str; belongs in MyTrait and not in DynMyTrait. I got errors about DynMyTrait not being object safe with that method.

Dampfwalze commented 2 months ago

@contagon Yes you are absolutely right. I made this example up from my experience in a bigger project and I didn't put in the time to properly test it... Sorry for this little inconvenience.

I have fixed this in my original post.

Pscheidl commented 1 month ago

The point of my example wasn't object safety (can be achieved), it's the generic output of the foo fn, bound by the Bar trait. This crate currently uses inventory and inventory by design doesn't support storing such types. I wonder if linkme's would solve this problem ?

#[typetag::serde(tag = "type")]
trait Bar{}

#[typetag::serde(tag = "type")]
trait Foo<R> where R: Bar{
    fn foo(&self) -> R;
}