From c399e9c36820f0b4985a779b69113f3aa6519f49 Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 8 May 2023 09:25:18 +0500 Subject: [PATCH 1/5] Remove FlatInternallyTaggedAccess because it is the same as FlatMapAccess --- serde/src/private/de.rs | 52 ++++------------------------------------- 1 file changed, 5 insertions(+), 47 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 5e1f7ff7c..756e3d03f 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2738,11 +2738,7 @@ where where V: Visitor<'de>, { - visitor.visit_map(FlatInternallyTaggedAccess { - iter: self.0.iter_mut(), - pending: None, - _marker: PhantomData, - }) + self.deserialize_map(visitor) } fn deserialize_enum( @@ -2878,6 +2874,10 @@ where for item in &mut self.iter { // Items in the vector are nulled out when used by a struct. if let Some((ref key, ref content)) = *item { + // Do not take(), instead borrow this entry. The internally tagged + // enum does its own buffering so we can't tell whether this entry + // is going to be consumed. Borrowing here leaves the entry + // available for later flattened fields. self.pending_content = Some(content); return seed.deserialize(ContentRefDeserializer::new(key)).map(Some); } @@ -2958,45 +2958,3 @@ where } } } - -#[cfg(any(feature = "std", feature = "alloc"))] -pub struct FlatInternallyTaggedAccess<'a, 'de: 'a, E> { - iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, - pending: Option<&'a Content<'de>>, - _marker: PhantomData, -} - -#[cfg(any(feature = "std", feature = "alloc"))] -impl<'a, 'de, E> MapAccess<'de> for FlatInternallyTaggedAccess<'a, 'de, E> -where - E: Error, -{ - type Error = E; - - fn next_key_seed(&mut self, seed: T) -> Result, Self::Error> - where - T: DeserializeSeed<'de>, - { - for item in &mut self.iter { - if let Some((ref key, ref content)) = *item { - // Do not take(), instead borrow this entry. The internally tagged - // enum does its own buffering so we can't tell whether this entry - // is going to be consumed. Borrowing here leaves the entry - // available for later flattened fields. - self.pending = Some(content); - return seed.deserialize(ContentRefDeserializer::new(key)).map(Some); - } - } - Ok(None) - } - - fn next_value_seed(&mut self, seed: T) -> Result - where - T: DeserializeSeed<'de>, - { - match self.pending.take() { - Some(value) => seed.deserialize(ContentRefDeserializer::new(value)), - None => panic!("value is missing"), - } - } -} From a901f50850d22133c01921d74247cd946fd50a7b Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 8 May 2023 09:37:15 +0500 Subject: [PATCH 2/5] FlatMapAccess and FlatStructAccess does not need to be public --- serde/src/private/de.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 756e3d03f..c4c3dc253 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2841,7 +2841,7 @@ where } #[cfg(any(feature = "std", feature = "alloc"))] -pub struct FlatMapAccess<'a, 'de: 'a, E> { +struct FlatMapAccess<'a, 'de: 'a, E> { iter: slice::Iter<'a, Option<(Content<'de>, Content<'de>)>>, pending_content: Option<&'a Content<'de>>, _marker: PhantomData, @@ -2897,7 +2897,7 @@ where } #[cfg(any(feature = "std", feature = "alloc"))] -pub struct FlatStructAccess<'a, 'de: 'a, E> { +struct FlatStructAccess<'a, 'de: 'a, E> { iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, pending_content: Option>, fields: &'static [&'static str], From e11d01fe1d4db936a1058a484f7c854326271fb2 Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 8 May 2023 09:40:06 +0500 Subject: [PATCH 3/5] Remove constructors for FlatMapAccess and FlatStructAccess They are used only in one place each, so for simplifying understanding it is better to inline them --- serde/src/private/de.rs | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index c4c3dc253..131f9f762 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2775,7 +2775,11 @@ where where V: Visitor<'de>, { - visitor.visit_map(FlatMapAccess::new(self.0.iter())) + visitor.visit_map(FlatMapAccess { + iter: self.0.iter(), + pending_content: None, + _marker: PhantomData, + }) } fn deserialize_struct( @@ -2787,7 +2791,12 @@ where where V: Visitor<'de>, { - visitor.visit_map(FlatStructAccess::new(self.0.iter_mut(), fields)) + visitor.visit_map(FlatStructAccess { + iter: self.0.iter_mut(), + pending_content: None, + fields: fields, + _marker: PhantomData, + }) } fn deserialize_newtype_struct(self, _name: &str, visitor: V) -> Result @@ -2847,19 +2856,6 @@ struct FlatMapAccess<'a, 'de: 'a, E> { _marker: PhantomData, } -#[cfg(any(feature = "std", feature = "alloc"))] -impl<'a, 'de, E> FlatMapAccess<'a, 'de, E> { - fn new( - iter: slice::Iter<'a, Option<(Content<'de>, Content<'de>)>>, - ) -> FlatMapAccess<'a, 'de, E> { - FlatMapAccess { - iter: iter, - pending_content: None, - _marker: PhantomData, - } - } -} - #[cfg(any(feature = "std", feature = "alloc"))] impl<'a, 'de, E> MapAccess<'de> for FlatMapAccess<'a, 'de, E> where @@ -2904,21 +2900,6 @@ struct FlatStructAccess<'a, 'de: 'a, E> { _marker: PhantomData, } -#[cfg(any(feature = "std", feature = "alloc"))] -impl<'a, 'de, E> FlatStructAccess<'a, 'de, E> { - fn new( - iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, - fields: &'static [&'static str], - ) -> FlatStructAccess<'a, 'de, E> { - FlatStructAccess { - iter: iter, - pending_content: None, - fields: fields, - _marker: PhantomData, - } - } -} - #[cfg(any(feature = "std", feature = "alloc"))] impl<'a, 'de, E> MapAccess<'de> for FlatStructAccess<'a, 'de, E> where From 1d11f03449be82e9422128b21267338b708b34fc Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 8 May 2023 10:11:05 +0500 Subject: [PATCH 4/5] Extract logic of taking flattened fields into a function --- serde/src/private/de.rs | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 131f9f762..35fb6a0a7 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2751,16 +2751,7 @@ where V: Visitor<'de>, { for item in self.0.iter_mut() { - // items in the vector are nulled out when used. So we can only use - // an item if it's still filled in and if the field is one we care - // about. - let use_item = match *item { - None => false, - Some((ref c, _)) => c.as_str().map_or(false, |x| variants.contains(&x)), - }; - - if use_item { - let (key, value) = item.take().unwrap(); + if let Some((key, value)) = use_item(item, variants) { return visitor.visit_enum(EnumDeserializer::new(key, Some(value))); } } @@ -2912,16 +2903,7 @@ where T: DeserializeSeed<'de>, { while let Some(item) = self.iter.next() { - // items in the vector are nulled out when used. So we can only use - // an item if it's still filled in and if the field is one we care - // about. In case we do not know which fields we want, we take them all. - let use_item = match *item { - None => false, - Some((ref c, _)) => c.as_str().map_or(false, |key| self.fields.contains(&key)), - }; - - if use_item { - let (key, content) = item.take().unwrap(); + if let Some((key, content)) = use_item(item, self.fields) { self.pending_content = Some(content); return seed.deserialize(ContentDeserializer::new(key)).map(Some); } @@ -2939,3 +2921,26 @@ where } } } + +/// Checks if first element of the specified pair matches one of the key from +/// `keys` parameter and if this is true, takes it from the option and returns. +/// Otherwise, or if `item` already empty, returns `None`. +#[cfg(any(feature = "std", feature = "alloc"))] +fn use_item<'de>( + item: &mut Option<(Content<'de>, Content<'de>)>, + keys: &[&str], +) -> Option<(Content<'de>, Content<'de>)> { + // items in the vector are nulled out when used. So we can only use + // an item if it's still filled in and if the field is one we care + // about. + let use_item = match *item { + None => false, + Some((ref c, _)) => c.as_str().map_or(false, |key| keys.contains(&key)), + }; + + if use_item { + item.take() + } else { + None + } +} From ab6588ef7413257d769778720e4fc43bea2fbf76 Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 12 Oct 2020 00:23:45 +0500 Subject: [PATCH 5/5] Extract duplicated code into a function --- serde_derive/src/de.rs | 51 ++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 63e839c43..5358c3b83 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -3084,23 +3084,29 @@ struct DeTypeGenerics<'a>(&'a Parameters); #[cfg(feature = "deserialize_in_place")] struct InPlaceTypeGenerics<'a>(&'a Parameters); +/// If `'de` lifetime is defined, prepends it to list of generics +/// and then produces tokens for declaration generics on type +fn to_tokens(mut generics: syn::Generics, borrowed: &BorrowedLifetimes, tokens: &mut TokenStream) { + if borrowed.de_lifetime_param().is_some() { + let def = syn::LifetimeParam { + attrs: Vec::new(), + lifetime: syn::Lifetime::new("'de", Span::call_site()), + colon_token: None, + bounds: Punctuated::new(), + }; + // Prepend 'de lifetime to list of generics + generics.params = Some(syn::GenericParam::Lifetime(def)) + .into_iter() + .chain(generics.params) + .collect(); + } + let (_, ty_generics, _) = generics.split_for_impl(); + ty_generics.to_tokens(tokens); +} + impl<'a> ToTokens for DeTypeGenerics<'a> { fn to_tokens(&self, tokens: &mut TokenStream) { - let mut generics = self.0.generics.clone(); - if self.0.borrowed.de_lifetime_param().is_some() { - let def = syn::LifetimeParam { - attrs: Vec::new(), - lifetime: syn::Lifetime::new("'de", Span::call_site()), - colon_token: None, - bounds: Punctuated::new(), - }; - generics.params = Some(syn::GenericParam::Lifetime(def)) - .into_iter() - .chain(generics.params) - .collect(); - } - let (_, ty_generics, _) = generics.split_for_impl(); - ty_generics.to_tokens(tokens); + to_tokens(self.0.generics.clone(), &self.0.borrowed, tokens); } } @@ -3113,20 +3119,7 @@ impl<'a> ToTokens for InPlaceTypeGenerics<'a> { .chain(generics.params) .collect(); - if self.0.borrowed.de_lifetime_param().is_some() { - let def = syn::LifetimeParam { - attrs: Vec::new(), - lifetime: syn::Lifetime::new("'de", Span::call_site()), - colon_token: None, - bounds: Punctuated::new(), - }; - generics.params = Some(syn::GenericParam::Lifetime(def)) - .into_iter() - .chain(generics.params) - .collect(); - } - let (_, ty_generics, _) = generics.split_for_impl(); - ty_generics.to_tokens(tokens); + to_tokens(generics, &self.0.borrowed, tokens); } }