From 2a36d1123865653be1c47eebcd3059c9bdee7b4b Mon Sep 17 00:00:00 2001 From: Mingun Date: Mon, 12 Oct 2020 00:23:45 +0500 Subject: [PATCH 1/5] Introduce a dedicated function for generating Field enum (the enum that represents all fields of a struct) --- serde_derive/src/de.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index d93cac6ec..d5c32b97b 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -965,12 +965,7 @@ fn deserialize_struct( ) }) .collect(); - let field_visitor = Stmts(deserialize_generated_identifier( - &field_names_idents, - cattrs, - false, - None, - )); + let field_visitor = deserialize_field_identifier(&field_names_idents, cattrs); // untagged struct variants do not get a visit_seq method. The same applies to // structs that only have a map representation. @@ -1128,12 +1123,7 @@ fn deserialize_struct_in_place( }) .collect(); - let field_visitor = Stmts(deserialize_generated_identifier( - &field_names_idents, - cattrs, - false, - None, - )); + let field_visitor = deserialize_field_identifier(&field_names_idents, cattrs); let mut_seq = if field_names_idents.is_empty() { quote!(_) @@ -2052,6 +2042,20 @@ fn deserialize_generated_identifier( } } +/// Generates enum and its `Deserialize` implementation that represents each +/// non-skipped field of the struct +fn deserialize_field_identifier( + fields: &[(&str, Ident, &BTreeSet)], + cattrs: &attr::Container, +) -> Stmts { + Stmts(deserialize_generated_identifier( + fields, + cattrs, + false, + None, + )) +} + // Generates `Deserialize::deserialize` body for an enum with // `serde(field_identifier)` or `serde(variant_identifier)` attribute. fn deserialize_custom_identifier( From 5e313a7330cae656190ca9d328537846df495eb8 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 16 Oct 2020 21:18:44 +0500 Subject: [PATCH 2/5] =?UTF-8?q?Move=20generi=D1=81=20code=20out-of-functio?= =?UTF-8?q?n,=20create=20more=20specialized=20and=20simple=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- serde_derive/src/de.rs | 45 ++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index d5c32b97b..cfbdfd637 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1247,11 +1247,20 @@ fn prepare_enum_variant_enum( } }; + let (ignore_variant, fallthrough) = if let Some(other_idx) = other_idx { + let ignore_variant = variant_names_idents[other_idx].1.clone(); + let fallthrough = quote!(_serde::__private::Ok(__Field::#ignore_variant)); + (None, Some(fallthrough)) + } else { + (None, None) + }; + let variant_visitor = Stmts(deserialize_generated_identifier( &variant_names_idents, cattrs, true, - other_idx, + ignore_variant, + fallthrough, )); (variants_stmt, variant_visitor) @@ -1976,27 +1985,12 @@ fn deserialize_generated_identifier( fields: &[(&str, Ident, &BTreeSet)], cattrs: &attr::Container, is_variant: bool, - other_idx: Option, + ignore_variant: Option, + fallthrough: Option, ) -> Fragment { let this_value = quote!(__Field); let field_idents: &Vec<_> = &fields.iter().map(|(_, ident, _)| ident).collect(); - let (ignore_variant, fallthrough) = if !is_variant && cattrs.has_flatten() { - let ignore_variant = quote!(__other(_serde::__private::de::Content<'de>),); - let fallthrough = quote!(_serde::__private::Ok(__Field::__other(__value))); - (Some(ignore_variant), Some(fallthrough)) - } else if let Some(other_idx) = other_idx { - let ignore_variant = fields[other_idx].1.clone(); - let fallthrough = quote!(_serde::__private::Ok(__Field::#ignore_variant)); - (None, Some(fallthrough)) - } else if is_variant || cattrs.deny_unknown_fields() { - (None, None) - } else { - let ignore_variant = quote!(__ignore,); - let fallthrough = quote!(_serde::__private::Ok(__Field::__ignore)); - (Some(ignore_variant), Some(fallthrough)) - }; - let visitor_impl = Stmts(deserialize_identifier( &this_value, fields, @@ -2048,11 +2042,24 @@ fn deserialize_field_identifier( fields: &[(&str, Ident, &BTreeSet)], cattrs: &attr::Container, ) -> Stmts { + let (ignore_variant, fallthrough) = if cattrs.has_flatten() { + let ignore_variant = quote!(__other(_serde::__private::de::Content<'de>),); + let fallthrough = quote!(_serde::__private::Ok(__Field::__other(__value))); + (Some(ignore_variant), Some(fallthrough)) + } else if cattrs.deny_unknown_fields() { + (None, None) + } else { + let ignore_variant = quote!(__ignore,); + let fallthrough = quote!(_serde::__private::Ok(__Field::__ignore)); + (Some(ignore_variant), Some(fallthrough)) + }; + Stmts(deserialize_generated_identifier( fields, cattrs, false, - None, + ignore_variant, + fallthrough, )) } From ada50b077e1ea0d573209823cba2afd0f5494a43 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 16 Oct 2020 21:33:22 +0500 Subject: [PATCH 3/5] ignore_variant variable is always None, let's take this into account --- serde_derive/src/de.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index cfbdfd637..dfdb8a25b 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1247,19 +1247,19 @@ fn prepare_enum_variant_enum( } }; - let (ignore_variant, fallthrough) = if let Some(other_idx) = other_idx { + let fallthrough = if let Some(other_idx) = other_idx { let ignore_variant = variant_names_idents[other_idx].1.clone(); let fallthrough = quote!(_serde::__private::Ok(__Field::#ignore_variant)); - (None, Some(fallthrough)) + Some(fallthrough) } else { - (None, None) + None }; let variant_visitor = Stmts(deserialize_generated_identifier( &variant_names_idents, cattrs, true, - ignore_variant, + None, fallthrough, )); From b58e8bac12bbc81d6d336356a292c920903dfaa8 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 16 Oct 2020 21:36:25 +0500 Subject: [PATCH 4/5] Replace `if let Some(...) = ...` to Option::map --- serde_derive/src/de.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index dfdb8a25b..20ca8ccc1 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1247,13 +1247,10 @@ fn prepare_enum_variant_enum( } }; - let fallthrough = if let Some(other_idx) = other_idx { + let fallthrough = other_idx.map(|other_idx| { let ignore_variant = variant_names_idents[other_idx].1.clone(); - let fallthrough = quote!(_serde::__private::Ok(__Field::#ignore_variant)); - Some(fallthrough) - } else { - None - }; + quote!(_serde::__private::Ok(__Field::#ignore_variant)) + }); let variant_visitor = Stmts(deserialize_generated_identifier( &variant_names_idents, From 070cce0d9c718ced830158a08fb886aa7228bb9d Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 16 Oct 2020 21:42:01 +0500 Subject: [PATCH 5/5] Get rid of temporary variable --- serde_derive/src/de.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 20ca8ccc1..aa3a7017d 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1237,7 +1237,12 @@ fn prepare_enum_variant_enum( }) .collect(); - let other_idx = deserialized_variants.position(|(_, variant)| variant.attrs.other()); + let fallthrough = deserialized_variants + .position(|(_, variant)| variant.attrs.other()) + .map(|other_idx| { + let ignore_variant = variant_names_idents[other_idx].1.clone(); + quote!(_serde::__private::Ok(__Field::#ignore_variant)) + }); let variants_stmt = { let variant_names = variant_names_idents.iter().map(|(name, _, _)| name); @@ -1247,11 +1252,6 @@ fn prepare_enum_variant_enum( } }; - let fallthrough = other_idx.map(|other_idx| { - let ignore_variant = variant_names_idents[other_idx].1.clone(); - quote!(_serde::__private::Ok(__Field::#ignore_variant)) - }); - let variant_visitor = Stmts(deserialize_generated_identifier( &variant_names_idents, cattrs,