Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 0.1.42 breaks code with #![deny(trivial_numeric_casts)] #6

Closed
Pzixel opened this issue Jan 23, 2018 · 5 comments
Closed

Version 0.1.42 breaks code with #![deny(trivial_numeric_casts)] #6

Pzixel opened this issue Jan 23, 2018 · 5 comments

Comments

@Pzixel
Copy link
Contributor

Pzixel commented Jan 23, 2018

Minimal reproducible example

Cargo.toml

[dependencies]
num = "0.1"
num-derive = "0.1"

main.rs

#![deny(trivial_numeric_casts)]
extern crate num;
#[macro_use]
extern crate num_derive;

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

fn main() {

}

Error:

   Compiling hello_world_clion v0.1.0 (file:///C:/Users/Alex/Documents/Rust/hello_world_clion)
error: trivial numeric cast: `isize` as `isize`. Cast can be replaced by coercion, this might require type ascription or a temporary variable
 --> src\main.rs:6:10
  |
6 | #[derive(FromPrimitive)]
  |          ^^^^^^^^^^^^^
  |
note: lint level defined here
 --> src\main.rs:1:9
  |
1 | #![deny(trivial_numeric_casts)]
  |         ^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Our project depends heavily on this deny attribute so disable it is a no-no. Currently we use num-derive = "=0.1.41" but I hope we can switch to higher version. Is seems to be an easy fix.

@cuviper
Copy link
Member

cuviper commented Jan 23, 2018

Sure, we can probably just add #[allow(trivial_numeric_casts)] in the derived code. Care to send a PR?
(and make sure we test with that deny somewhere too)

@Pzixel
Copy link
Contributor Author

Pzixel commented Jan 23, 2018

This deny is useful so I think we may just tune the macro a bit, instead of a as Foo it could just generate let a_as_foo: Foo : a. And it's safer than allow directive.

@cuviper
Copy link
Member

cuviper commented Jan 23, 2018

You're welcome to experiment with different approaches, of course.

@Pzixel
Copy link
Contributor Author

Pzixel commented Jan 23, 2018

Ok, I can try. However, I'm not really familiar with macros because I'm new in rust and I just wanted my code to compile 😄 , but the whole last month I'm just fixing issues one by one in my dependencies... And this one seems to be really hard. Any help about how can I debug macro or something? Because I have no idea how to do it.

@cuviper
Copy link
Member

cuviper commented Jan 23, 2018

I went ahead and allowed the warning in #7, if you'd like to test it on your code. We can followup with a different approach if you figure out something better.

Debugging macros is kind of a pain. On nightly rustc, you can use rustc --pretty=expanded to see what gets derived. There's also -Ztrace-macros and -Zdebug-macros, but I think those are only for macro_rules!.

bors bot added a commit that referenced this issue Jan 23, 2018
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.
@bors bors bot closed this as completed in #7 Jan 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants