Skip to content

Conversation

@theweipeng
Copy link
Member

What does this PR do?

Support serialization and deserialization of trait objects

The relationship between a trait object and a concrete type is generated by a macro and stored in a Fury instance. We can access the serializer and deserializer using the Fury type ID and Rust type ID.

Use as follows

    #[impl_polymorph]
    trait Animal {
        fn get_name(&self) -> String;
    }

    #[derive(Fury, Debug)]
    #[polymorphic_traits("Animal")]
    struct Dog {
        name: String
    }

    impl Animal for Dog {
        fn get_name(&self) -> String {
            self.name.clone()
        }
    }

    #[derive(Fury)]
    struct Person {
        pet: Box<dyn Animal>
    }

    let p = Person {
        pet: Box::new(Dog {
            name: String::from("puppy")
        })
    };
    let mut fury = Fury::default();

    fury.register::<Person>(501);
    fury.register::<Dog>(500);
    let bin = fury.serialize(&p);
    let obj: Person = fury.deserialize(&bin).unwrap();
    assert_eq!(obj.pet.get_name(), "puppy");

We should add impl_polymorph for trait which is use for serializing Box<dyn xxxx> and polymorphic_traits for struct which is use for generate code to cast upcast concete type to trait object. Note that rust not support upcast trait yet(rust-lang/rust#65991).

}

#[derive(Fury, Debug)]
#[polymorphic_traits("Animal")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we avoid annotate polymorphic_traits

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try something else, see if I can get rid of this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think of one solution to this issue.

Before this PR, dyn Any is already supported, so solution below should work? Let me know if there is anything I'm missing.

Suppose the trait already contains a fn as_any(&self) -> &dyn Any method, then we can use trait_object.as_any().type_id() to get underlying struct's type id, then we don't need this polymorphic_traits.

as_any solution is a frequently used tricks in rust, so I think adding this requirement is an acceptable choice.

Copy link
Member Author

@theweipeng theweipeng Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think of one solution to this issue.

Before this PR, dyn Any is already supported, so solution below should work? Let me know if there is anything I'm missing.

Suppose the trait already contains a fn as_any(&self) -> &dyn Any method, then we can use trait_object.as_any().type_id() to get underlying struct's type id, then we don't need this polymorphic_traits.

as_any solution is a frequently used tricks in rust, so I think adding this requirement is an acceptable choice.

We already supported dyn Any , but even if we have the type id of the underlying struct, I don't quite understand how to cast to the dyn object through type_id.

My understanding of the solution is as follows

impl Serializer for Box<dyn Foo> {
    fn deserialize() {
          let type_id_in_binary = fury.reader.i16();  // We read the type_id from binary, which was received from another app.
          let concrete_type_deserializer = fury.get_deserializer(type_id_in_binary); // We obtain the deserializer that was registered by the user.
          let concrete_instance: Box<dyn Any> = concrete_type_deserializer();     // Call the function pointer, and we get the instance of the underlying struct. The type of the result should be Any, as the function pointer should be compatible with all the deserializers.
         
        concrete_instance as Box<dyn Foo> // We can't do this, as Rust can only cast from primitive types or concrete types. The compiler can't deduce the type of `concrete_instance` at compile time, so it can't create the v-table. 
    }
}

Let me know if there are some misunderstandings

MaybeTraitObject { ptr, type_id }
}

pub fn to_trait_object<T: 'static>(self) -> Result<T, Error> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If user don't call this function, then T is leaked?

If it's, we can impl Drop for MaybeTraitObject to release memory..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, I will fixed it.

pub fn get_harness_by_type(&self, type_id: TypeId) -> Option<&Harness> {
self.get_harness(*self.type_id_map.get(&type_id).unwrap())
pub fn get_class_info_by_fury_type(&self, type_id: u32) -> &ClassInfo {
let type_id = self.fury_type_id_map.get(&type_id).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too many unwrap here, how about return a Result?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The codebase contain too many unwrap, I will fix it one by one.

@chaokunyang chaokunyang closed this Oct 6, 2025
chaokunyang added a commit that referenced this pull request Oct 6, 2025
…#2704)

## Why?

<!-- Describe the purpose of this PR. -->

## What does this PR do?

suport dyn any trait object serialization for box/arc/rc:
- introduce `ForyDefault` trait to work around orphan rule, so we can
impl `default` for `Rc<dny Any>`
- rename `Fory` derive to `ForyObject` derive
- add serializer impl for `Box/Rc/Arc<any>`
- support shared reference for `Rc/Arc<any>`
- introduce `write_any_typeinfo/read_typeinfo` for dynamic type-meta
read/write

## Related issues

#2691
#1797 
#1795

## Does this PR introduce any user-facing change?

<!--
If any user-facing interface changes, please [open an
issue](https://github.com/apache/fory/issues/new/choose) describing the
need to do so and update the document if necessary.

Delete section if not applicable.
-->

- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?

## Benchmark

<!--
When the PR has an impact on performance (if you don't know whether the
PR will have an impact on performance, you can submit the PR first, and
if it will have impact on performance, the code reviewer will explain
it), be sure to attach a benchmark data here.

Delete section if not applicable.
-->
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 this pull request may close these issues.

3 participants