Skip to content

Commit

Permalink
TryFromPrimitive ignores default attributes
Browse files Browse the repository at this point in the history
If exhaustive behaviour is desired, FromPrimitive should be derived
instead.

Because these traits are exclusive, there's no room for ambiguity here -
either TryFromPrimitive is derived and defaults are ignored, or
FromPrimitive is derived and defaults are paid attention to.

This is, however, a breaking change, as it changes the implementation of
TryFromPrimitive significantly.

Fixes #75
Fixes #31
  • Loading branch information
illicitonion committed Mar 18, 2023
1 parent b32c406 commit 0e2d006
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 55 deletions.
20 changes: 11 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@ Default variant

Sometimes it is desirable to have an `Other` variant in an enum that acts as a kind of a wildcard matching all the value not yet covered by other variants.

The `#[num_enum(default)]` attribute allows you to mark variant as the default.
The `#[num_enum(default)]` attribute (or the stdlib `#[default]` attribute) allows you to mark variant as the default.

(The behavior of `IntoPrimitive` is unaffected by this attribute, it will always return the canonical value.)

```rust
use num_enum::TryFromPrimitive;
use num_enum::FromPrimitive;
use std::convert::TryFrom;

#[derive(Debug, Eq, PartialEq, TryFromPrimitive)]
#[derive(Debug, Eq, PartialEq, FromPrimitive)]
#[repr(u8)]
enum Number {
Zero = 0,
Expand All @@ -144,17 +144,19 @@ enum Number {
}

fn main() {
let zero = Number::try_from(0u8);
assert_eq!(zero, Ok(Number::Zero));
let zero = Number::from(0u8);
assert_eq!(zero, Number::Zero);

let one = Number::try_from(1u8);
assert_eq!(one, Ok(Number::NonZero));
let one = Number::from(1u8);
assert_eq!(one, Number::NonZero);

let two = Number::try_from(2u8);
assert_eq!(two, Ok(Number::NonZero));
let two = Number::from(2u8);
assert_eq!(two, Number::NonZero);
}
```

Only `FromPrimitive` pays attention to `default` attributes, `TryFromPrimitive` ignores them.

Safely turning a primitive into an exhaustive enum with from_primitive
-------------------------------------------------------------

Expand Down
10 changes: 10 additions & 0 deletions num_enum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,13 @@ impl<Enum: TryFromPrimitive> fmt::Display for TryFromPrimitiveError<Enum> {

#[cfg(feature = "std")]
impl<Enum: TryFromPrimitive> ::std::error::Error for TryFromPrimitiveError<Enum> {}

// This trait exists to try to give a more clear error message when someone attempts to derive both FromPrimitive and TryFromPrimitive.
// This isn't allowed because both end up creating a `TryFrom<primitive>` implementation.
// TryFromPrimitive explicitly implements TryFrom<primitive> with Error=TryFromPrimitiveError, which conflicts with:
// FromPrimitive explicitly implements From<primitive> which has a blanket implementation of TryFrom<primitive> with Error=Infallible.
//
// This is a private implementation detail of the num_enum crate which should not be depended on externally.
// It is subject to change in any release regardless of semver.
#[doc(hidden)]
pub trait CannotDeriveBothFromPrimitiveAndTryFromPrimitive {}
12 changes: 5 additions & 7 deletions num_enum/tests/from_primitive.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use ::std::convert::TryFrom;

use ::num_enum::{FromPrimitive, TryFromPrimitive};
use ::num_enum::FromPrimitive;

// Guard against https://github.com/illicitonion/num_enum/issues/27
mod alloc {}
Expand Down Expand Up @@ -89,11 +87,11 @@ fn from_primitive_number() {
let from = Enum::from(0_u8);
assert_eq!(from, Enum::Whatever);

let try_from_primitive = Enum::try_from_primitive(0_u8);
assert_eq!(try_from_primitive, Ok(Enum::Whatever));
let from_primitive = Enum::from_primitive(1_u8);
assert_eq!(from_primitive, Enum::Whatever);

let try_from = Enum::try_from(0_u8);
assert_eq!(try_from, Ok(Enum::Whatever));
let from = Enum::from(1_u8);
assert_eq!(from, Enum::Whatever);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0119]: conflicting implementations of trait `TryFromPrimitive` for type `Numbers`
error[E0119]: conflicting implementations of trait `CannotDeriveBothFromPrimitiveAndTryFromPrimitive` for type `Numbers`
--> tests/try_build/compile_fail/conflicting_derive.rs:1:35
|
1 | #[derive(num_enum::FromPrimitive, num_enum::TryFromPrimitive)]
Expand Down
10 changes: 8 additions & 2 deletions num_enum/tests/try_from_primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,10 @@ fn default_value() {
assert_eq!(two, Ok(Enum::Other));

let max_value: Result<Enum, _> = u8::max_value().try_into();
assert_eq!(max_value, Ok(Enum::Other));
assert_eq!(
max_value.unwrap_err().to_string(),
"No discriminant in enum `Enum` matches the value `255`"
);
}

#[test]
Expand Down Expand Up @@ -472,7 +475,10 @@ fn alternative_values_and_default_value() {
assert_eq!(four, Ok(Enum::Four));

let five: Result<Enum, _> = 5u8.try_into();
assert_eq!(five, Ok(Enum::Zero));
assert_eq!(
five.unwrap_err().to_string(),
"No discriminant in enum `Enum` matches the value `5`"
);
}

#[test]
Expand Down
44 changes: 8 additions & 36 deletions num_enum_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,24 +785,8 @@ pub fn derive_from_primitive(input: TokenStream) -> TokenStream {
}
}

// The Rust stdlib will implement `#name: From<#repr>` for us for free!

impl ::#krate::TryFromPrimitive for #name {
type Primitive = #repr;

const NAME: &'static str = stringify!(#name);

#[inline]
fn try_from_primitive (
number: Self::Primitive,
) -> ::core::result::Result<
Self,
::#krate::TryFromPrimitiveError<Self>,
>
{
Ok(::#krate::FromPrimitive::from_primitive(number))
}
}
#[doc(hidden)]
impl ::#krate::CannotDeriveBothFromPrimitiveAndTryFromPrimitive for #name {}
})
}

Expand Down Expand Up @@ -846,23 +830,6 @@ pub fn derive_try_from_primitive(input: TokenStream) -> TokenStream {

debug_assert_eq!(variant_idents.len(), variant_expressions.len());

let default_arm = match enum_info.default() {
Some(ident) => {
quote! {
_ => ::core::result::Result::Ok(
#name::#ident
)
}
}
None => {
quote! {
_ => ::core::result::Result::Err(
::#krate::TryFromPrimitiveError { number }
)
}
}
};

TokenStream::from(quote! {
impl ::#krate::TryFromPrimitive for #name {
type Primitive = #repr;
Expand Down Expand Up @@ -890,7 +857,9 @@ pub fn derive_try_from_primitive(input: TokenStream) -> TokenStream {
=> ::core::result::Result::Ok(Self::#variant_idents),
)*
#[allow(unreachable_patterns)]
#default_arm,
_ => ::core::result::Result::Err(
::#krate::TryFromPrimitiveError { number }
),
}
}
}
Expand All @@ -906,6 +875,9 @@ pub fn derive_try_from_primitive(input: TokenStream) -> TokenStream {
::#krate::TryFromPrimitive::try_from_primitive(number)
}
}

#[doc(hidden)]
impl ::#krate::CannotDeriveBothFromPrimitiveAndTryFromPrimitive for #name {}
})
}

Expand Down

0 comments on commit 0e2d006

Please sign in to comment.