chaokunyang commented on code in PR #3092:
URL: https://github.com/apache/fory/pull/3092#discussion_r2647140974
##########
rust/fory-derive/src/object/read.rs:
##########
@@ -268,48 +291,59 @@ pub fn gen_read_type_info() -> TokenStream {
}
}
-fn get_fields_loop_ts(fields: &[&Field]) -> TokenStream {
- let read_fields_ts: Vec<_> = fields
+fn get_sorted_fields_loop_ts(sorted_fields: &[SortedField<'_>]) -> TokenStream
{
+ let read_fields_ts: Vec<_> = sorted_fields
.iter()
- .enumerate()
- .map(|(idx, field)| {
- let private_ident = create_private_field_name(field, idx);
- let field_name = super::util::get_field_name(field, idx);
- gen_read_field(field, &private_ident, &field_name)
+ .map(|sf| {
+ let private_ident = create_private_field_name(sf.field,
sf.original_index);
+ let field_name = super::util::get_field_name(sf.field,
sf.original_index);
+ gen_read_field(sf.field, &private_ident, &field_name)
})
.collect();
quote! {
#(#read_fields_ts)*
}
}
-pub fn gen_read_data(fields: &[&Field]) -> TokenStream {
- let version_hash = compute_struct_version_hash(fields);
- let sorted_read = if fields.is_empty() {
+pub fn gen_read_data(sorted_fields: &[SortedField<'_>]) -> TokenStream {
+ let fields: Vec<&Field> = sorted_fields.iter().map(|sf|
sf.field).collect();
+ let version_hash = compute_struct_version_hash(&fields);
+ let sorted_read = if sorted_fields.is_empty() {
quote! {}
} else {
- let loop_ts = get_fields_loop_ts(fields);
+ let loop_ts = get_sorted_fields_loop_ts(sorted_fields);
quote! {
#loop_ts
}
};
- let is_tuple = super::util::is_tuple_struct(fields);
- let field_idents: Vec<_> = fields
- .iter()
- .enumerate()
- .map(|(idx, field)| {
- let private_ident = create_private_field_name(field, idx);
- if is_tuple {
- // For tuple structs, just use the variable
- quote! { #private_ident }
- } else {
- // For named structs, include the field name
- let original_ident = &field.ident;
+ let is_tuple = super::util::is_tuple_struct(&fields);
+
+ // For tuple structs, we need to order the field values by their original
index
+ // to construct Self(field0, field1, field2, ...) correctly.
+ let field_idents: Vec<_> = if is_tuple {
Review Comment:
Could we unify thoese two branchs into one by extends
`FieldInfo/SortedField`.
It's ok to introduce some kind of abstraction/escaption in
`FieldInfo/SortedField`. So we can make the logic more central instead of
scattered in many places
--
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]
