Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for PHF in EnumString #220

Merged
merged 8 commits into from Apr 30, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,10 @@
# Changelog

## Unreleased

* [#220](https://github.com/Peternator7/strum/pull/220). Add support for PHF in `EnumString` (opt-in runtime
performance improvements for large enums as `#[strum(use_phf)]`, requires `phf` feature and increases MSRV to `1.46`)

## 0.24.0

* [#212](https://github.com/Peternator7/strum/pull/212). Fix some clippy lints
Expand Down
1 change: 1 addition & 0 deletions strum/Cargo.toml
Expand Up @@ -16,6 +16,7 @@ readme = "../README.md"

[dependencies]
strum_macros = { path = "../strum_macros", optional = true, version = "0.24" }
phf = { version = "0.10", features = ["macros"], optional = true }

[dev-dependencies]
strum_macros = { path = "../strum_macros", version = "0.24" }
Expand Down
4 changes: 4 additions & 0 deletions strum/src/lib.rs
Expand Up @@ -30,6 +30,10 @@
// only for documentation purposes
pub mod additional_attributes;

#[cfg(feature = "phf")]
Peternator7 marked this conversation as resolved.
Show resolved Hide resolved
#[doc(hidden)]
pub use phf as _private_phf_reexport_for_macro_if_phf_feature;

/// The `ParseError` enum is a collection of all the possible reasons
/// an enum can fail to parse from a string.
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
Expand Down
28 changes: 19 additions & 9 deletions strum_macros/src/helpers/metadata.rs
Expand Up @@ -5,7 +5,8 @@ use syn::{
parse2, parse_str,
punctuated::Punctuated,
spanned::Spanned,
Attribute, DeriveInput, Ident, Lit, LitBool, LitStr, Meta, MetaNameValue, Path, Token, Variant, Visibility,
Attribute, DeriveInput, Ident, Lit, LitBool, LitStr, Meta, MetaNameValue, Path, Token, Variant,
Visibility,
};

use super::case_style::CaseStyle;
Expand All @@ -16,6 +17,7 @@ pub mod kw {

// enum metadata
custom_keyword!(serialize_all);
custom_keyword!(use_phf);

// enum discriminant metadata
custom_keyword!(derive);
Expand Down Expand Up @@ -43,6 +45,7 @@ pub enum EnumMeta {
kw: kw::Crate,
crate_module_path: Path,
},
UsePhf(kw::use_phf),
}

impl Parse for EnumMeta {
Expand All @@ -64,8 +67,9 @@ impl Parse for EnumMeta {
crate_module_path,
})
} else if lookahead.peek(kw::ascii_case_insensitive) {
let kw = input.parse()?;
Ok(EnumMeta::AsciiCaseInsensitive(kw))
Ok(EnumMeta::AsciiCaseInsensitive(input.parse()?))
} else if lookahead.peek(kw::use_phf) {
Ok(EnumMeta::UsePhf(input.parse()?))
} else {
Err(lookahead.error())
}
Expand All @@ -78,6 +82,7 @@ impl Spanned for EnumMeta {
EnumMeta::SerializeAll { kw, .. } => kw.span(),
EnumMeta::AsciiCaseInsensitive(kw) => kw.span(),
EnumMeta::Crate { kw, .. } => kw.span(),
EnumMeta::UsePhf(use_phf) => use_phf.span(),
}
}
}
Expand Down Expand Up @@ -275,14 +280,19 @@ pub trait VariantExt {
impl VariantExt for Variant {
fn get_metadata(&self) -> syn::Result<Vec<VariantMeta>> {
let result = get_metadata_inner("strum", &self.attrs)?;
self.attrs.iter()
self.attrs
.iter()
.filter(|attr| attr.path.is_ident("doc"))
.try_fold(result, |mut vec, attr| {
if let Meta::NameValue(MetaNameValue { lit: Lit::Str(value), .. }) = attr.parse_meta()? {
vec.push(VariantMeta::Documentation { value })
}
Ok(vec)
})
if let Meta::NameValue(MetaNameValue {
lit: Lit::Str(value),
..
}) = attr.parse_meta()?
{
vec.push(VariantMeta::Documentation { value })
}
Ok(vec)
})
}
}

Expand Down
10 changes: 10 additions & 0 deletions strum_macros/src/helpers/type_props.rs
Expand Up @@ -20,6 +20,7 @@ pub struct StrumTypeProperties {
pub discriminant_name: Option<Ident>,
pub discriminant_others: Vec<TokenStream>,
pub discriminant_vis: Option<Visibility>,
pub use_phf: bool,
}

