urlyy commented on code in PR #2699:
URL: https://github.com/apache/fory/pull/2699#discussion_r2403712893
##########
rust/fory-derive/src/object/read.rs:
##########
@@ -136,7 +136,7 @@ pub fn gen_read(struct_ident: &Ident) -> TokenStream {
<Self as
fory_core::serializer::Serializer>::fory_read_data(context, false)
},
fory_core::types::Mode::Compatible => {
- <#struct_ident as
fory_core::serializer::StructSerializer>::fory_read_compatible(context)
+ <#struct_ident as
fory_core::serializer::Serializer>::fory_read_compatible(context)
Review Comment:
Although it may look a bit semantically confusing… In order to split up the
code and improve readability, I moved the deserialization logic of
`compatible_struct` into `read_compatible()`, which is on the same level as
`read()`. However, since the code for `read_compatible()` is generated at
compile time, the variable names don’t reveal whether the type is a struct or
an EXT or an enum, so in such cases `read_compatible()` is always used.
The problem is that EXT types only implement the `Serializer` trait. With
the previous implementation, they didn’t have a `read_compatible()` method,
which would result in a “method not found” error. To fix this, I moved
`read_compatible()` from the `StructSerializer` trait into the `Serializer`
trait, and provided a default implementation whose logic applies only to
`EXT/NAMED_EXT`. The only possible callers of `read_compatible()` are
`struct/enum/ext`. And struct/enum will override the default implementation.
##########
rust/fory-derive/src/object/read.rs:
##########
@@ -136,7 +136,7 @@ pub fn gen_read(struct_ident: &Ident) -> TokenStream {
<Self as
fory_core::serializer::Serializer>::fory_read_data(context, false)
},
fory_core::types::Mode::Compatible => {
- <#struct_ident as
fory_core::serializer::StructSerializer>::fory_read_compatible(context)
+ <#struct_ident as
fory_core::serializer::Serializer>::fory_read_compatible(context)
Review Comment:
It may look a bit semantically confusing… In order to split up the code and
improve readability, I moved the deserialization logic of `compatible_struct`
into `read_compatible()`, which is on the same level as `read()`. However,
since the code for `read_compatible()` is generated at compile time, the
variable names don’t reveal whether the type is a struct or an EXT or an enum,
so in such cases `read_compatible()` is always used.
The problem is that EXT types only implement the `Serializer` trait. With
the previous implementation, they didn’t have a `read_compatible()` method,
which would result in a “method not found” error. To fix this, I moved
`read_compatible()` from the `StructSerializer` trait into the `Serializer`
trait, and provided a default implementation whose logic applies only to
`EXT/NAMED_EXT`. The only possible callers of `read_compatible()` are
`struct/enum/ext`. And struct/enum will override the default implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
