diff --git a/README.md b/README.md index 9b2f645..5442ec3 100644 --- a/README.md +++ b/README.md @@ -224,14 +224,34 @@ fn main() { As this is naturally exhaustive, this is only supported for `FromPrimitive`, not also `TryFromPrimitive`. -Unsafely turning a primitive into an enum with from_unchecked -------------------------------------------------------------- +Unsafely turning a primitive into an enum with unchecked_transmute_from +----------------------------------------------------------------------- If you're really certain a conversion will succeed (and have not made use of `#[num_enum(default)]` or `#[num_enum(alternatives = [..])]` for any of its variants), and want to avoid a small amount of overhead, you can use unsafe code to do this conversion. Unless you have data showing that the match statement generated in the `try_from` above is a bottleneck for you, you should avoid doing this, as the unsafe code has potential to cause serious memory issues in your program. +Note that this derive ignores any `default`, `catch_all`, and `alternatives` attributes on the enum. +If you need support for conversions from these values, you should use `TryFromPrimitive` or `FromPrimitive`. + + - This means, for instance, that the following is UB: + + ```rust,no_run + use num_enum::UnsafeFromPrimitive; + + #[derive(UnsafeFromPrimitive)] + #[repr(u8)] + enum Number { + Zero = 0, + + // Same for `#[num_enum(catch_all)]`, and `#[num_enum(alternatives = [2, ...])]` + #[num_enum(default)] + One = 1, + } + let _undefined_behavior = unsafe { Number::unchecked_transmute_from(2) }; + ``` + ```rust use num_enum::UnsafeFromPrimitive; @@ -244,17 +264,17 @@ enum Number { fn main() { assert_eq!( - unsafe { Number::from_unchecked(0_u8) }, + unsafe { Number::unchecked_transmute_from(0_u8) }, Number::Zero, ); assert_eq!( - unsafe { Number::from_unchecked(1_u8) }, + unsafe { Number::unchecked_transmute_from(1_u8) }, Number::One, ); } unsafe fn undefined_behavior() { - let _ = Number::from_unchecked(2); // 2 is not a valid discriminant! + let _ = Number::unchecked_transmute_from(2); // 2 is not a valid discriminant! } ``` diff --git a/num_enum/src/lib.rs b/num_enum/src/lib.rs index bccb08e..9d4161e 100644 --- a/num_enum/src/lib.rs +++ b/num_enum/src/lib.rs @@ -35,7 +35,20 @@ pub trait UnsafeFromPrimitive: Sized { /// # Safety /// /// - `number` must represent a valid discriminant of `Self`. - unsafe fn from_unchecked(number: Self::Primitive) -> Self; + #[deprecated( + since = "0.6.0", + note = "Prefer to use `unchecked_transmute_from`, `from_unchecked` will be removed in a future release." + )] + unsafe fn from_unchecked(number: Self::Primitive) -> Self { + Self::unchecked_transmute_from(number) + } + + /// Transmutes into an enum from its primitive. + /// + /// # Safety + /// + /// - `number` must represent a valid discriminant of `Self`. + unsafe fn unchecked_transmute_from(number: Self::Primitive) -> Self; } pub struct TryFromPrimitiveError { diff --git a/num_enum/tests/try_build/compile_fail/from_unchecked_deprecated_warning.rs b/num_enum/tests/try_build/compile_fail/from_unchecked_deprecated_warning.rs new file mode 100644 index 0000000..6e65827 --- /dev/null +++ b/num_enum/tests/try_build/compile_fail/from_unchecked_deprecated_warning.rs @@ -0,0 +1,17 @@ +#![deny(deprecated)] + +use num_enum::UnsafeFromPrimitive; + +#[derive(Debug, Eq, PartialEq, UnsafeFromPrimitive)] +#[repr(u8)] +enum Enum { + Zero, + One, +} + +fn main() { + unsafe { + assert_eq!(Enum::from_unchecked(0_u8), Enum::Zero); + assert_eq!(Enum::from_unchecked(1_u8), Enum::One); + } +} diff --git a/num_enum/tests/try_build/compile_fail/from_unchecked_deprecated_warning.stderr b/num_enum/tests/try_build/compile_fail/from_unchecked_deprecated_warning.stderr new file mode 100644 index 0000000..0c450ed --- /dev/null +++ b/num_enum/tests/try_build/compile_fail/from_unchecked_deprecated_warning.stderr @@ -0,0 +1,17 @@ +error: use of deprecated associated function `num_enum::UnsafeFromPrimitive::from_unchecked`: Prefer to use `unchecked_transmute_from`, `from_unchecked` will be removed in a future release. + --> tests/try_build/compile_fail/from_unchecked_deprecated_warning.rs:14:26 + | +14 | assert_eq!(Enum::from_unchecked(0_u8), Enum::Zero); + | ^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> tests/try_build/compile_fail/from_unchecked_deprecated_warning.rs:1:9 + | +1 | #![deny(deprecated)] + | ^^^^^^^^^^ + +error: use of deprecated associated function `num_enum::UnsafeFromPrimitive::from_unchecked`: Prefer to use `unchecked_transmute_from`, `from_unchecked` will be removed in a future release. + --> tests/try_build/compile_fail/from_unchecked_deprecated_warning.rs:15:26 + | +15 | assert_eq!(Enum::from_unchecked(1_u8), Enum::One); + | ^^^^^^^^^^^^^^ diff --git a/num_enum/tests/try_build/compile_fail/unpexpected_alternatives.rs b/num_enum/tests/try_build/compile_fail/unpexpected_alternatives.rs deleted file mode 100644 index 4dbdabb..0000000 --- a/num_enum/tests/try_build/compile_fail/unpexpected_alternatives.rs +++ /dev/null @@ -1,11 +0,0 @@ -#[derive(num_enum::UnsafeFromPrimitive)] -#[repr(u8)] -enum Numbers { - Zero, - #[num_enum(alternatives = [2])] - One, -} - -fn main() { - -} diff --git a/num_enum/tests/try_build/compile_fail/unpexpected_alternatives.stderr b/num_enum/tests/try_build/compile_fail/unpexpected_alternatives.stderr deleted file mode 100644 index 7aac45c..0000000 --- a/num_enum/tests/try_build/compile_fail/unpexpected_alternatives.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: #[derive(UnsafeFromPrimitive)] does not support `#[num_enum(alternatives = [..])]` - --> $DIR/unpexpected_alternatives.rs:5:16 - | -5 | #[num_enum(alternatives = [2])] - | ^^^^^^^^^^^^ diff --git a/num_enum/tests/try_build/compile_fail/unpexpected_default.rs b/num_enum/tests/try_build/compile_fail/unpexpected_default.rs deleted file mode 100644 index ffb1a0e..0000000 --- a/num_enum/tests/try_build/compile_fail/unpexpected_default.rs +++ /dev/null @@ -1,11 +0,0 @@ -#[derive(num_enum::UnsafeFromPrimitive)] -#[repr(u8)] -enum Numbers { - Zero, - #[num_enum(default)] - NoneZero, -} - -fn main() { - -} diff --git a/num_enum/tests/try_build/compile_fail/unpexpected_default.stderr b/num_enum/tests/try_build/compile_fail/unpexpected_default.stderr deleted file mode 100644 index c4127cb..0000000 --- a/num_enum/tests/try_build/compile_fail/unpexpected_default.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: #[derive(UnsafeFromPrimitive)] does not support `#[num_enum(default)]` - --> $DIR/unpexpected_default.rs:5:16 - | -5 | #[num_enum(default)] - | ^^^^^^^ diff --git a/num_enum/tests/unsafe_from_primitive.rs b/num_enum/tests/unsafe_from_primitive.rs index 79fb582..e9c72f0 100644 --- a/num_enum/tests/unsafe_from_primitive.rs +++ b/num_enum/tests/unsafe_from_primitive.rs @@ -15,8 +15,68 @@ fn has_unsafe_from_primitive_number() { One, } + unsafe { + assert_eq!(Enum::unchecked_transmute_from(0_u8), Enum::Zero); + assert_eq!(Enum::unchecked_transmute_from(1_u8), Enum::One); + } +} + +#[test] +fn has_unsafe_from_primitive_number_with_alternatives_and_default_which_are_ignored() { + #[derive(Debug, Eq, PartialEq, UnsafeFromPrimitive)] + #[repr(u8)] + enum Enum { + Zero, + One, + #[num_enum(alternatives = [3, 4])] + Some, + #[num_enum(default)] + Many = 5, + } + + unsafe { + assert_eq!(Enum::unchecked_transmute_from(0_u8), Enum::Zero); + assert_eq!(Enum::unchecked_transmute_from(1_u8), Enum::One); + assert_eq!(Enum::unchecked_transmute_from(2_u8), Enum::Some); + assert_eq!(Enum::unchecked_transmute_from(5_u8), Enum::Many); + // Any other conversions would be undefined behavior. + } + + #[allow(deprecated)] + unsafe { + assert_eq!(Enum::from_unchecked(0_u8), Enum::Zero); + assert_eq!(Enum::from_unchecked(1_u8), Enum::One); + assert_eq!(Enum::from_unchecked(2_u8), Enum::Some); + assert_eq!(Enum::from_unchecked(5_u8), Enum::Many); + } +} + +#[test] +fn has_unsafe_from_primitive_number_with_alternatives_and_std_default_which_are_ignored() { + #[derive(Debug, Default, Eq, PartialEq, UnsafeFromPrimitive)] + #[repr(u8)] + enum Enum { + Zero, + One, + #[num_enum(alternatives = [3, 4])] + Some, + #[default] + Many = 5, + } + + unsafe { + assert_eq!(Enum::unchecked_transmute_from(0_u8), Enum::Zero); + assert_eq!(Enum::unchecked_transmute_from(1_u8), Enum::One); + assert_eq!(Enum::unchecked_transmute_from(2_u8), Enum::Some); + assert_eq!(Enum::unchecked_transmute_from(5_u8), Enum::Many); + // Any other conversions would be undefined behavior. + } + + #[allow(deprecated)] unsafe { assert_eq!(Enum::from_unchecked(0_u8), Enum::Zero); assert_eq!(Enum::from_unchecked(1_u8), Enum::One); + assert_eq!(Enum::from_unchecked(2_u8), Enum::Some); + assert_eq!(Enum::from_unchecked(5_u8), Enum::Many); } } diff --git a/num_enum_derive/src/lib.rs b/num_enum_derive/src/lib.rs index 5773bd7..c4e22fa 100644 --- a/num_enum_derive/src/lib.rs +++ b/num_enum_derive/src/lib.rs @@ -227,16 +227,8 @@ impl Spanned for VariantAlternativesAttribute { } } -#[derive(::core::default::Default)] -struct AttributeSpans { - default: Vec, - catch_all: Vec, - alternatives: Vec, -} - struct VariantInfo { ident: Ident, - attr_spans: AttributeSpans, is_default: bool, is_catch_all: bool, canonical_value: Expr, @@ -247,10 +239,6 @@ impl VariantInfo { fn all_values(&self) -> impl Iterator { ::core::iter::once(&self.canonical_value).chain(self.alternative_values.iter()) } - - fn is_complex(&self) -> bool { - !self.alternative_values.is_empty() - } } struct EnumInfo { @@ -284,14 +272,6 @@ impl EnumInfo { die!(self.repr.clone() => "Failed to parse repr into bit size"); } - fn has_default_variant(&self) -> bool { - self.default().is_some() - } - - fn has_complex_variant(&self) -> bool { - self.variants.iter().any(|info| info.is_complex()) - } - fn default(&self) -> Option<&Ident> { self.variants .iter() @@ -306,18 +286,6 @@ impl EnumInfo { .map(|info| &info.ident) } - fn first_default_attr_span(&self) -> Option<&Span> { - self.variants - .iter() - .find_map(|info| info.attr_spans.default.first()) - } - - fn first_alternatives_attr_span(&self) -> Option<&Span> { - self.variants - .iter() - .find_map(|info| info.attr_spans.alternatives.first()) - } - fn variant_idents(&self) -> Vec { self.variants .iter() @@ -406,7 +374,6 @@ impl Parse for EnumInfo { None => next_discriminant.clone(), }; - let mut attr_spans: AttributeSpans = Default::default(); let mut raw_alternative_values: Vec = vec![]; // Keep the attribute around for better error reporting. let mut alt_attr_ref: Vec<&Attribute> = vec![]; @@ -428,7 +395,6 @@ impl Parse for EnumInfo { "Attribute `default` is mutually exclusive with `catch_all`" ); } - attr_spans.default.push(attribute.span()); is_default = true; has_default_variant = true; } @@ -448,7 +414,6 @@ impl Parse for EnumInfo { "Attribute `default` is mutually exclusive with `catch_all`" ); } - attr_spans.default.push(default.span()); is_default = true; has_default_variant = true; } @@ -473,7 +438,6 @@ impl Parse for EnumInfo { ty: syn::Type::Path(syn::TypePath { path, .. }), .. }] if path.is_ident(&repr) => { - attr_spans.catch_all.push(catch_all.span()); is_catch_all = true; has_catch_all_variant = true; } @@ -485,7 +449,6 @@ impl Parse for EnumInfo { } } NumEnumVariantAttributeItem::Alternatives(alternatives) => { - attr_spans.alternatives.push(alternatives.span()); raw_alternative_values.extend(alternatives.expressions); alt_attr_ref.push(attribute); } @@ -615,7 +578,6 @@ impl Parse for EnumInfo { variants.push(VariantInfo { ident, - attr_spans, is_default, is_catch_all, canonical_value: discriminant, @@ -905,17 +867,20 @@ fn get_crate_name() -> String { String::from("num_enum") } -/// Generates a `unsafe fn from_unchecked (number: Primitive) -> Self` +/// Generates a `unsafe fn unchecked_transmute_from(number: Primitive) -> Self` /// associated function. /// -/// Allows unsafely turning a primitive into an enum with from_unchecked. -/// ------------------------------------------------------------- +/// Allows unsafely turning a primitive into an enum with unchecked_transmute_from +/// ------------------------------------------------------------------------------ /// /// If you're really certain a conversion will succeed, and want to avoid a small amount of overhead, you can use unsafe /// code to do this conversion. Unless you have data showing that the match statement generated in the `try_from` above is a /// bottleneck for you, you should avoid doing this, as the unsafe code has potential to cause serious memory issues in /// your program. /// +/// Note that this derive ignores any `default`, `catch_all`, and `alternatives` attributes on the enum. +/// If you need support for conversions from these values, you should use `TryFromPrimitive` or `FromPrimitive`. +/// /// ```rust /// use num_enum::UnsafeFromPrimitive; /// @@ -929,16 +894,16 @@ fn get_crate_name() -> String { /// fn main() { /// assert_eq!( /// Number::Zero, -/// unsafe { Number::from_unchecked(0_u8) }, +/// unsafe { Number::unchecked_transmute_from(0_u8) }, /// ); /// assert_eq!( /// Number::One, -/// unsafe { Number::from_unchecked(1_u8) }, +/// unsafe { Number::unchecked_transmute_from(1_u8) }, /// ); /// } /// /// unsafe fn undefined_behavior() { -/// let _ = Number::from_unchecked(2); // 2 is not a valid discriminant! +/// let _ = Number::unchecked_transmute_from(2); // 2 is not a valid discriminant! /// } /// ``` #[proc_macro_derive(UnsafeFromPrimitive, attributes(num_enum))] @@ -946,25 +911,6 @@ pub fn derive_unsafe_from_primitive(stream: TokenStream) -> TokenStream { let enum_info = parse_macro_input!(stream as EnumInfo); let krate = Ident::new(&get_crate_name(), Span::call_site()); - if enum_info.has_default_variant() { - let span = enum_info - .first_default_attr_span() - .cloned() - .expect("Expected span"); - let message = "#[derive(UnsafeFromPrimitive)] does not support `#[num_enum(default)]`"; - return syn::Error::new(span, message).to_compile_error().into(); - } - - if enum_info.has_complex_variant() { - let span = enum_info - .first_alternatives_attr_span() - .cloned() - .expect("Expected span"); - let message = - "#[derive(UnsafeFromPrimitive)] does not support `#[num_enum(alternatives = [..])]`"; - return syn::Error::new(span, message).to_compile_error().into(); - } - let EnumInfo { ref name, ref repr, .. } = enum_info; @@ -973,7 +919,7 @@ pub fn derive_unsafe_from_primitive(stream: TokenStream) -> TokenStream { impl ::#krate::UnsafeFromPrimitive for #name { type Primitive = #repr; - unsafe fn from_unchecked(number: Self::Primitive) -> Self { + unsafe fn unchecked_transmute_from(number: Self::Primitive) -> Self { ::core::mem::transmute(number) } } diff --git a/renamed_num_enum/src/lib.rs b/renamed_num_enum/src/lib.rs index 43bb4e0..d751342 100644 --- a/renamed_num_enum/src/lib.rs +++ b/renamed_num_enum/src/lib.rs @@ -48,8 +48,8 @@ mod submodule { "No discriminant in enum `Number` matches the value `2`", ); - assert_eq!(unsafe { Number::from_unchecked(0) }, Number::Zero); + assert_eq!(unsafe { Number::unchecked_transmute_from(0) }, Number::Zero); - assert_eq!(unsafe { Number::from_unchecked(1) }, Number::One); + assert_eq!(unsafe { Number::unchecked_transmute_from(1) }, Number::One); } }