Peternator7/update phf logic (Peternator7#224)

* Support phf crate for faster match on large enums

* comment

* add changelog entry

* add tests, embed phf, and improve lowercase support

* fix doc change

* Refactor & support case insensitive with case sensitive phf match

* more tests, some fixes of new implementation and prep for feature

* Modify the initial implementation.

Co-authored-by: Thomas BESSOU <[email protected]>
Co-authored-by: Peter Glotfelty <[email protected]>

strum_macros/src/macros/strings/from_string.rs

@@ -20,12 +20,8 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
     let mut default_kw = None;
     let mut default =
         quote! { ::core::result::Result::Err(#strum_module_path::ParseError::VariantNotFound) };
+
     let mut phf_exact_match_arms = Vec::new();
-    // We'll use the first one if there are many variants
-    let mut phf_lowercase_arms = Vec::new();
-    // However if there are few variants we'll want to integrate these in the standard match to avoid alloc
-    let mut case_insensitive_arms_alternative = Vec::new();
-    // Later we can add custom arms in there
     let mut standard_match_arms = Vec::new();
     for variant in variants {
         let ident = &variant.ident;
@@ -80,19 +76,19 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
         // If we don't have any custom variants, add the default serialized name.
         for serialization in variant_properties.get_serializations(type_properties.case_style) {
             if type_properties.use_phf {
-                if !is_ascii_case_insensitive {
-                    phf_exact_match_arms.push(quote! { #serialization => #name::#ident #params, });
-                } else {
-                    // In that case we'll store the lowercase values in phf, and lowercase at runtime
-                    // before searching
-                    // Unless there are few such variants, in that case we'll use the standard match with
-                    // eq_ignore_ascii_case to avoid allocating
-                    case_insensitive_arms_alternative.push(quote! { s if s.eq_ignore_ascii_case(#serialization) => #name::#ident #params, });
-
-                    let mut ser_string = serialization.value();
-                    ser_string.make_ascii_lowercase();
-                    let serialization = syn::LitStr::new(&ser_string, serialization.span());
-                    phf_lowercase_arms.push(quote! { #serialization => #name::#ident #params, });
+                phf_exact_match_arms.push(quote! { #serialization => #name::#ident #params, });
+
+                if is_ascii_case_insensitive {
+                    // Store the lowercase and UPPERCASE variants in the phf map to capture 
+                    let ser_string = serialization.value();
+
+                    let lower =
+                        syn::LitStr::new(&ser_string.to_ascii_lowercase(), serialization.span());
+                    let upper =
+                        syn::LitStr::new(&ser_string.to_ascii_uppercase(), serialization.span());
+                    phf_exact_match_arms.push(quote! { #lower => #name::#ident #params, });
+                    phf_exact_match_arms.push(quote! { #upper => #name::#ident #params, });
+                    standard_match_arms.push(quote! { s if s.eq_ignore_ascii_case(#serialization) => #name::#ident #params, });
                 }
             } else {
                 standard_match_arms.push(if !is_ascii_case_insensitive {
@@ -104,24 +100,11 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
         }
     }
 
-    // Probably under that string allocation is more expensive than matching few times
-    // Proper threshold is not benchmarked - feel free to do so :)
-    if phf_lowercase_arms.len() <= 3 {
-        standard_match_arms.extend(case_insensitive_arms_alternative);
-        phf_lowercase_arms.clear();
-    }
-
-    let use_phf = if phf_exact_match_arms.is_empty() && phf_lowercase_arms.is_empty() {
-        quote!()
-    } else {
-        quote! {
-            use #strum_module_path::_private_phf_reexport_for_macro_if_phf_feature as phf;
-        }
-    };
     let phf_body = if phf_exact_match_arms.is_empty() {
         quote!()
     } else {
         quote! {
+            use #strum_module_path::_private_phf_reexport_for_macro_if_phf_feature as phf;
             static PHF: phf::Map<&'static str, #name> = phf::phf_map! {
                 #(#phf_exact_match_arms)*
             };
@@ -130,18 +113,6 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
             }
         }
     };
-    let phf_lowercase_body = if phf_lowercase_arms.is_empty() {
-        quote!()
-    } else {
-        quote! {
-            static PHF_LOWERCASE: phf::Map<&'static str, #name> = phf::phf_map! {
-                #(#phf_lowercase_arms)*
-            };
-            if let Some(value) = PHF_LOWERCASE.get(&s.to_ascii_lowercase()).cloned() {
-                return ::core::result::Result::Ok(value);
-            }
-        }
-    };
     let standard_match_body = if standard_match_arms.is_empty() {
         default
     } else {
@@ -158,9 +129,7 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
         impl #impl_generics ::core::str::FromStr for #name #ty_generics #where_clause {
             type Err = #strum_module_path::ParseError;
             fn from_str(s: &str) -> ::core::result::Result< #name #ty_generics , <Self as ::core::str::FromStr>::Err> {
-                #use_phf
                 #phf_body
-                #phf_lowercase_body
                 #standard_match_body
             }
         }