impl HasTypeProperties for DeriveInput {
Expand All @@ -31,6 +32,7 @@ impl HasTypeProperties for DeriveInput {

let mut serialize_all_kw = None;
let mut ascii_case_insensitive_kw = None;
let mut use_phf_kw = None;
let mut crate_module_path_kw = None;
for meta in strum_meta {
match meta {
Expand All @@ -50,6 +52,14 @@ impl HasTypeProperties for DeriveInput {
ascii_case_insensitive_kw = Some(kw);
output.ascii_case_insensitive = true;
}
EnumMeta::UsePhf(kw) => {
if let Some(fst_kw) = use_phf_kw {
return Err(occurrence_error(fst_kw, kw, "use_phf"));
}

use_phf_kw = Some(kw);
output.use_phf = true;
}
EnumMeta::Crate {
crate_module_path,
kw,
Expand Down
7 changes: 5 additions & 2 deletions strum_macros/src/lib.rs
Expand Up @@ -47,6 +47,9 @@ fn debug_print_generated(ast: &DeriveInput, toks: &TokenStream) {
/// See the [Additional Attributes](https://docs.rs/strum/0.22/strum/additional_attributes/index.html)
/// Section for more information on using this feature.
///
/// If you have a large enum, you may want to consider using the `use_phf` attribute here. It leverages
/// perfect hash functions to parse much quicker than a standard `match`. (MSRV 1.46)
///
/// # Example howto use `EnumString`
/// ```
/// use std::str::FromStr;
Expand Down Expand Up @@ -471,11 +474,11 @@ pub fn from_repr(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
/// Encode strings into the enum itself. The `strum_macros::EmumMessage` macro implements the `strum::EnumMessage` trait.
/// `EnumMessage` looks for `#[strum(message="...")]` attributes on your variants.
/// You can also provided a `detailed_message="..."` attribute to create a seperate more detailed message than the first.
///
///
/// `EnumMessage` also exposes the variants doc comments through `get_documentation()`. This is useful in some scenarios,
/// but `get_message` should generally be preferred. Rust doc comments are intended for developer facing documentation,
/// not end user messaging.
///
///
/// ```
/// // You need to bring the trait into scope to use it
/// use strum::EnumMessage;
Expand Down
67 changes: 56 additions & 11 deletions strum_macros/src/macros/strings/from_string.rs
Expand Up @@ -18,7 +18,8 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
let strum_module_path = type_properties.crate_module_path();

let mut default_kw = None;
let mut default = quote! { _ => ::core::result::Result::Err(#strum_module_path::ParseError::VariantNotFound) };
let mut default =
quote! { ::core::result::Result::Err(#strum_module_path::ParseError::VariantNotFound) };
let mut arms = Vec::new();
for variant in variants {
let ident = &variant.ident;
Expand All @@ -45,21 +46,43 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {

default_kw = Some(kw);
default = quote! {
default => ::core::result::Result::Ok(#name::#ident(default.into()))
::core::result::Result::Ok(#name::#ident(s.into()))
};
continue;
}

let is_ascii_case_insensitive = variant_properties
.ascii_case_insensitive
.unwrap_or(type_properties.ascii_case_insensitive);
let is_ascii_case_insensitive = match (
type_properties.use_phf,
type_properties.ascii_case_insensitive,
variant_properties.ascii_case_insensitive,
) {
(false, _, Some(variant_case)) => variant_case,
(false, type_case, None) => type_case,
(true, false, None | Some(false)) => false,
(true, true, None | Some(true)) => true,
(true, false, Some(true)) | (true, true, Some(false)) => {
return Err(syn::Error::new(
proc_macro2::Span::call_site(),
"Cannot use per-variant case insensitive parsing in combination with `phf`",
))
}
};
// If we don't have any custom variants, add the default serialized name.
let attrs = variant_properties
.get_serializations(type_properties.case_style)
.into_iter()
.map(|serialization| {
if is_ascii_case_insensitive {
quote! { s if s.eq_ignore_ascii_case(#serialization) }
if type_properties.use_phf {
// In that case we'll store the lowercase values in phf, and lowercase at runtime
// before searching
let mut ser_string = serialization.value();
ser_string.make_ascii_lowercase();
let serialization = syn::LitStr::new(&ser_string, serialization.span());
quote! { #serialization }
} else {
quote! { s if s.eq_ignore_ascii_case(#serialization) }
}
} else {
quote! { #serialization }
}
Expand All @@ -81,19 +104,41 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
}
};

arms.push(quote! { #(#attrs => ::core::result::Result::Ok(#name::#ident #params)),* });
arms.push(quote! { #(#attrs => #name::#ident #params,)* });
}

arms.push(default);
let body = if type_properties.use_phf {
let maybe_to_lowercase = if type_properties.ascii_case_insensitive {
// This will allocate but I'm afraid we don't have anything better ATM :(
quote! {
let s = &s.to_ascii_lowercase();
}
} else {
quote!()
};
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! {
#(#arms)*
};
#maybe_to_lowercase
PHF.get(s).cloned().map_or_else(|| #default, ::core::result::Result::Ok)
}
} else {
quote! {
::core::result::Result::Ok(match s {
#(#arms)*
_ => return #default,
})
}
};

let from_str = quote! {
#[allow(clippy::use_self)]
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> {
match s {
#(#arms),*
}
#body
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion strum_tests/Cargo.toml
Expand Up @@ -5,7 +5,7 @@ edition = "2018"
authors = ["Peter Glotfelty <peglotfe@microsoft.com>"]

[dependencies]
strum = { path = "../strum", features = ["derive"] }
strum = { path = "../strum", features = ["derive", "phf"] }
strum_macros = { path = "../strum_macros", features = [] }
clap = "=2.33.0"
enum_variant_type = "=0.2.0"
Expand Down
23 changes: 23 additions & 0 deletions strum_tests/tests/phf.rs
@@ -0,0 +1,23 @@
#[rustversion::since(1.46)]
#[test]
fn from_str_with_phf() {
#[derive(Debug, PartialEq, Eq, Clone, strum::EnumString)]
#[strum(use_phf)]
enum Color {
Blue,
Red,
}
assert_eq!("Blue".parse::<Color>().unwrap(), Color::Blue);
}

#[rustversion::since(1.46)]
#[test]
fn from_str_with_phf_case_insensitive() {
#[derive(Debug, PartialEq, Eq, Clone, strum::EnumString)]
#[strum(use_phf, ascii_case_insensitive)]
enum Color {
Blue,
Red,
}
assert_eq!("bLuE".parse::<Color>().unwrap(), Color::Blue);
}