Merge #3

3: Update deps and derivation algorithm r=cuviper a=hcpl Fixes #2. An updated version of rust-num/num#353 which includes suggestions outlined [here](rust-num/num#353 (review)) and [here](https://github.com/rust-num/num/pull/353/files/76b5b2189f2b45e864e14c38c7856be578125931#r157100221): - Update `quote` and `syn` to parse new Rust syntax; - Update `compiletest_rs` to solve Manishearth/compiletest-rs#86; - Add support for arbitrary constant expressions as enum discriminants; - Remove the need to `extern crate num` just for deriving. Some notes: - `#[derive(FromPrimitive)]` now uses if-else to do its job because non-literal expressions are not allowed for pattern matching. - I added tests for self-containment of `#[derive]` alongside the code testing derivation functionality to keep the tests small. Would it be better to separate concerns? - `with_custom_value` should have all three tests like `trivial`.
rust-num · Jan 22, 2018 · 60e9acd · 60e9acd
2 parents 14f9a98 + 0f58c1c
commit 60e9acd
Show file tree

Hide file tree

Showing 8 changed files with 188 additions and 76 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -12,15 +12,23 @@ version = "0.1.41"
 readme = "README.md"
 
 [dependencies]
-quote = "0.1.3"
-syn = "0.7.0"
+proc-macro2 = "0.2.1"
+quote = "0.4.2"
+syn = "0.12.7"
 
 [dev-dependencies]
-compiletest_rs = "0.2.5"
+compiletest_rs = "0.3.5"
 
 [dev-dependencies.num]
 version = "0.1"
 
+[dev-dependencies.num-derive]
+path = "."
+features = ["full-syntax"]
+
+[features]
+full-syntax = ["syn/full"]
+
 [lib]
 name = "num_derive"
 proc-macro = true

diff --git a/README.md b/README.md
@@ -12,7 +12,8 @@ Add this to your `Cargo.toml`:
 
 ```toml
 [dependencies]
-num-derive= "0.1"
+num = "0.1"
+num-derive = "0.1"
 ```
 
 and this to your crate root:
