Skip to content

Commit

Permalink
Merge #7
Browse files Browse the repository at this point in the history
7: Use i64, and allow trivial_numeric_casts r=cuviper a=cuviper

The traits are implemented from 64-bit values, and we don't want to lose
any bits when comparing on platforms with smaller `isize`.

Simple enum expressions may have no explicit type, like `A = 1`, so then
the derived `1 as i64` is inferred like `1i64 as i64`, a trivial numeric
cast.  But it's quite possible that other expressions could have explicit
types, so we can't just assign it directly either.  The simple solution is
to just allow the `trivial_numeric_casts` in derived code.

Fixes #6.
  • Loading branch information
bors[bot] committed Jan 23, 2018
2 parents 6498f98 + c833a9e commit 7bc1968
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 17 deletions.
36 changes: 19 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ pub fn from_primitive(input: TokenStream) -> TokenStream {
_ => panic!("`FromPrimitive` can be applied only to the enums, {} is not an enum", name)
};

let from_u64_var = quote! { n };
let mut expr = quote! { 0isize };
let mut offset = 0isize;
let from_i64_var = quote! { n };
let mut expr = quote! { 0i64 };
let mut offset = 0i64;
let clauses: Vec<_> = variants.iter()
.map(|variant| {
let ident = &variant.ident;
Expand All @@ -49,7 +49,7 @@ pub fn from_primitive(input: TokenStream) -> TokenStream {

let discriminant_expr = match variant.discriminant {
Some((_, ref const_expr)) => {
expr = quote! { (#const_expr) as isize };
expr = quote! { (#const_expr) as i64 };
offset = 1;
expr.clone()
}
Expand All @@ -61,30 +61,31 @@ pub fn from_primitive(input: TokenStream) -> TokenStream {
};

quote! {
if #from_u64_var as isize == #discriminant_expr {
if #from_i64_var == #discriminant_expr {
Some(#name::#ident)
}
}
})
.collect();

let from_u64_var = if clauses.is_empty() { quote!(_) } else { from_u64_var };
let from_i64_var = if clauses.is_empty() { quote!(_) } else { from_i64_var };

let res = quote! {
#[allow(non_upper_case_globals)]
const #dummy_const: () = {
extern crate num as _num;

impl _num::traits::FromPrimitive for #name {
fn from_i64(n: i64) -> Option<Self> {
Self::from_u64(n as u64)
}

fn from_u64(#from_u64_var: u64) -> Option<Self> {
#[allow(trivial_numeric_casts)]
fn from_i64(#from_i64_var: i64) -> Option<Self> {
#(#clauses else)* {
None
}
}

fn from_u64(n: u64) -> Option<Self> {
Self::from_i64(n as i64)
}
}
};
};
Expand All @@ -103,8 +104,8 @@ pub fn to_primitive(input: TokenStream) -> TokenStream {
_ => panic!("`ToPrimitive` can be applied only to the enums, {} is not an enum", name)
};

let mut expr = quote! { 0isize };
let mut offset = 0isize;
let mut expr = quote! { 0i64 };
let mut offset = 0i64;
let variants: Vec<_> = variants.iter()
.map(|variant| {
let ident = &variant.ident;
Expand All @@ -117,7 +118,7 @@ pub fn to_primitive(input: TokenStream) -> TokenStream {

let discriminant_expr = match variant.discriminant {
Some((_, ref const_expr)) => {
expr = quote! { (#const_expr) as isize };
expr = quote! { (#const_expr) as i64 };
offset = 1;
expr.clone()
}
Expand All @@ -128,7 +129,7 @@ pub fn to_primitive(input: TokenStream) -> TokenStream {
}
};

quote!(#name::#ident => (#discriminant_expr) as u64)
quote!(#name::#ident => (#discriminant_expr))
})
.collect();

Expand All @@ -151,12 +152,13 @@ pub fn to_primitive(input: TokenStream) -> TokenStream {
extern crate num as _num;

impl _num::traits::ToPrimitive for #name {
#[allow(trivial_numeric_casts)]
fn to_i64(&self) -> Option<i64> {
self.to_u64().map(|x| x as i64)
#match_expr
}

fn to_u64(&self) -> Option<u64> {
#match_expr
self.to_i64().map(|x| x as u64)
}
}
};
Expand Down
17 changes: 17 additions & 0 deletions tests/issue-6.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![deny(trivial_numeric_casts)]
extern crate num;
#[macro_use]
extern crate num_derive;

#[derive(FromPrimitive, ToPrimitive)]
pub enum SomeEnum {
A = 1
}

#[test]
fn test_trivial_numeric_casts() {
use num::{FromPrimitive, ToPrimitive};
assert!(SomeEnum::from_u64(1).is_some());
assert!(SomeEnum::from_i64(-1).is_none());
assert_eq!(SomeEnum::A.to_u64(), Some(1));
}

0 comments on commit 7bc1968

Please sign in to comment.