#[derive(FromPrimitive)] panics on pub(crate)

[cuviper]

From @hmvp on December 14, 2017 15:6

This does not compile:

#[derive(Debug, FromPrimitive)]
pub(crate) enum Foo {
    A = 1,
    B = 2,
}

removing (crate) makes it work

Copied from original issue: rust-num/num#352

[cuviper]

From @hcpl on December 14, 2017 16:3

Applying this patch to num will make the OP's code compilable:

diff --git a/derive/Cargo.toml b/derive/Cargo.toml
index 2a17ce0..94cf68a 100644
--- a/derive/Cargo.toml
+++ b/derive/Cargo.toml
@@ -11,8 +11,8 @@ repository = "https://github.com/rust-num/num"
 version = "0.1.41"
 
 [dependencies]
-quote = "0.1.3"
-syn = "0.7.0"
+quote = "0.3.15"
+syn = "0.11.11"
 
 [dev-dependencies]
 compiletest_rs = "0.2.5"
diff --git a/derive/src/lib.rs b/derive/src/lib.rs
index e75fa06..8c0f420 100644
--- a/derive/src/lib.rs
+++ b/derive/src/lib.rs
@@ -42,8 +42,10 @@ pub fn from_primitive(input: TokenStream) -> TokenStream {
                     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;
+            if let Some(ref val) = variant.discriminant {
+                if let syn::ConstExpr::Lit(syn::Lit::Int(value, _)) = *val {
+                    idx = value;
+                }
             }
             let tt = quote!(#idx => Some(#name::#ident));
             idx += 1;
@@ -91,8 +93,10 @@ pub fn to_primitive(input: TokenStream) -> TokenStream {
                     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;
+            if let Some(ref val) = variant.discriminant {
+                if let syn::ConstExpr::Lit(syn::Lit::Int(value, _)) = *val {
+                    idx = value;
+                }
             }
             let tt = quote!(#name::#ident => #idx);
             idx += 1;

@cuviper, what do you think about this code and related compatibility issues? If everything is ok, I'll send a PR containing this patch.

[cuviper]

From @hcpl on December 14, 2017 16:50

And I'd like to mention that

#[macro_use]
extern crate num_derive;

#[derive(Debug, FromPrimitive)]
pub(crate) enum Foo {
    A = 1,
    B = 2,
}

doesn't work because num-derive doesn't re-export traits from num-traits -- you also need extern crate num; (not even extern crate num-traits; can fix this because the derived code depends directly on num). This is something I'd also like to rectify when I get some time.

[cuviper]

what do you think about this code and related compatibility issues?

The code is fine with me if it works. What exactly are the compatibility issues?

I think bumping semver on num-derive wouldn't be a problem, if needed. It's not the sort of crate that shows up as public dependencies -- only the traits it implements.

num-derive doesn't re-export traits from num-traits

I don't think proc-macro crates are even allowed to export anything but their #[proc_macro_derive] functions.

But I think it would be OK to make this depend on num-traits instead of num (as a breaking change).

[cuviper]

From @hcpl on December 14, 2017 17:28

What exactly are the compatibility issues?

Well, my question was more like "is there any possibility for breaking changes to happen" if this patch gets accepted. Though you already said this is a non-issue, so we're good.

I don't think proc-macro crates are even allowed to export anything but their #[proc_macro_derive] functions.

Yeah, that was my mistake (funny thing, I crossed my part out right after you posted the comment 😄).

The thing is, we could make num-derive to (ab)use such a Rust construct:

const _IMPL_NUM_FROM_PRIMITIVE_FOR_Foo: () = {
    extern crate num_traits as _num_traits;
    /* ... */
};

like serde_derive does so that we would only need a num-traits = "0.1" entry in Cargo.toml but not extern crate num_traits; to make this work.

Replacing num with num-traits is another matter which is not that important compared to the above one, but is a nice thing for builds that don't need all num features.

[cuviper]

Well, my question was more like "is there any possibility for breaking changes to happen" if this patch gets accepted. Though you already said this is a non-issue, so we're good.

It probably just depends on what the rustc minimum is for the newer syn/quote, but it looks like they both have CI for 1.15.1, so that should be fine. We could still bump for num-traits though.

(ab)use such a Rust construct: [...] like serde_derive does

I'm happy to follow serde's lead on this.