@@ -22,6 +23,16 @@ and this to your crate root:
 extern crate num_derive;
 ```
 
+## Optional features
+
+- **`full-syntax`** — Enables `num-derive` to handle enum discriminants
+  represented by complex expressions. Usually can be avoided by
+  [utilizing constants], so only use this feature if namespace pollution is
+  undesired and [compile time doubling] is acceptable.
+
+[utilizing constants]: https://github.com/rust-num/num-derive/pull/3#issuecomment-359044704
+[compile time doubling]: https://github.com/rust-num/num-derive/pull/3#issuecomment-359172588
+
 ## Compatibility
 
 The `num-derive` crate is tested for rustc 1.15 and greater.
diff --git a/src/lib.rs b/src/lib.rs
@@ -11,109 +11,156 @@
 #![crate_type = "proc-macro"]
 #![doc(html_root_url = "https://docs.rs/num-derive/0.1")]
 
-extern crate syn;
+extern crate proc_macro;
+
+extern crate proc_macro2;
 #[macro_use]
 extern crate quote;
-extern crate proc_macro;
+extern crate syn;
 
 use proc_macro::TokenStream;
+use proc_macro2::Span;
 
-use syn::Body::Enum;
-use syn::VariantData::Unit;
+use syn::{Data, Fields, Ident};
 
 #[proc_macro_derive(FromPrimitive)]
 pub fn from_primitive(input: TokenStream) -> TokenStream {
-    let source = input.to_string();
-
-    let ast = syn::parse_macro_input(&source).unwrap();
+    let ast: syn::DeriveInput = syn::parse(input).unwrap();
     let name = &ast.ident;
+    let dummy_const = Ident::new(&format!("_IMPL_NUM_FROM_PRIMITIVE_FOR_{}", name), Span::call_site());
 
-    let variants = match ast.body {
-        Enum(ref variants) => variants,
+    let variants = match ast.data {
+        Data::Enum(ref data_enum) => &data_enum.variants,
         _ => panic!("`FromPrimitive` can be applied only to the enums, {} is not an enum", name)
     };
 
-    let mut idx = 0;
-    let variants: Vec<_> = variants.iter()
+    let from_u64_var = quote! { n };
+    let mut expr = quote! { 0isize };
+    let mut offset = 0isize;
+    let clauses: Vec<_> = variants.iter()
         .map(|variant| {
             let ident = &variant.ident;
-            match variant.data {
-                Unit => (),
+            match variant.fields {
+                Fields::Unit => (),
                 _ => {
                     panic!("`FromPrimitive` can be applied only to unitary enums, {}::{} is either struct or tuple", name, ident)
                 },
             }
-            if let Some(val) = variant.discriminant {
-                idx = val.value;
+
+            let discriminant_expr = match variant.discriminant {
+                Some((_, ref const_expr)) => {
+                    expr = quote! { (#const_expr) as isize };
+                    offset = 1;
+                    expr.clone()
+                }
+                None => {
+                    let tt = quote! { #expr + #offset };
+                    offset += 1;
+                    tt
+                }
+            };
+
+            quote! {
+                if #from_u64_var as isize == #discriminant_expr {
+                    Some(#name::#ident)
+                }
             }
-            let tt = quote!(#idx => Some(#name::#ident));
-            idx += 1;
-            tt
         })
         .collect();
 
+    let from_u64_var = if clauses.is_empty() { quote!(_) } else { from_u64_var };
+
     let res = quote! {
-        impl ::num::traits::FromPrimitive for #name {
-            fn from_i64(n: i64) -> Option<Self> {
-                Self::from_u64(n as u64)
-            }
+        #[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(n: u64) -> Option<Self> {
-                match n {
-                    #(variants,)*
-                    _ => None,
+                fn from_u64(#from_u64_var: u64) -> Option<Self> {
+                    #(#clauses else)* {
+                        None
+                    }
                 }
             }
-        }
+        };
     };
 
-    res.to_string().parse().unwrap()
+    res.into()
 }
 
 #[proc_macro_derive(ToPrimitive)]
 pub fn to_primitive(input: TokenStream) -> TokenStream {
-    let source = input.to_string();
-
-    let ast = syn::parse_macro_input(&source).unwrap();
+    let ast: syn::DeriveInput = syn::parse(input).unwrap();
     let name = &ast.ident;
+    let dummy_const = Ident::new(&format!("_IMPL_NUM_TO_PRIMITIVE_FOR_{}", name), Span::call_site());
 
-    let variants = match ast.body {
-        Enum(ref variants) => variants,
+    let variants = match ast.data {
+        Data::Enum(ref data_enum) => &data_enum.variants,
         _ => panic!("`ToPrimitive` can be applied only to the enums, {} is not an enum", name)
     };
 
-    let mut idx = 0;
+    let mut expr = quote! { 0isize };
+    let mut offset = 0isize;
     let variants: Vec<_> = variants.iter()
         .map(|variant| {
             let ident = &variant.ident;
-            match variant.data {
-                Unit => (),
+            match variant.fields {
+                Fields::Unit => (),
                 _ => {
                     panic!("`ToPrimitive` can be applied only to unitary enums, {}::{} is either struct or tuple", name, ident)
                 },
             }
-            if let Some(val) = variant.discriminant {
-                idx = val.value;
-            }
-            let tt = quote!(#name::#ident => #idx);
-            idx += 1;
-            tt
+
+            let discriminant_expr = match variant.discriminant {
+                Some((_, ref const_expr)) => {
+                    expr = quote! { (#const_expr) as isize };
+                    offset = 1;
+                    expr.clone()
+                }
+                None => {
+                    let tt = quote! { #expr + #offset };
+                    offset += 1;
+                    tt
+                }
+            };
+
+            quote!(#name::#ident => (#discriminant_expr) as u64)
         })
         .collect();
 
+    let match_expr = if variants.is_empty() {
+        // No variants found, so do not use Some to not to trigger `unreachable_code` lint
+        quote! {
+            match *self {}
+        }
+    } else {
+        quote! {
+            Some(match *self {
+                #(#variants,)*
+            })
+        }
+    };
+
     let res = quote! {
-        impl ::num::traits::ToPrimitive for #name {
-            fn to_i64(&self) -> Option<i64> {
-                self.to_u64().map(|x| x as i64)
-            }
+        #[allow(non_upper_case_globals)]
+        const #dummy_const: () = {
+            extern crate num as _num;
 
-            fn to_u64(&self) -> Option<u64> {
-                Some(match *self {
-                    #(variants,)*
-                })
+            impl _num::traits::ToPrimitive for #name {
+                fn to_i64(&self) -> Option<i64> {
+                    self.to_u64().map(|x| x as i64)
+                }
+
+                fn to_u64(&self) -> Option<u64> {
+                    #match_expr
+                }
             }
-        }
+        };
     };
 
-    res.to_string().parse().unwrap()
+    res.into()
 }
diff --git a/tests/compiletest.rs b/tests/compiletest.rs
@@ -3,8 +3,10 @@ extern crate compiletest_rs as compiletest;
 use std::path::PathBuf;
 use std::env::var;
 
+use compiletest::Config;
+
 fn run_mode(mode: &'static str) {
-    let mut config = compiletest::default_config();
+    let mut config = Config::default();
 
     let cfg_mode = mode.parse().ok().expect("Invalid mode");
 

diff --git a/tests/empty_enum.rs b/tests/empty_enum.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-extern crate num;
+extern crate num as num_renamed;
 #[macro_use]
 extern crate num_derive;
 
@@ -17,7 +17,7 @@ enum Color {}
 
 #[test]
 fn test_empty_enum() {
-    let v: [Option<Color>; 1] = [num::FromPrimitive::from_u64(0)];
+    let v: [Option<Color>; 1] = [num_renamed::FromPrimitive::from_u64(0)];
 
     assert_eq!(v, [None]);
 }
diff --git a/tests/num_derive_without_num.rs b/tests/num_derive_without_num.rs
@@ -0,0 +1,20 @@
+// Copyright 2013-2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate num_derive;
+
+#[derive(Debug, FromPrimitive, ToPrimitive)]
+enum Direction {
+    Up,
+    Down,
+    Left,
+    Right,
+}
diff --git a/tests/trivial.rs b/tests/trivial.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-extern crate num;
+extern crate num as num_renamed;
 #[macro_use]
 extern crate num_derive;
 
@@ -21,20 +21,20 @@ enum Color {
 
 #[test]
 fn test_from_primitive_for_trivial_case() {
-    let v: [Option<Color>; 4] = [num::FromPrimitive::from_u64(0),
-                                 num::FromPrimitive::from_u64(1),
-                                 num::FromPrimitive::from_u64(2),
-                                 num::FromPrimitive::from_u64(3)];
+    let v: [Option<Color>; 4] = [num_renamed::FromPrimitive::from_u64(0),
+                                 num_renamed::FromPrimitive::from_u64(1),
+                                 num_renamed::FromPrimitive::from_u64(2),
+                                 num_renamed::FromPrimitive::from_u64(3)];
 
     assert_eq!(v,
                [Some(Color::Red), Some(Color::Blue), Some(Color::Green), None]);
 }
 
 #[test]
 fn test_to_primitive_for_trivial_case() {
-    let v: [Option<u64>; 3] = [num::ToPrimitive::to_u64(&Color::Red),
-                               num::ToPrimitive::to_u64(&Color::Blue),
-                               num::ToPrimitive::to_u64(&Color::Green)];
+    let v: [Option<u64>; 3] = [num_renamed::ToPrimitive::to_u64(&Color::Red),
+                               num_renamed::ToPrimitive::to_u64(&Color::Blue),
+                               num_renamed::ToPrimitive::to_u64(&Color::Green)];
 
     assert_eq!(v, [Some(0), Some(1), Some(2)]);
 }
@@ -43,8 +43,8 @@ fn test_to_primitive_for_trivial_case() {
 fn test_reflexive_for_trivial_case() {
     let before: [u64; 3] = [0, 1, 2];
     let after: Vec<Option<u64>> = before.iter()
-        .map(|&x| -> Option<Color> { num::FromPrimitive::from_u64(x) })
-        .map(|x| x.and_then(|x| num::ToPrimitive::to_u64(&x)))
+        .map(|&x| -> Option<Color> { num_renamed::FromPrimitive::from_u64(x) })
+        .map(|x| x.and_then(|x| num_renamed::ToPrimitive::to_u64(&x)))
         .collect();
     let before = before.into_iter().cloned().map(Some).collect::<Vec<_>>();