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

Move the trivial_numeric_casts lint to allow-by-default #1020

Closed
alexcrichton opened this issue Mar 26, 2015 · 6 comments
Closed

Move the trivial_numeric_casts lint to allow-by-default #1020

alexcrichton opened this issue Mar 26, 2015 · 6 comments

Comments

@alexcrichton
Copy link
Member

This lint was recently added in rust-lang/rust#23630 as part of #803 and it disallows casts such as foo as i64 when foo has the type i64.

In theory this does sound like a lint which is nice to have, but I've found that in the context of FFI it's doesn't always play out so well. Because libc::c_int is simply an alias of some concrete primitive type, it is typically casted out into a known integer to be worked with in Rust. In this specific case the target type of i32 is frequently chosen, triggering this lint. Having to annotate crates with #[allow(trivial_numeric_casts)] is somewhat annoying for this common use case, and the broader lint (trivial_casts) seems more relevant to remain as warn-by-default.

cc @nrc

@nrc
Copy link
Member

nrc commented Mar 26, 2015

I do not oppose this change. I think we ended up with warn by default by miscommunication, rather than intention.

I marginally prefer warn by default because I think that ought to be the default behaviour for most integer casts, but I think I have a much higher tolerance for allows in my code than others and I see the merit of allow by default for the sake of FFI.

@sfackler
Copy link
Member

See also rust-lang/rust#23734

@mahkoh
Copy link
Contributor

mahkoh commented Mar 27, 2015

If only someone had mentioned this exact problem during the RFC discussion.

@eddyb
Copy link
Member

eddyb commented Mar 27, 2015

At the risk of sounding repetitive, what about an attribute (maybe #[allow(trivial_numeric_casts)] itself, but that might not work) on the types aliases which may vary between compilations?

@lilyball
Copy link
Contributor

I would much prefer to have the lint simply not warn if casting between type aliases. Warning me by default when I say something like 42i32 as i32 is reasonable; warning me by default when I say something like 42i32 as c_int is not. If we make this lint allow-by-default I'm not sure who would ever actually turn it on.

Note that this isn't just an issue with type aliases that may vary by configuration, it's also an issue with type aliases where the programmer simply doesn't necessarily know which concrete type the type alias is an alias for (and isn't required to know either; if they were then why would I be using a type alias?) or does know but simply wants to make it clear to any potential readers that they're intentionally using the type alias because it carries some semantic meaning.

@scottmcm
Copy link
Member

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

7 participants