From 5c18bfeda6f192661e6c7b0207c3617135f3b57e Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 1 May 2023 01:00:40 +0500 Subject: [PATCH 1/8] Inline `deserialize_struct_as_struct_in_place_visitor` --- serde_derive/src/de.rs | 63 ++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 1434680b..904fd5fa 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1158,10 +1158,31 @@ fn deserialize_struct_in_place( }; let expecting = cattrs.expecting().unwrap_or(&expecting); - let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs, expecting)); + let field_names_idents: Vec<_> = fields + .iter() + .enumerate() + .filter(|&(_, field)| !field.attrs.skip_deserializing()) + .map(|(i, field)| { + ( + field.attrs.name().deserialize_name(), + field_i(i), + field.attrs.aliases(), + ) + }) + .collect(); - let (field_visitor, fields_stmt, visit_map) = - deserialize_struct_as_struct_in_place_visitor(params, fields, cattrs); + let fields_stmt = { + let field_names = field_names_idents.iter().map(|(name, _, _)| name); + quote_block! { + #[doc(hidden)] + const FIELDS: &'static [&'static str] = &[ #(#field_names),* ]; + } + }; + + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, None); + + let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs, expecting)); + let visit_map = deserialize_map_in_place(params, fields, cattrs); let field_visitor = Stmts(field_visitor); let fields_stmt = Stmts(fields_stmt); @@ -2707,42 +2728,6 @@ fn deserialize_map( } } -#[cfg(feature = "deserialize_in_place")] -fn deserialize_struct_as_struct_in_place_visitor( - params: &Parameters, - fields: &[Field], - cattrs: &attr::Container, -) -> (Fragment, Fragment, Fragment) { - assert!(!cattrs.has_flatten()); - - let field_names_idents: Vec<_> = fields - .iter() - .enumerate() - .filter(|&(_, field)| !field.attrs.skip_deserializing()) - .map(|(i, field)| { - ( - field.attrs.name().deserialize_name(), - field_i(i), - field.attrs.aliases(), - ) - }) - .collect(); - - let fields_stmt = { - let field_names = field_names_idents.iter().map(|(name, _, _)| name); - quote_block! { - #[doc(hidden)] - const FIELDS: &'static [&'static str] = &[ #(#field_names),* ]; - } - }; - - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, None); - - let visit_map = deserialize_map_in_place(params, fields, cattrs); - - (field_visitor, fields_stmt, visit_map) -} - #[cfg(feature = "deserialize_in_place")] fn deserialize_map_in_place( params: &Parameters, From 935f0bd70fa5935ed462056aae5402e3269ae574 Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 1 May 2023 02:15:54 +0500 Subject: [PATCH 2/8] Merge some quote! blocks --- serde_derive/src/de.rs | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 904fd5fa..cad79d30 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1171,22 +1171,14 @@ fn deserialize_struct_in_place( }) .collect(); - let fields_stmt = { - let field_names = field_names_idents.iter().map(|(name, _, _)| name); - quote_block! { - #[doc(hidden)] - const FIELDS: &'static [&'static str] = &[ #(#field_names),* ]; - } - }; - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, None); let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs, expecting)); let visit_map = deserialize_map_in_place(params, fields, cattrs); let field_visitor = Stmts(field_visitor); - let fields_stmt = Stmts(fields_stmt); let visit_map = Stmts(visit_map); + let field_names = field_names_idents.iter().map(|(name, _, _)| name); let visitor_expr = quote! { __Visitor { @@ -1216,16 +1208,6 @@ fn deserialize_struct_in_place( quote!(mut __seq) }; - let visit_seq = quote! { - #[inline] - fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::__private::Result - where - __A: _serde::de::SeqAccess<#delife>, - { - #visit_seq - } - }; - let in_place_impl_generics = de_impl_generics.in_place(); let in_place_ty_generics = de_ty_generics.in_place(); let place_life = place_lifetime(); @@ -1246,7 +1228,13 @@ fn deserialize_struct_in_place( _serde::__private::Formatter::write_str(__formatter, #expecting) } - #visit_seq + #[inline] + fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::__private::Result + where + __A: _serde::de::SeqAccess<#delife>, + { + #visit_seq + } #[inline] fn visit_map<__A>(self, mut __map: __A) -> _serde::__private::Result @@ -1257,7 +1245,8 @@ fn deserialize_struct_in_place( } } - #fields_stmt + #[doc(hidden)] + const FIELDS: &'static [&'static str] = &[ #(#field_names),* ]; #dispatch }) From 3a3e6bf103aec362e6be9c527ce0e8498aa981d0 Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 1 May 2023 01:14:58 +0500 Subject: [PATCH 3/8] Reorder variables to match order in final quote! --- serde_derive/src/de.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index cad79d30..162ae5e1 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1171,13 +1171,21 @@ fn deserialize_struct_in_place( }) .collect(); - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, None); + let field_visitor = Stmts(deserialize_generated_identifier( + &field_names_idents, + cattrs, + false, + None, + )); + let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); + let visitor_var = if all_skipped { + quote!(_) + } else { + quote!(mut __seq) + }; let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs, expecting)); - let visit_map = deserialize_map_in_place(params, fields, cattrs); - - let field_visitor = Stmts(field_visitor); - let visit_map = Stmts(visit_map); + let visit_map = Stmts(deserialize_map_in_place(params, fields, cattrs)); let field_names = field_names_idents.iter().map(|(name, _, _)| name); let visitor_expr = quote! { @@ -1201,13 +1209,6 @@ fn deserialize_struct_in_place( } }; - let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); - let visitor_var = if all_skipped { - quote!(_) - } else { - quote!(mut __seq) - }; - let in_place_impl_generics = de_impl_generics.in_place(); let in_place_ty_generics = de_ty_generics.in_place(); let place_life = place_lifetime(); From afe3872810c48f0b135808761a47aeaaab117599 Mon Sep 17 00:00:00 2001 From: Mingun Date: Sat, 6 May 2023 20:03:04 +0500 Subject: [PATCH 4/8] Simplify check for missing fields --- serde_derive/src/de.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 162ae5e1..1aa4d482 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1178,8 +1178,7 @@ fn deserialize_struct_in_place( None, )); - let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); - let visitor_var = if all_skipped { + let mut_seq = if field_names_idents.is_empty() { quote!(_) } else { quote!(mut __seq) @@ -1230,7 +1229,7 @@ fn deserialize_struct_in_place( } #[inline] - fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::__private::Result + fn visit_seq<__A>(self, #mut_seq: __A) -> _serde::__private::Result where __A: _serde::de::SeqAccess<#delife>, { From 99fde4ee3e5e1c87411837ec97f5822f2209693f Mon Sep 17 00:00:00 2001 From: Mingun Date: Sat, 6 May 2023 20:07:28 +0500 Subject: [PATCH 5/8] Implement #2387 also for deserialize_in_place method --- serde_derive/src/de.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 1aa4d482..c3e65393 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1185,7 +1185,9 @@ fn deserialize_struct_in_place( }; let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs, expecting)); let visit_map = Stmts(deserialize_map_in_place(params, fields, cattrs)); - let field_names = field_names_idents.iter().map(|(name, _, _)| name); + let field_names = field_names_idents + .iter() + .flat_map(|(_, _, aliases)| aliases); let visitor_expr = quote! { __Visitor { From cae1b43829f2292fec65513be2d73b3c0c90ed4a Mon Sep 17 00:00:00 2001 From: Mingun Date: Sat, 6 May 2023 22:25:45 +0500 Subject: [PATCH 6/8] Inline `deserialize_newtype_struct_in_place` --- serde_derive/src/de.rs | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index c3e65393..7f7e821a 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -610,7 +610,19 @@ fn deserialize_tuple_in_place( let nfields = fields.len(); let visit_newtype_struct = if !is_enum && nfields == 1 { - Some(deserialize_newtype_struct_in_place(params, &fields[0])) + // We do not generate deserialize_in_place if every field has a + // deserialize_with. + assert!(fields[0].attrs.deserialize_with().is_none()); + + Some(quote! { + #[inline] + fn visit_newtype_struct<__E>(self, __e: __E) -> _serde::__private::Result + where + __E: _serde::Deserializer<#delife>, + { + _serde::Deserialize::deserialize_in_place(__e, &mut self.place.0) + } + }) } else { None }; @@ -918,25 +930,6 @@ fn deserialize_newtype_struct( } } -#[cfg(feature = "deserialize_in_place")] -fn deserialize_newtype_struct_in_place(params: &Parameters, field: &Field) -> TokenStream { - // We do not generate deserialize_in_place if every field has a - // deserialize_with. - assert!(field.attrs.deserialize_with().is_none()); - - let delife = params.borrowed.de_lifetime(); - - quote! { - #[inline] - fn visit_newtype_struct<__E>(self, __e: __E) -> _serde::__private::Result - where - __E: _serde::Deserializer<#delife>, - { - _serde::Deserialize::deserialize_in_place(__e, &mut self.place.0) - } - } -} - enum StructForm<'a> { Struct, /// Contains a variant name From 59ec8b7db2485ebd2e935a9c929e050cb72380e2 Mon Sep 17 00:00:00 2001 From: Mingun Date: Sun, 7 May 2023 00:53:09 +0500 Subject: [PATCH 7/8] Remove dead code - variant_ident and deserializer are always None --- serde_derive/src/de.rs | 39 +++++++-------------------------------- 1 file changed, 7 insertions(+), 32 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 7f7e821a..40fd3678 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -324,10 +324,10 @@ fn deserialize_in_place_body(cont: &Container, params: &Parameters) -> Option { - deserialize_struct_in_place(None, params, fields, &cont.attrs, None)? + deserialize_struct_in_place(params, fields, &cont.attrs)? } Data::Struct(Style::Tuple, fields) | Data::Struct(Style::Newtype, fields) => { - deserialize_tuple_in_place(None, params, fields, &cont.attrs, None) + deserialize_tuple_in_place(params, fields, &cont.attrs) } Data::Enum(_) | Data::Struct(Style::Unit, _) => { return None; @@ -582,11 +582,9 @@ fn deserialize_tuple( #[cfg(feature = "deserialize_in_place")] fn deserialize_tuple_in_place( - variant_ident: Option, params: &Parameters, fields: &[Field], cattrs: &attr::Container, - deserializer: Option, ) -> Fragment { assert!(!cattrs.has_flatten()); @@ -600,16 +598,12 @@ fn deserialize_tuple_in_place( split_with_de_lifetime(params); let delife = params.borrowed.de_lifetime(); - let is_enum = variant_ident.is_some(); - let expecting = match variant_ident { - Some(variant_ident) => format!("tuple variant {}::{}", params.type_name(), variant_ident), - None => format!("tuple struct {}", params.type_name()), - }; + let expecting = format!("tuple struct {}", params.type_name()); let expecting = cattrs.expecting().unwrap_or(&expecting); let nfields = fields.len(); - let visit_newtype_struct = if !is_enum && nfields == 1 { + let visit_newtype_struct = if nfields == 1 { // We do not generate deserialize_in_place if every field has a // deserialize_with. assert!(fields[0].attrs.deserialize_with().is_none()); @@ -636,11 +630,7 @@ fn deserialize_tuple_in_place( } }; - let dispatch = if let Some(deserializer) = deserializer { - quote!(_serde::Deserializer::deserialize_tuple(#deserializer, #field_count, #visitor_expr)) - } else if is_enum { - quote!(_serde::de::VariantAccess::tuple_variant(__variant, #field_count, #visitor_expr)) - } else if nfields == 1 { + let dispatch = if nfields == 1 { let type_name = cattrs.name().deserialize_name(); quote!(_serde::Deserializer::deserialize_newtype_struct(__deserializer, #type_name, #visitor_expr)) } else { @@ -1126,14 +1116,10 @@ fn deserialize_struct( #[cfg(feature = "deserialize_in_place")] fn deserialize_struct_in_place( - variant_ident: Option, params: &Parameters, fields: &[Field], cattrs: &attr::Container, - deserializer: Option, ) -> Option { - let is_enum = variant_ident.is_some(); - // for now we do not support in_place deserialization for structs that // are represented as map. if cattrs.has_flatten() { @@ -1145,10 +1131,7 @@ fn deserialize_struct_in_place( split_with_de_lifetime(params); let delife = params.borrowed.de_lifetime(); - let expecting = match variant_ident { - Some(variant_ident) => format!("struct variant {}::{}", params.type_name(), variant_ident), - None => format!("struct {}", params.type_name()), - }; + let expecting = format!("struct {}", params.type_name()); let expecting = cattrs.expecting().unwrap_or(&expecting); let field_names_idents: Vec<_> = fields @@ -1188,15 +1171,7 @@ fn deserialize_struct_in_place( lifetime: _serde::__private::PhantomData, } }; - let dispatch = if let Some(deserializer) = deserializer { - quote! { - _serde::Deserializer::deserialize_any(#deserializer, #visitor_expr) - } - } else if is_enum { - quote! { - _serde::de::VariantAccess::struct_variant(__variant, FIELDS, #visitor_expr) - } - } else { + let dispatch = { let type_name = cattrs.name().deserialize_name(); quote! { _serde::Deserializer::deserialize_struct(__deserializer, #type_name, FIELDS, #visitor_expr) From 878110a4bc602505605fc12938ee22dbc1feb3a6 Mon Sep 17 00:00:00 2001 From: Mingun Date: Sun, 7 May 2023 00:59:44 +0500 Subject: [PATCH 8/8] Simplify code after dead code elimination --- serde_derive/src/de.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 40fd3678..1852b5be 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -630,11 +630,10 @@ fn deserialize_tuple_in_place( } }; + let type_name = cattrs.name().deserialize_name(); let dispatch = if nfields == 1 { - let type_name = cattrs.name().deserialize_name(); quote!(_serde::Deserializer::deserialize_newtype_struct(__deserializer, #type_name, #visitor_expr)) } else { - let type_name = cattrs.name().deserialize_name(); quote!(_serde::Deserializer::deserialize_tuple_struct(__deserializer, #type_name, #field_count, #visitor_expr)) }; @@ -1164,19 +1163,7 @@ fn deserialize_struct_in_place( let field_names = field_names_idents .iter() .flat_map(|(_, _, aliases)| aliases); - - let visitor_expr = quote! { - __Visitor { - place: __place, - lifetime: _serde::__private::PhantomData, - } - }; - let dispatch = { - let type_name = cattrs.name().deserialize_name(); - quote! { - _serde::Deserializer::deserialize_struct(__deserializer, #type_name, FIELDS, #visitor_expr) - } - }; + let type_name = cattrs.name().deserialize_name(); let in_place_impl_generics = de_impl_generics.in_place(); let in_place_ty_generics = de_ty_generics.in_place(); @@ -1218,7 +1205,10 @@ fn deserialize_struct_in_place( #[doc(hidden)] const FIELDS: &'static [&'static str] = &[ #(#field_names),* ]; - #dispatch + _serde::Deserializer::deserialize_struct(__deserializer, #type_name, FIELDS, __Visitor { + place: __place, + lifetime: _serde::__private::PhantomData, + }) }) }