Make length prefix usable more that once

oknozor · oknozor · commit 22f1afa607cc · 2021-07-20T11:39:32.000+02:00
diff --git a/examples/get_enum_discriminant.rs b/examples/get_enum_discriminant.rs
@@ -18,8 +18,8 @@ pub struct Goodbye {
     reason: String,
 }
 
-#[protocol(discriminant = "integer")]
 #[derive(Protocol, Clone, Debug, PartialEq)]
+#[protocol(discriminant = "integer")]
 #[repr(u16)]
 pub enum PacketKind {
     #[protocol(discriminator(0x00))]
diff --git a/protocol-derive/src/codegen/mod.rs b/protocol-derive/src/codegen/mod.rs
@@ -1,12 +1,13 @@
-pub mod enums;
-
-use crate::attr;
-use proc_macro2::{TokenStream, Ident};
+use proc_macro2::TokenStream;
 use syn;
 use syn::Field;
 
+use crate::attr;
+
+pub mod enums;
+
 pub fn read_fields(fields: &syn::Fields)
-    -> TokenStream {
+                   -> TokenStream {
     match *fields {
         syn::Fields::Named(ref fields_named) => read_named_fields(fields_named),
         syn::Fields::Unnamed(ref fields_unnamed) => read_unnamed_fields(fields_unnamed),
@@ -15,7 +16,7 @@ pub fn read_fields(fields: &syn::Fields)
 }
 
 pub fn write_fields(fields: &syn::Fields)
-    -> TokenStream {
+                    -> TokenStream {
     match *fields {
         syn::Fields::Named(ref fields_named) => write_named_fields(fields_named),
         syn::Fields::Unnamed(ref fields_unnamed) => write_unnamed_fields(fields_unnamed),
@@ -32,7 +33,6 @@ fn read_named_fields(fields_named: &syn::FieldsNamed)
                      -> TokenStream {
     let field_initializers: Vec<_> = fields_named.named.iter().map(|field| {
         let field_name = &field.ident;
-        println!("READING NAMED FIELD {:?}", field_name);
         let field_ty = &field.ty;
         // This field may store the length prefix of one or more other field.
         let update_hints = update_hints_after_read(field, &fields_named.named);
@@ -55,29 +55,30 @@ fn read_named_fields(fields_named: &syn::FieldsNamed)
 fn update_hints_after_read<'a>(field: &'a syn::Field,
                                fields: impl IntoIterator<Item=&'a syn::Field> + Clone)
                                -> TokenStream {
-    if let Some((length_prefix_of, kind, prefix_subfield_names)) = length_prefix_of(field, fields.clone()) {
+    let hint_setters = length_prefix_of(field, fields.clone()).iter().map(|(length_prefix_of, kind, prefix_subfield_names)| {
         let kind = kind.path_expr();
-        let field_name = &field.ident.clone().map(|i| i.to_string());
-        let debug = (field_name, &length_prefix_of, &kind, &prefix_subfield_names);
-        println!("About to set hints for {:?}, of_index : {:?}, subfield names {:?}", field_name, &length_prefix_of, &prefix_subfield_names);
+        quote! {
+            __hints.set_field_length(#length_prefix_of,
+                (parcel #(.#prefix_subfield_names)* ).clone() as usize,
+                #kind);
+        }
+    }).collect::<Vec<TokenStream>>();
+
+    if hint_setters.is_empty() {
+        quote! {}
+    } else {
         quote! {
             if let Ok(parcel) = res.as_ref() {
-                 println!(" setting {:?} fields hint {:?}, {:?}, {:?}",#field_name, #length_prefix_of, (parcel #(.#prefix_subfield_names)* ).clone() as usize, #kind);
-                      __hints.set_field_length(#length_prefix_of,
-                                     (parcel #(.#prefix_subfield_names)* ).clone() as usize,
-                                     #kind);
+                #(#hint_setters);*
             }
         }
-    }else {
-        quote! { }
     }
 }
 
 fn update_hint_fixed_length<'a>(field: &'a syn::Field,
                                 fields: impl IntoIterator<Item=&'a syn::Field> + Clone)
                                 -> TokenStream {
-
-     if let Some(attr::Protocol::FixedLength(length)) = attr::protocol(&field.attrs) {
+    if let Some(attr::Protocol::FixedLength(length)) = attr::protocol(&field.attrs) {
         let position = fields.clone().into_iter().position(|f| f == field).unwrap();
 
         quote! {
@@ -90,22 +91,26 @@ fn update_hint_fixed_length<'a>(field: &'a syn::Field,
 
 fn update_hints_after_write<'a>(field: &'a syn::Field,
                                 fields: impl IntoIterator<Item=&'a syn::Field> + Clone)
-    -> TokenStream {
-    if let Some((length_prefix_of, kind, prefix_subfield_names)) = length_prefix_of(field, fields.clone()) {
+                                -> TokenStream {
+    let hint_setters = length_prefix_of(field, fields.clone()).iter().map(|(length_prefix_of, kind, prefix_subfield_names)| {
         let field_name = &field.ident;
         let kind = kind.path_expr();
-        let debug = (&field.ident.clone().map(|i| i.to_string()), &length_prefix_of, &kind, &prefix_subfield_names);
-        println!("About setting hints for {:?}", debug);
+
         quote! {
-            println!(" WRITE setting {:?} fields hint {:?}, {:?}",self.#field_name #(.#prefix_subfield_names)*, #length_prefix_of, #kind);
-            if let Ok(()) = res {
                 __hints.set_field_length(#length_prefix_of,
                                          (self.#field_name #(.#prefix_subfield_names)* ).clone() as usize,
                                          #kind);
-            }
         }
+    }).collect::<Vec<TokenStream>>();
+
+    if hint_setters.is_empty() {
+        quote! {}
     } else {
-        quote! { }
+        quote! {
+            if let Ok(()) = res {
+                #(#hint_setters);*
+            }
+        }
     }
 }
 
@@ -117,37 +122,33 @@ fn update_hints_after_write<'a>(field: &'a syn::Field,
 /// Returns the field index of the field whose length is specified.
 fn length_prefix_of<'a>(field: &'a syn::Field,
                         fields: impl IntoIterator<Item=&'a syn::Field> + Clone)
-    -> Option<Vec<(usize, attr::LengthPrefixKind, Vec<syn::Ident>)>> {
+                        -> Vec<(usize, attr::LengthPrefixKind, Vec<syn::Ident>)> {
     let potential_prefix = field.ident.as_ref();
 
-    let prefixes_of: Vec<&Ident> = fields.clone().into_iter().filter(|potential_prefix_of| {
+    let prefixes_of: Vec<&'a Field> = fields.clone().into_iter().filter(|potential_prefix_of| {
         match attr::protocol(&potential_prefix_of.attrs) {
             Some(attr::Protocol::LengthPrefix { ref prefix_field_name, .. }) => {
                 if !fields.clone().into_iter().any(|f| f.ident.as_ref() == Some(prefix_field_name)) {
                     panic!("length prefix is invalid: there is no sibling field named '{}", prefix_field_name);
                 }
 
                 potential_prefix == Some(prefix_field_name)
-            },
+            }
             _ => false,
         }
     }).collect();
 
 
     prefixes_of.iter()
         .map(|prefix_of| {
-            if let Some(prefix_of) = prefix_of {
-                let prefix_of_index = fields.clone().into_iter().position(|f| f == prefix_of).unwrap();
-                match attr::protocol(&prefix_of.attrs).unwrap() {
-                    attr::Protocol::LengthPrefix { kind, prefix_subfield_names, .. } => {
-                        Some((prefix_of_index, kind.clone(), prefix_subfield_names))
-                    },
-                    _ => unreachable!(),
+            let prefix_of_index = fields.clone().into_iter().position(|f| &f == prefix_of).unwrap();
+            match attr::protocol(&prefix_of.attrs).unwrap() {
+                attr::Protocol::LengthPrefix { kind, prefix_subfield_names, .. } => {
+                    (prefix_of_index, kind.clone(), prefix_subfield_names)
                 }
-            } else {
-                None
+                _ => unreachable!(),
             }
-        }).flatten().collect()
+        }).collect()
 }
 
 fn write_named_fields(fields_named: &syn::FieldsNamed)
@@ -158,7 +159,6 @@ fn write_named_fields(fields_named: &syn::FieldsNamed)
         let update_hints = update_hints_after_write(field, &fields_named.named);
         let update_hints_fixed = update_hint_fixed_length(field, &fields_named.named);
 
-        println!("WRITE Hints after ");
         quote! {
             {
                 #update_hints_fixed
@@ -174,7 +174,7 @@ fn write_named_fields(fields_named: &syn::FieldsNamed)
 }
 
 fn read_unnamed_fields(fields_unnamed: &syn::FieldsUnnamed)
-    -> TokenStream {
+                       -> TokenStream {
     let field_initializers: Vec<_> = fields_unnamed.unnamed.iter().map(|field| {
         let field_ty = &field.ty;
 
@@ -191,7 +191,7 @@ fn read_unnamed_fields(fields_unnamed: &syn::FieldsUnnamed)
 }
 
 fn write_unnamed_fields(fields_unnamed: &syn::FieldsUnnamed)
-    -> TokenStream {
+                        -> TokenStream {
     let field_indices = (0..fields_unnamed.unnamed.len()).into_iter().map(syn::Index::from);
 
     let field_writers: Vec<_> = field_indices.map(|field_index| {
diff --git a/protocol-derive/src/lib.rs b/protocol-derive/src/lib.rs
@@ -73,9 +73,7 @@ fn build_generics(ast: &syn::DeriveInput) -> (Vec<proc_macro2::TokenStream>, Vec
 fn impl_parcel_for_struct(ast: &syn::DeriveInput,
                           strukt: &syn::DataStruct) -> proc_macro2::TokenStream {
     let strukt_name = &ast.ident;
-    println!("Reading struct fields {:?}", &strukt.fields);
     let read_fields = codegen::read_fields(&strukt.fields);
-    println!("{}", read_fields);
     let write_fields = codegen::write_fields(&strukt.fields);
 
     impl_trait_for(ast, quote!(protocol::Parcel), quote! {
@@ -86,7 +84,6 @@ fn impl_parcel_for_struct(ast: &syn::DeriveInput,
                       __settings: &protocol::Settings,
                       _: &mut protocol::hint::Hints)
             -> Result<Self, protocol::Error> {
-            println!("reading fields with default Hints");
             // Each type gets its own hints.
             let mut __hints = protocol::hint::Hints::default();
             __hints.begin_fields();
diff --git a/protocol/src/hint.rs b/protocol/src/hint.rs
@@ -56,14 +56,12 @@ mod protocol_derive_helpers {
         // Sets hints indicating a new set of fields are beginning.
         #[doc(hidden)]
         pub fn begin_fields(&mut self) {
-            println!("Starting from field 0");
             self.current_field_index = Some(0);
         }
 
         // Updates the hints to indicate a field was just read.
         #[doc(hidden)]
         pub fn next_field(&mut self) {
-            println!("Current field +1");
             *self.current_field_index.as_mut()
                 .expect("cannot increment next field when not in a struct")+= 1;
         }
@@ -74,7 +72,6 @@ mod protocol_derive_helpers {
                                 field_index: FieldIndex,
                                 length: usize,
                                 kind: LengthPrefixKind) {
-            println!("Set field length {:?}", (field_index, length, kind));
             self.known_field_lengths.insert(field_index, FieldLength { kind, length });
         }
     }
diff --git a/protocol/src/util.rs b/protocol/src/util.rs
@@ -75,7 +75,6 @@ pub fn read_list_ext<S,T>(read: &mut dyn Read,
     -> Result<Vec<T>, Error>
     where S: Integer,
           T: Parcel {
-    println!("About to read list ext with prefix and hint {:?}", hints);
     match hints.current_field_length() {
         Some(length) => {
             match length.kind {
diff --git a/tests/src/length_prefix.rs b/tests/src/length_prefix.rs
@@ -103,9 +103,9 @@ fn can_read_twice_the_same_prefix_length() {
             0, 0, 0, 1, // 1
             0, 0, 0, 2, // 2
             0, 0, 0, 3, // 3
-            0, 0, 0, 1, // 4
-            0, 0, 0, 2, // 5
-            0, 0, 0, 3  // 6
+            0, 0, 0, 4, // 4
+            0, 0, 0, 5, // 5
+            0, 0, 0, 6  // 6
         ],
         &Settings::default()).unwrap());
 }

Original file line number	Diff line number	Diff line change
`@@ -18,8 +18,8 @@ pub struct Goodbye {`
`18`	`18`	`reason: String,`
`19`	`19`	`}`
`20`	`20`
`21`		`-#[protocol(discriminant = "integer")]`
`22`	`21`	`#[derive(Protocol, Clone, Debug, PartialEq)]`
	`22`	`+#[protocol(discriminant = "integer")]`
`23`	`23`	`#[repr(u16)]`
`24`	`24`	`pub enum PacketKind {`
`25`	`25`	`#[protocol(discriminator(0x00))]`
Original file line number	Diff line number	Diff line change
`@@ -56,14 +56,12 @@ mod protocol_derive_helpers {`
`56`	`56`	`// Sets hints indicating a new set of fields are beginning.`
`57`	`57`	`#[doc(hidden)]`
`58`	`58`	`pub fn begin_fields(&mut self) {`
`59`		`- println!("Starting from field 0");`
`60`	`59`	`self.current_field_index = Some(0);`
`61`	`60`	`}`
`62`	`61`
`63`	`62`	`// Updates the hints to indicate a field was just read.`
`64`	`63`	`#[doc(hidden)]`
`65`	`64`	`pub fn next_field(&mut self) {`
`66`		`- println!("Current field +1");`
`67`	`65`	`*self.current_field_index.as_mut()`
`68`	`66`	`.expect("cannot increment next field when not in a struct")+= 1;`
`69`	`67`	`}`
`@@ -74,7 +72,6 @@ mod protocol_derive_helpers {`
`74`	`72`	`field_index: FieldIndex,`
`75`	`73`	`length: usize,`
`76`	`74`	`kind: LengthPrefixKind) {`
`77`		`- println!("Set field length {:?}", (field_index, length, kind));`
`78`	`75`	`self.known_field_lengths.insert(field_index, FieldLength { kind, length });`
`79`	`76`	`}`
`80`	`77`	`}`