From 575c001a35ed80f7b1c262dd74bc846af175ce96 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sun, 23 Jan 2022 16:20:02 -0800 Subject: [PATCH 1/6] minimal extraction of enum_metadata from PR #207. --- strum/src/lib.rs | 36 ++++++ strum_macros/src/lib.rs | 11 ++ strum_macros/src/macros/enum_metadata.rs | 149 +++++++++++++++++++++++ strum_macros/src/macros/from_repr.rs | 4 + strum_macros/src/macros/mod.rs | 1 + strum_tests/tests/enum_metadata.rs | 31 +++++ 6 files changed, 232 insertions(+) create mode 100644 strum_macros/src/macros/enum_metadata.rs create mode 100644 strum_tests/tests/enum_metadata.rs diff --git a/strum/src/lib.rs b/strum/src/lib.rs index 912cdb0f..a516b020 100644 --- a/strum/src/lib.rs +++ b/strum/src/lib.rs @@ -30,6 +30,8 @@ // only for documentation purposes pub mod additional_attributes; +use core::ops; + /// 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)] @@ -190,6 +192,39 @@ pub trait VariantNames { const VARIANTS: &'static [&'static str]; } +pub trait EnumMetadata { + /// The repr type as a trait associated type. + type Repr: Copy + + ops::BitOr + + ops::BitAnd + + ops::BitXor + + ops::Shr + + ops::Shl + + ops::Not + + ops::BitOrAssign + + ops::BitAndAssign + + ops::BitXorAssign + + ops::ShrAssign + + ops::ShlAssign + + core::fmt::Debug; + + /// The enum type, generally Self unless deriving EnumMetadata for a type which returns + /// metadata for another enum. + type EnumT: EnumMetadata; + + /// Variant names + const VARIANTS: &'static [&'static str]; + /// Number of variants + const COUNT: usize; + /// std::mem::size_of(). + const REPR_SIZE: usize; + + /// convert to the enums #[repr(..)] equivalent to `self as ..` + fn to_repr(self) -> Self::Repr; + /// Trait equivalent of EnumT::from_repr(...) + fn from_repr(repr: Self::Repr) -> Option; +} + #[cfg(feature = "derive")] pub use strum_macros::*; @@ -214,6 +249,7 @@ DocumentMacroRexports! { EnumCount, EnumDiscriminants, EnumIter, + EnumMetadata, EnumMessage, EnumProperty, EnumString, diff --git a/strum_macros/src/lib.rs b/strum_macros/src/lib.rs index 73185fe9..15e49e27 100644 --- a/strum_macros/src/lib.rs +++ b/strum_macros/src/lib.rs @@ -466,6 +466,17 @@ pub fn from_repr(input: proc_macro::TokenStream) -> proc_macro::TokenStream { toks.into() } +/// Enum Metadata documentation. +#[proc_macro_derive(EnumMetadata, attributes(strum))] +pub fn enum_metadata(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let ast = syn::parse_macro_input!(input as DeriveInput); + + let toks = macros::enum_metadata::enum_metadata_inner(&ast) + .unwrap_or_else(|err| err.to_compile_error()); + debug_print_generated(&ast, &toks); + toks.into() +} + /// Add a verbose message to an enum variant. /// /// Encode strings into the enum itself. The `strum_macros::EmumMessage` macro implements the `strum::EnumMessage` trait. diff --git a/strum_macros/src/macros/enum_metadata.rs b/strum_macros/src/macros/enum_metadata.rs new file mode 100644 index 00000000..dfe4b60c --- /dev/null +++ b/strum_macros/src/macros/enum_metadata.rs @@ -0,0 +1,149 @@ +use proc_macro2::{Span, TokenStream}; +use quote::{format_ident, quote}; +use syn::{Data, DeriveInput, PathArguments, Type, TypeParen}; + +use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; + +pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { + let name = &ast.ident; + let gen = &ast.generics; + let (impl_generics, ty_generics, where_clause) = gen.split_for_impl(); + let attrs = &ast.attrs; + + let mut discriminant_type: Type = syn::parse("usize".parse().unwrap()).unwrap(); + for attr in attrs { + let path = &attr.path; + let tokens = &attr.tokens; + if path.leading_colon.is_some() { + continue; + } + if path.segments.len() != 1 { + continue; + } + let segment = path.segments.first().unwrap(); + if segment.ident != "repr" { + continue; + } + if segment.arguments != PathArguments::None { + continue; + } + let typ_paren = match syn::parse2::(tokens.clone()) { + Ok(Type::Paren(TypeParen { elem, .. })) => *elem, + _ => continue, + }; + let inner_path = match &typ_paren { + Type::Path(t) => t, + _ => continue, + }; + if let Some(seg) = inner_path.path.segments.last() { + for t in &[ + "u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize", + ] { + if seg.ident == t { + discriminant_type = typ_paren; + break; + } + } + } + } + + if gen.lifetimes().count() > 0 { + return Err(syn::Error::new( + Span::call_site(), + "This macro doesn't support enums with lifetimes. \ + The resulting enums would be unbounded.", + )); + } + + let variants = match &ast.data { + Data::Enum(v) => &v.variants, + _ => return Err(non_enum_error()), + }; + + let mut arms = Vec::new(); + let mut constant_defs = Vec::new(); + let mut prev_const_var_ident = None; + for variant in variants { + use syn::Fields::*; + + if variant.get_variant_properties()?.disabled.is_some() { + continue; + } + + let ident = &variant.ident; + let params = match &variant.fields { + Unit => quote! {}, + Unnamed(fields) => { + let defaults = ::std::iter::repeat(quote!(::core::default::Default::default())) + .take(fields.unnamed.len()); + quote! { (#(#defaults),*) } + } + Named(fields) => { + let fields = fields + .named + .iter() + .map(|field| field.ident.as_ref().unwrap()); + quote! { {#(#fields: ::core::default::Default::default()),*} } + } + }; + + use heck::ToShoutySnakeCase; + let const_var_str = format!("{}_DISCRIMINANT", variant.ident).to_shouty_snake_case(); + let const_var_ident = format_ident!("{}", const_var_str); + + let const_val_expr = match &variant.discriminant { + Some((_, expr)) => quote! { #expr }, + None => match &prev_const_var_ident { + Some(prev) => quote! { #prev + 1 }, + None => quote! { 0 }, + }, + }; + + constant_defs.push(quote! {const #const_var_ident: #discriminant_type = #const_val_expr;}); + arms.push(quote! {v if v == #const_var_ident => ::core::option::Option::Some(#name::#ident #params)}); + + prev_const_var_ident = Some(const_var_ident); + } + + arms.push(quote! { _ => ::core::option::Option::None }); + + let type_properties = ast.get_type_properties()?; + + let variant_names = variants + .iter() + .map(|v| { + let props = v.get_variant_properties()?; + Ok(props.get_preferred_name(type_properties.case_style)) + }) + .collect::>>()?; + + let enum_count = variants.len(); + + Ok(quote! { + impl #impl_generics EnumMetadata for #name #ty_generics #where_clause { + #[doc = "The Repr type."] + type Repr = #discriminant_type; + #[doc = "The Enum type, typically Self unless implementing EnumMetadata for another enum type."] + type EnumT = Self; + + const VARIANTS: &'static [&'static str] = &[ #(#variant_names),* ]; + const COUNT: usize = #enum_count; + const REPR_SIZE: usize = std::mem::size_of::(); + + fn to_repr(self) -> #discriminant_type { + self as #discriminant_type + } + + // Note: synchronize changes with `FromRepr::from_repr`, + // it duplicates this logic in an inherent impl. + // Making it possible to have both impls on the same type; + // so their behavior must be kept the same. + fn from_repr(discriminant: #discriminant_type) -> Option { + #(#constant_defs)* + match discriminant { + #(#arms),* + } + } + } + }) +} diff --git a/strum_macros/src/macros/from_repr.rs b/strum_macros/src/macros/from_repr.rs index e3f2ac6f..efa81663 100644 --- a/strum_macros/src/macros/from_repr.rs +++ b/strum_macros/src/macros/from_repr.rs @@ -126,6 +126,10 @@ pub fn from_repr_inner(ast: &DeriveInput) -> syn::Result { filter_by_rust_version(quote! { const }) }; + // Note: synchronize changes with `EnumMetadata::from_repr`, + // it duplicates this logic in an inherent impl. + // Making it possible to have both impls on the same type; + // so their behavior must be kept the same. Ok(quote! { impl #impl_generics #name #ty_generics #where_clause { #[doc = "Try to create [Self] from the raw representation"] diff --git a/strum_macros/src/macros/mod.rs b/strum_macros/src/macros/mod.rs index b4129697..6580cb47 100644 --- a/strum_macros/src/macros/mod.rs +++ b/strum_macros/src/macros/mod.rs @@ -2,6 +2,7 @@ pub mod enum_count; pub mod enum_discriminants; pub mod enum_iter; pub mod enum_messages; +pub mod enum_metadata; pub mod enum_properties; pub mod enum_variant_names; pub mod from_repr; diff --git a/strum_tests/tests/enum_metadata.rs b/strum_tests/tests/enum_metadata.rs new file mode 100644 index 00000000..af1cbf83 --- /dev/null +++ b/strum_tests/tests/enum_metadata.rs @@ -0,0 +1,31 @@ +use strum::EnumMetadata; +// To check that from_repr impls on both the enum type, +// and EnumMetadata don't clash in any way. +use core::ops; +use strum::FromRepr; + +#[derive(Debug, Eq, PartialEq, EnumMetadata, FromRepr)] +#[repr(u8)] +enum ABC { + A = 1 << 0, + B = 1 << 1, + C = 1 << 2, +} + +#[test] +fn abc_variant_names() { + assert_eq!(ABC::VARIANTS, ["A", "B", "C"]); +} + +#[test] +fn abc_variant_count() { + assert_eq!(ABC::COUNT, 3); +} + +#[test] +fn abc_from_repr_same() { + assert_eq!( + ABC::from_repr(ABC::A as u8), + ::from_repr(ABC::A.to_repr()) + ) +} From 5c6b99a2502f07a4a1adfa6cab6f721df02e24ad Mon Sep 17 00:00:00 2001 From: matt rice Date: Tue, 25 Jan 2022 08:54:30 -0800 Subject: [PATCH 2/6] use core instead of std. --- strum_macros/src/macros/enum_metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/strum_macros/src/macros/enum_metadata.rs b/strum_macros/src/macros/enum_metadata.rs index dfe4b60c..d6c52405 100644 --- a/strum_macros/src/macros/enum_metadata.rs +++ b/strum_macros/src/macros/enum_metadata.rs @@ -74,7 +74,7 @@ pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { let params = match &variant.fields { Unit => quote! {}, Unnamed(fields) => { - let defaults = ::std::iter::repeat(quote!(::core::default::Default::default())) + let defaults = ::core::iter::repeat(quote!(::core::default::Default::default())) .take(fields.unnamed.len()); quote! { (#(#defaults),*) } } @@ -128,7 +128,7 @@ pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { const VARIANTS: &'static [&'static str] = &[ #(#variant_names),* ]; const COUNT: usize = #enum_count; - const REPR_SIZE: usize = std::mem::size_of::(); + const REPR_SIZE: usize = ::core::mem::size_of::(); fn to_repr(self) -> #discriminant_type { self as #discriminant_type From 66be5773a8346d4bc0450b32cda488b772cf7e0b Mon Sep 17 00:00:00 2001 From: matt rice Date: Wed, 26 Jan 2022 10:06:01 -0800 Subject: [PATCH 3/6] enum_metadata and from_repr, refactor duplicated code. --- strum_macros/src/helpers/metadata_impl.rs | 185 ++++++++++++++++++++++ strum_macros/src/helpers/mod.rs | 1 + strum_macros/src/macros/enum_metadata.rs | 118 ++------------ strum_macros/src/macros/from_repr.rs | 113 ++----------- strum_tests/tests/enum_metadata.rs | 1 - 5 files changed, 214 insertions(+), 204 deletions(-) create mode 100644 strum_macros/src/helpers/metadata_impl.rs diff --git a/strum_macros/src/helpers/metadata_impl.rs b/strum_macros/src/helpers/metadata_impl.rs new file mode 100644 index 00000000..415d1cd6 --- /dev/null +++ b/strum_macros/src/helpers/metadata_impl.rs @@ -0,0 +1,185 @@ +use proc_macro2::TokenStream; +use quote::{format_ident, quote}; +use syn::{Data, DeriveInput, PathArguments, Type, TypeParen}; + +use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; + +pub struct MetadataImpl<'a> { + ast: &'a syn::DeriveInput, + gen_names: Option>, + gen_from_repr: Option, + pub enum_count: usize, + pub has_additional_data: bool, +} + +pub struct FromReprTokens { + pub constant_defs: Vec, + pub match_arms: Vec, +} + +impl<'a> MetadataImpl<'a> { + pub fn new(ast: &'a DeriveInput) -> Self { + MetadataImpl { + ast, + enum_count: 0, + gen_names: None, + gen_from_repr: None, + has_additional_data: false, + } + } + + pub fn use_name_info(mut self) -> Self { + self.gen_names = Some(Vec::new()); + self + } + + pub fn use_from_repr(mut self) -> Self { + self.gen_from_repr = Some(FromReprTokens { + constant_defs: Vec::new(), + match_arms: Vec::new(), + }); + self + } + + pub fn discriminant_type(&self) -> Type { + let mut discriminant_type: Type = syn::parse("usize".parse().unwrap()).unwrap(); + for attr in &self.ast.attrs { + let path = &attr.path; + let tokens = &attr.tokens; + if path.leading_colon.is_some() { + continue; + } + if path.segments.len() != 1 { + continue; + } + let segment = path.segments.first().unwrap(); + if segment.ident != "repr" { + continue; + } + if segment.arguments != PathArguments::None { + continue; + } + let typ_paren = match syn::parse2::(tokens.clone()) { + Ok(Type::Paren(TypeParen { elem, .. })) => *elem, + _ => continue, + }; + let inner_path = match &typ_paren { + Type::Path(t) => t, + _ => continue, + }; + if let Some(seg) = inner_path.path.segments.last() { + for t in &[ + "u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize", + ] { + if seg.ident == t { + discriminant_type = typ_paren; + break; + } + } + } + } + discriminant_type + } + + fn params_from_fields(fields: &syn::Fields) -> (TokenStream, bool) { + use syn::Fields::*; + match &fields { + Unit => (quote! {}, false), + Unnamed(fields) => { + let defaults = ::std::iter::repeat(quote!(::core::default::Default::default())) + .take(fields.unnamed.len()); + (quote! { (#(#defaults),*) }, true) + } + Named(fields) => { + let fields = fields + .named + .iter() + .map(|field| field.ident.as_ref().unwrap()); + ( + quote! { {#(#fields: ::core::default::Default::default()),*} }, + true, + ) + } + } + } + + fn case_style(&self) -> Option { + if let Ok(props) = self.ast.get_type_properties() { + props.case_style + } else { + None + } + } + + pub fn generate(&mut self) -> syn::Result<()> { + let name = &self.ast.ident; + let discriminant_type = self.discriminant_type(); + + let case_style = self.case_style(); + let mut prev_const_var_ident = None; + let variants = match &self.ast.data { + Data::Enum(v) => &v.variants, + _ => return Err(non_enum_error()), + }; + self.enum_count = variants.len(); + for variant in variants { + let props = variant.get_variant_properties()?; + + if let Some(variant_names) = &mut self.gen_names { + variant_names.push(props.get_preferred_name(case_style)); + } + + if let Some(FromReprTokens { + match_arms, + constant_defs, + }) = &mut self.gen_from_repr + { + if props.disabled.is_some() { + continue; + } + + let ident = &variant.ident; + let (params, has_additional_data) = Self::params_from_fields(&variant.fields); + if has_additional_data { + self.has_additional_data = has_additional_data; + } + + let const_var_ident = { + use heck::ToShoutySnakeCase; + let const_var_str = format!("{}_DISCRIMINANT", ident).to_shouty_snake_case(); + format_ident!("{}", const_var_str) + }; + + let const_val_expr = match &variant.discriminant { + Some((_, expr)) => quote! { #expr }, + None => match &prev_const_var_ident { + Some(prev) => quote! { #prev + 1 }, + None => quote! { 0 }, + }, + }; + + constant_defs + .push(quote! {const #const_var_ident: #discriminant_type = #const_val_expr;}); + match_arms.push(quote! {v if v == #const_var_ident => ::core::option::Option::Some(#name::#ident #params)}); + prev_const_var_ident = Some(const_var_ident); + } + } + if let Some(FromReprTokens { match_arms, .. }) = &mut self.gen_from_repr { + match_arms.push(quote! { _ => ::core::option::Option::None }); + } + + Ok(()) + } + + pub fn variant_names(&self) -> &Option> { + &self.gen_names + } + + pub fn from_repr(&self) -> &Option { + &self.gen_from_repr + } + + pub fn enum_count(&self) -> usize { + self.enum_count + } +} diff --git a/strum_macros/src/helpers/mod.rs b/strum_macros/src/helpers/mod.rs index 30787b28..f13f1c66 100644 --- a/strum_macros/src/helpers/mod.rs +++ b/strum_macros/src/helpers/mod.rs @@ -4,6 +4,7 @@ pub use self::variant_props::HasStrumVariantProperties; pub mod case_style; mod metadata; +pub mod metadata_impl; pub mod type_props; pub mod variant_props; diff --git a/strum_macros/src/macros/enum_metadata.rs b/strum_macros/src/macros/enum_metadata.rs index d6c52405..8328298e 100644 --- a/strum_macros/src/macros/enum_metadata.rs +++ b/strum_macros/src/macros/enum_metadata.rs @@ -1,51 +1,13 @@ +use crate::helpers::metadata_impl::{FromReprTokens, MetadataImpl}; +use crate::helpers::non_enum_error; use proc_macro2::{Span, TokenStream}; -use quote::{format_ident, quote}; -use syn::{Data, DeriveInput, PathArguments, Type, TypeParen}; - -use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; +use quote::quote; +use syn::{Data, DeriveInput}; pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let gen = &ast.generics; let (impl_generics, ty_generics, where_clause) = gen.split_for_impl(); - let attrs = &ast.attrs; - - let mut discriminant_type: Type = syn::parse("usize".parse().unwrap()).unwrap(); - for attr in attrs { - let path = &attr.path; - let tokens = &attr.tokens; - if path.leading_colon.is_some() { - continue; - } - if path.segments.len() != 1 { - continue; - } - let segment = path.segments.first().unwrap(); - if segment.ident != "repr" { - continue; - } - if segment.arguments != PathArguments::None { - continue; - } - let typ_paren = match syn::parse2::(tokens.clone()) { - Ok(Type::Paren(TypeParen { elem, .. })) => *elem, - _ => continue, - }; - let inner_path = match &typ_paren { - Type::Path(t) => t, - _ => continue, - }; - if let Some(seg) = inner_path.path.segments.last() { - for t in &[ - "u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize", - ] { - if seg.ident == t { - discriminant_type = typ_paren; - break; - } - } - } - } if gen.lifetimes().count() > 0 { return Err(syn::Error::new( @@ -55,69 +17,21 @@ pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { )); } - let variants = match &ast.data { - Data::Enum(v) => &v.variants, + match &ast.data { + Data::Enum(_) => (), _ => return Err(non_enum_error()), }; - let mut arms = Vec::new(); - let mut constant_defs = Vec::new(); - let mut prev_const_var_ident = None; - for variant in variants { - use syn::Fields::*; - - if variant.get_variant_properties()?.disabled.is_some() { - continue; - } - - let ident = &variant.ident; - let params = match &variant.fields { - Unit => quote! {}, - Unnamed(fields) => { - let defaults = ::core::iter::repeat(quote!(::core::default::Default::default())) - .take(fields.unnamed.len()); - quote! { (#(#defaults),*) } - } - Named(fields) => { - let fields = fields - .named - .iter() - .map(|field| field.ident.as_ref().unwrap()); - quote! { {#(#fields: ::core::default::Default::default()),*} } - } - }; - - use heck::ToShoutySnakeCase; - let const_var_str = format!("{}_DISCRIMINANT", variant.ident).to_shouty_snake_case(); - let const_var_ident = format_ident!("{}", const_var_str); - - let const_val_expr = match &variant.discriminant { - Some((_, expr)) => quote! { #expr }, - None => match &prev_const_var_ident { - Some(prev) => quote! { #prev + 1 }, - None => quote! { 0 }, - }, - }; - - constant_defs.push(quote! {const #const_var_ident: #discriminant_type = #const_val_expr;}); - arms.push(quote! {v if v == #const_var_ident => ::core::option::Option::Some(#name::#ident #params)}); - - prev_const_var_ident = Some(const_var_ident); - } - - arms.push(quote! { _ => ::core::option::Option::None }); - - let type_properties = ast.get_type_properties()?; - - let variant_names = variants - .iter() - .map(|v| { - let props = v.get_variant_properties()?; - Ok(props.get_preferred_name(type_properties.case_style)) - }) - .collect::>>()?; + let mut metadata = MetadataImpl::new(ast).use_name_info().use_from_repr(); + let discriminant_type = metadata.discriminant_type(); + metadata.generate()?; + let enum_count = metadata.enum_count(); + let variant_names = metadata.variant_names().as_ref().unwrap(); - let enum_count = variants.len(); + let FromReprTokens { + constant_defs, + match_arms, + } = &metadata.from_repr().as_ref().unwrap(); Ok(quote! { impl #impl_generics EnumMetadata for #name #ty_generics #where_clause { @@ -141,7 +55,7 @@ pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { fn from_repr(discriminant: #discriminant_type) -> Option { #(#constant_defs)* match discriminant { - #(#arms),* + #(#match_arms),* } } } diff --git a/strum_macros/src/macros/from_repr.rs b/strum_macros/src/macros/from_repr.rs index efa81663..43ed5f5f 100644 --- a/strum_macros/src/macros/from_repr.rs +++ b/strum_macros/src/macros/from_repr.rs @@ -1,52 +1,14 @@ use proc_macro2::{Span, TokenStream}; -use quote::{format_ident, quote}; -use syn::{Data, DeriveInput, PathArguments, Type, TypeParen}; +use quote::quote; +use syn::DeriveInput; -use crate::helpers::{non_enum_error, HasStrumVariantProperties}; +use crate::helpers::metadata_impl::{FromReprTokens, MetadataImpl}; pub fn from_repr_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let gen = &ast.generics; let (impl_generics, ty_generics, where_clause) = gen.split_for_impl(); let vis = &ast.vis; - let attrs = &ast.attrs; - - let mut discriminant_type: Type = syn::parse("usize".parse().unwrap()).unwrap(); - for attr in attrs { - let path = &attr.path; - let tokens = &attr.tokens; - if path.leading_colon.is_some() { - continue; - } - if path.segments.len() != 1 { - continue; - } - let segment = path.segments.first().unwrap(); - if segment.ident != "repr" { - continue; - } - if segment.arguments != PathArguments::None { - continue; - } - let typ_paren = match syn::parse2::(tokens.clone()) { - Ok(Type::Paren(TypeParen { elem, .. })) => *elem, - _ => continue, - }; - let inner_path = match &typ_paren { - Type::Path(t) => t, - _ => continue, - }; - if let Some(seg) = inner_path.path.segments.last() { - for t in &[ - "u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize", - ] { - if seg.ident == t { - discriminant_type = typ_paren; - break; - } - } - } - } if gen.lifetimes().count() > 0 { return Err(syn::Error::new( @@ -56,62 +18,15 @@ pub fn from_repr_inner(ast: &DeriveInput) -> syn::Result { )); } - let variants = match &ast.data { - Data::Enum(v) => &v.variants, - _ => return Err(non_enum_error()), - }; - - let mut arms = Vec::new(); - let mut constant_defs = Vec::new(); - let mut has_additional_data = false; - let mut prev_const_var_ident = None; - for variant in variants { - use syn::Fields::*; - - if variant.get_variant_properties()?.disabled.is_some() { - continue; - } - - let ident = &variant.ident; - let params = match &variant.fields { - Unit => quote! {}, - Unnamed(fields) => { - has_additional_data = true; - let defaults = ::std::iter::repeat(quote!(::core::default::Default::default())) - .take(fields.unnamed.len()); - quote! { (#(#defaults),*) } - } - Named(fields) => { - has_additional_data = true; - let fields = fields - .named - .iter() - .map(|field| field.ident.as_ref().unwrap()); - quote! { {#(#fields: ::core::default::Default::default()),*} } - } - }; - - use heck::ToShoutySnakeCase; - let const_var_str = format!("{}_DISCRIMINANT", variant.ident).to_shouty_snake_case(); - let const_var_ident = format_ident!("{}", const_var_str); - - let const_val_expr = match &variant.discriminant { - Some((_, expr)) => quote! { #expr }, - None => match &prev_const_var_ident { - Some(prev) => quote! { #prev + 1 }, - None => quote! { 0 }, - }, - }; - - constant_defs.push(quote! {const #const_var_ident: #discriminant_type = #const_val_expr;}); - arms.push(quote! {v if v == #const_var_ident => ::core::option::Option::Some(#name::#ident #params)}); - - prev_const_var_ident = Some(const_var_ident); - } - - arms.push(quote! { _ => ::core::option::Option::None }); + let mut metadata = MetadataImpl::new(ast).use_from_repr(); + let discriminant_type = metadata.discriminant_type(); + metadata.generate()?; + let FromReprTokens { + constant_defs, + match_arms, + } = &metadata.from_repr().as_ref().unwrap(); - let const_if_possible = if has_additional_data { + let const_if_possible = if metadata.has_additional_data { quote! {} } else { #[rustversion::before(1.46)] @@ -126,17 +41,13 @@ pub fn from_repr_inner(ast: &DeriveInput) -> syn::Result { filter_by_rust_version(quote! { const }) }; - // Note: synchronize changes with `EnumMetadata::from_repr`, - // it duplicates this logic in an inherent impl. - // Making it possible to have both impls on the same type; - // so their behavior must be kept the same. Ok(quote! { impl #impl_generics #name #ty_generics #where_clause { #[doc = "Try to create [Self] from the raw representation"] #vis #const_if_possible fn from_repr(discriminant: #discriminant_type) -> Option<#name #ty_generics> { #(#constant_defs)* match discriminant { - #(#arms),* + #(#match_arms),* } } } diff --git a/strum_tests/tests/enum_metadata.rs b/strum_tests/tests/enum_metadata.rs index af1cbf83..49a2bf63 100644 --- a/strum_tests/tests/enum_metadata.rs +++ b/strum_tests/tests/enum_metadata.rs @@ -1,7 +1,6 @@ use strum::EnumMetadata; // To check that from_repr impls on both the enum type, // and EnumMetadata don't clash in any way. -use core::ops; use strum::FromRepr; #[derive(Debug, Eq, PartialEq, EnumMetadata, FromRepr)] From b6e78565638b78bbec3dff574649706f41d77f8d Mon Sep 17 00:00:00 2001 From: matt rice Date: Wed, 26 Jan 2022 10:23:08 -0800 Subject: [PATCH 4/6] add Eq, Ord, Display for EnumMetadata::Repr. --- strum/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/strum/src/lib.rs b/strum/src/lib.rs index a516b020..cfdb4100 100644 --- a/strum/src/lib.rs +++ b/strum/src/lib.rs @@ -206,6 +206,11 @@ pub trait EnumMetadata { + ops::BitXorAssign + ops::ShrAssign + ops::ShlAssign + + core::cmp::Eq + + core::cmp::Ord + + core::cmp::PartialEq + + core::cmp::PartialOrd + + core::fmt::Display + core::fmt::Debug; /// The enum type, generally Self unless deriving EnumMetadata for a type which returns From 73cc59fe24ac7f90772d6f9f5620c9e526cc4c74 Mon Sep 17 00:00:00 2001 From: matt rice Date: Thu, 27 Jan 2022 00:52:23 -0800 Subject: [PATCH 5/6] Move error handling into MetadataImpl::new This uses MetadataImpl in enum_count and enum_variant_names, in addition to enum_metadata, and from_repr from the previous commits. --- strum_macros/src/helpers/metadata_impl.rs | 52 ++++++++++++++----- strum_macros/src/macros/enum_count.rs | 10 ++-- strum_macros/src/macros/enum_metadata.rs | 24 ++------- strum_macros/src/macros/enum_variant_names.rs | 27 +++------- strum_macros/src/macros/from_repr.rs | 14 ++--- 5 files changed, 59 insertions(+), 68 deletions(-) diff --git a/strum_macros/src/helpers/metadata_impl.rs b/strum_macros/src/helpers/metadata_impl.rs index 415d1cd6..c09f78e7 100644 --- a/strum_macros/src/helpers/metadata_impl.rs +++ b/strum_macros/src/helpers/metadata_impl.rs @@ -1,13 +1,17 @@ -use proc_macro2::TokenStream; +use proc_macro2::{Span, TokenStream}; use quote::{format_ident, quote}; -use syn::{Data, DeriveInput, PathArguments, Type, TypeParen}; +use syn::{ + Data, DeriveInput, ImplGenerics, PathArguments, Type, TypeGenerics, TypeParen, WhereClause, +}; use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; pub struct MetadataImpl<'a> { ast: &'a syn::DeriveInput, + variants: &'a syn::punctuated::Punctuated, gen_names: Option>, gen_from_repr: Option, + generics_split: (ImplGenerics<'a>, TypeGenerics<'a>, Option<&'a WhereClause>), pub enum_count: usize, pub has_additional_data: bool, } @@ -18,14 +22,38 @@ pub struct FromReprTokens { } impl<'a> MetadataImpl<'a> { - pub fn new(ast: &'a DeriveInput) -> Self { - MetadataImpl { + pub fn new(ast: &'a DeriveInput) -> syn::Result { + let gen = &ast.generics; + let generics_split = gen.split_for_impl(); + + if gen.lifetimes().count() > 0 { + return Err(syn::Error::new( + Span::call_site(), + "This macro doesn't support enums with lifetimes. \ + The resulting enums would be unbounded.", + )); + } + + match &ast.data { + Data::Enum(_) => (), + _ => return Err(non_enum_error()), + }; + + let variants = match &ast.data { + Data::Enum(v) => &v.variants, + _ => return Err(non_enum_error()), + }; + let enum_count = variants.len(); + + Ok(MetadataImpl { ast, - enum_count: 0, + enum_count, gen_names: None, gen_from_repr: None, + generics_split, + variants, has_additional_data: false, - } + }) } pub fn use_name_info(mut self) -> Self { @@ -117,12 +145,8 @@ impl<'a> MetadataImpl<'a> { let case_style = self.case_style(); let mut prev_const_var_ident = None; - let variants = match &self.ast.data { - Data::Enum(v) => &v.variants, - _ => return Err(non_enum_error()), - }; - self.enum_count = variants.len(); - for variant in variants { + + for variant in self.variants { let props = variant.get_variant_properties()?; if let Some(variant_names) = &mut self.gen_names { @@ -182,4 +206,8 @@ impl<'a> MetadataImpl<'a> { pub fn enum_count(&self) -> usize { self.enum_count } + + pub fn generics_split(&self) -> &(ImplGenerics<'a>, TypeGenerics<'a>, Option<&'a WhereClause>) { + &self.generics_split + } } diff --git a/strum_macros/src/macros/enum_count.rs b/strum_macros/src/macros/enum_count.rs index 44c7f2e5..553f0924 100644 --- a/strum_macros/src/macros/enum_count.rs +++ b/strum_macros/src/macros/enum_count.rs @@ -1,14 +1,12 @@ +use crate::helpers::metadata_impl::MetadataImpl; use proc_macro2::TokenStream; use quote::quote; -use syn::{Data, DeriveInput}; +use syn::DeriveInput; -use crate::helpers::{non_enum_error, HasTypeProperties}; +use crate::helpers::HasTypeProperties; pub(crate) fn enum_count_inner(ast: &DeriveInput) -> syn::Result { - let n = match &ast.data { - Data::Enum(v) => v.variants.len(), - _ => return Err(non_enum_error()), - }; + let n = MetadataImpl::new(ast)?.enum_count; let type_properties = ast.get_type_properties()?; let strum_module_path = type_properties.crate_module_path(); diff --git a/strum_macros/src/macros/enum_metadata.rs b/strum_macros/src/macros/enum_metadata.rs index 8328298e..087cefab 100644 --- a/strum_macros/src/macros/enum_metadata.rs +++ b/strum_macros/src/macros/enum_metadata.rs @@ -1,32 +1,16 @@ use crate::helpers::metadata_impl::{FromReprTokens, MetadataImpl}; -use crate::helpers::non_enum_error; -use proc_macro2::{Span, TokenStream}; +use proc_macro2::TokenStream; use quote::quote; -use syn::{Data, DeriveInput}; +use syn::DeriveInput; pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; - let gen = &ast.generics; - let (impl_generics, ty_generics, where_clause) = gen.split_for_impl(); - - if gen.lifetimes().count() > 0 { - return Err(syn::Error::new( - Span::call_site(), - "This macro doesn't support enums with lifetimes. \ - The resulting enums would be unbounded.", - )); - } - - match &ast.data { - Data::Enum(_) => (), - _ => return Err(non_enum_error()), - }; - - let mut metadata = MetadataImpl::new(ast).use_name_info().use_from_repr(); + let mut metadata = MetadataImpl::new(ast)?.use_name_info().use_from_repr(); let discriminant_type = metadata.discriminant_type(); metadata.generate()?; let enum_count = metadata.enum_count(); let variant_names = metadata.variant_names().as_ref().unwrap(); + let (impl_generics, ty_generics, where_clause) = &metadata.generics_split(); let FromReprTokens { constant_defs, diff --git a/strum_macros/src/macros/enum_variant_names.rs b/strum_macros/src/macros/enum_variant_names.rs index c54d45dc..1d3b5b95 100644 --- a/strum_macros/src/macros/enum_variant_names.rs +++ b/strum_macros/src/macros/enum_variant_names.rs @@ -1,34 +1,23 @@ +use crate::helpers::metadata_impl::MetadataImpl; +use crate::helpers::HasTypeProperties; use proc_macro2::TokenStream; use quote::quote; -use syn::{Data, DeriveInput}; - -use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; +use syn::DeriveInput; pub fn enum_variant_names_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; - let gen = &ast.generics; - let (impl_generics, ty_generics, where_clause) = gen.split_for_impl(); - - let variants = match &ast.data { - Data::Enum(v) => &v.variants, - _ => return Err(non_enum_error()), - }; + let mut metadata = MetadataImpl::new(ast)?.use_name_info(); + metadata.generate()?; + let variant_names = metadata.variant_names().as_ref().unwrap(); + let (impl_generics, ty_generics, where_clause) = &metadata.generics_split(); // Derives for the generated enum let type_properties = ast.get_type_properties()?; let strum_module_path = type_properties.crate_module_path(); - let names = variants - .iter() - .map(|v| { - let props = v.get_variant_properties()?; - Ok(props.get_preferred_name(type_properties.case_style)) - }) - .collect::>>()?; - Ok(quote! { impl #impl_generics #strum_module_path::VariantNames for #name #ty_generics #where_clause { - const VARIANTS: &'static [&'static str] = &[ #(#names),* ]; + const VARIANTS: &'static [&'static str] = &[ #(#variant_names),* ]; } }) } diff --git a/strum_macros/src/macros/from_repr.rs b/strum_macros/src/macros/from_repr.rs index 43ed5f5f..9eff5895 100644 --- a/strum_macros/src/macros/from_repr.rs +++ b/strum_macros/src/macros/from_repr.rs @@ -1,4 +1,4 @@ -use proc_macro2::{Span, TokenStream}; +use proc_macro2::TokenStream; use quote::quote; use syn::DeriveInput; @@ -6,25 +6,17 @@ use crate::helpers::metadata_impl::{FromReprTokens, MetadataImpl}; pub fn from_repr_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; - let gen = &ast.generics; - let (impl_generics, ty_generics, where_clause) = gen.split_for_impl(); let vis = &ast.vis; - if gen.lifetimes().count() > 0 { - return Err(syn::Error::new( - Span::call_site(), - "This macro doesn't support enums with lifetimes. \ - The resulting enums would be unbounded.", - )); - } + let mut metadata = MetadataImpl::new(ast)?.use_from_repr(); - let mut metadata = MetadataImpl::new(ast).use_from_repr(); let discriminant_type = metadata.discriminant_type(); metadata.generate()?; let FromReprTokens { constant_defs, match_arms, } = &metadata.from_repr().as_ref().unwrap(); + let (impl_generics, ty_generics, where_clause) = metadata.generics_split(); let const_if_possible = if metadata.has_additional_data { quote! {} From d3022da4d0f62dbeca7caa82e16745d57d3ba1ee Mon Sep 17 00:00:00 2001 From: matt rice Date: Thu, 27 Jan 2022 01:26:36 -0800 Subject: [PATCH 6/6] Fix and add tests for macros that allow lifetimes. --- strum_macros/src/helpers/metadata_impl.rs | 21 +++++++++++---------- strum_macros/src/macros/enum_metadata.rs | 2 +- strum_macros/src/macros/from_repr.rs | 2 +- strum_tests/tests/enum_count.rs | 12 ++++++++++++ strum_tests/tests/enum_variant_names.rs | 12 ++++++++++++ 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/strum_macros/src/helpers/metadata_impl.rs b/strum_macros/src/helpers/metadata_impl.rs index c09f78e7..c3f80e7a 100644 --- a/strum_macros/src/helpers/metadata_impl.rs +++ b/strum_macros/src/helpers/metadata_impl.rs @@ -26,14 +26,6 @@ impl<'a> MetadataImpl<'a> { let gen = &ast.generics; let generics_split = gen.split_for_impl(); - if gen.lifetimes().count() > 0 { - return Err(syn::Error::new( - Span::call_site(), - "This macro doesn't support enums with lifetimes. \ - The resulting enums would be unbounded.", - )); - } - match &ast.data { Data::Enum(_) => (), _ => return Err(non_enum_error()), @@ -61,12 +53,21 @@ impl<'a> MetadataImpl<'a> { self } - pub fn use_from_repr(mut self) -> Self { + pub fn use_from_repr(mut self) -> syn::Result { + let gen = &self.ast.generics; + if gen.lifetimes().count() > 0 { + return Err(syn::Error::new( + Span::call_site(), + "This macro doesn't support enums with lifetimes. \ + The resulting enums would be unbounded.", + )); + } + self.gen_from_repr = Some(FromReprTokens { constant_defs: Vec::new(), match_arms: Vec::new(), }); - self + Ok(self) } pub fn discriminant_type(&self) -> Type { diff --git a/strum_macros/src/macros/enum_metadata.rs b/strum_macros/src/macros/enum_metadata.rs index 087cefab..21af031a 100644 --- a/strum_macros/src/macros/enum_metadata.rs +++ b/strum_macros/src/macros/enum_metadata.rs @@ -5,7 +5,7 @@ use syn::DeriveInput; pub fn enum_metadata_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; - let mut metadata = MetadataImpl::new(ast)?.use_name_info().use_from_repr(); + let mut metadata = MetadataImpl::new(ast)?.use_name_info().use_from_repr()?; let discriminant_type = metadata.discriminant_type(); metadata.generate()?; let enum_count = metadata.enum_count(); diff --git a/strum_macros/src/macros/from_repr.rs b/strum_macros/src/macros/from_repr.rs index 9eff5895..51de9a55 100644 --- a/strum_macros/src/macros/from_repr.rs +++ b/strum_macros/src/macros/from_repr.rs @@ -8,7 +8,7 @@ pub fn from_repr_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let vis = &ast.vis; - let mut metadata = MetadataImpl::new(ast)?.use_from_repr(); + let mut metadata = MetadataImpl::new(ast)?.use_from_repr()?; let discriminant_type = metadata.discriminant_type(); metadata.generate()?; diff --git a/strum_tests/tests/enum_count.rs b/strum_tests/tests/enum_count.rs index 794e5638..b2945bd5 100644 --- a/strum_tests/tests/enum_count.rs +++ b/strum_tests/tests/enum_count.rs @@ -40,3 +40,15 @@ fn crate_module_path_test() { assert_eq!(7, Week::COUNT); assert_eq!(Week::iter().count(), Week::COUNT); } + +// EnumIter doesn't support lifetimes so we can't check consistency with that. +#[derive(Debug, EnumCount)] +enum HasLifetime<'a> { + Hello(&'a str), +} + +#[test] +fn lifetime_test() { + let _ = HasLifetime::Hello("world"); + assert_eq!(1, HasLifetime::COUNT); +} diff --git a/strum_tests/tests/enum_variant_names.rs b/strum_tests/tests/enum_variant_names.rs index 54ee857c..669ddfa1 100644 --- a/strum_tests/tests/enum_variant_names.rs +++ b/strum_tests/tests/enum_variant_names.rs @@ -126,3 +126,15 @@ fn crate_module_path_test() { assert_eq!(Color::VARIANTS, &["Red", "b", "y"]); } + +#[derive(Debug, EnumVariantNames)] +enum HasLifetime<'a> { + #[strum(serialize = "hello")] + Hello(&'a str), +} + +#[test] +fn has_lifetime_variant_name() { + let _ = HasLifetime::Hello("world"); + assert_eq!(HasLifetime::VARIANTS, &["hello"]); +}