-
Notifications
You must be signed in to change notification settings - Fork 358
feat(Rust): Support serialization and deserialization of trait objects #1797
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
Conversation
| } | ||
|
|
||
| #[derive(Fury, Debug)] | ||
| #[polymorphic_traits("Animal")] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 Anymethod, then we can usetrait_object.as_any().type_id()to get underlying struct's type id, then we don't need thispolymorphic_traits.
as_any solution is a frequently used tricks in rust, so I think adding this requirement is an acceptable choice.
There was a problem hiding this comment.
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 Anyis 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 Anymethod, then we can usetrait_object.as_any().type_id()to get underlying struct's type id, then we don't need thispolymorphic_traits.
as_anysolution 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> { |
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
…#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. -->
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 IDandRust type ID.Use as follows
We should add
impl_polymorphfor trait which is use for serializingBox<dyn xxxx>andpolymorphic_traitsfor 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).