rust-clippy
rust-clippy copied to clipboard
rust-lang
Have a lint against usize-to-u64 casts (or, against *all* integer casts)
What it does
I would like clippy to lint against all integer casts. So I have set:
#![warn(
clippy::cast_possible_wrap, // unsigned -> signed
clippy::cast_sign_loss, // signed -> unsigned
clippy::cast_lossless,
clippy::cast_possible_truncation,
)]
However, I just by accident noticed that this does not lint against usize-to-u64 casts. I guess cast_possible_truncation says "this cannot truncate because we don't have more than 64bit pointer size", and "cast_lossless" says "ah this might be lossy on platforms with pointers larger than 64bit", and then neither of them does anything.
I would be happy to either have one of these lints also trigger on usize-to-u64 casts, or to have a new lint against all integer casts.
Lint Name
cast_integer
Category
pedantic
Advantage
Integer casts are subtle and should be done via From/TryFrom, never via as, so I want to rule out all of them in my codebase.
Drawbacks
No response
Example
pub fn foo(x: usize) -> u64 { x as u64 }
Could be written as:
pub fn foo(x: usize) -> u64 { u64::try_from(x).unwrap() }
@xFrednet I was thinking about claiming this as a first RustClippy issue. it looks like the misunderstanding was whether PR #10038 fixed this issue, and that it actually doesn't. Am I correctly understanding? If so, if this one open to claim?
Hey @ctaymor , welcome to Clippy! You're correct, https://github.com/rust-lang/rust-clippy/pull/10038 updated the output a bit to be cleaner, but didn't address the issue. You're welcome to take it :)
Hey @xFrednet, I believe there has been no active development on this issue for the last couple of months, right? Could I claim this?
Do I understand correctly that the approach would be to create a new lint that warns on integer casts using as?
Hey @gernot-ohner, you should be able to claim this, since there hasn't been any activity to my knowledge.
For the correct approach, it depends a bit on how @RalfJung would like to use the new lint. My understanding, is that they want to restrict all integer casts. Looking at our lint list I noticed cast_lossless which sounds like a lint, that should trigger on this. But it doesn't (See Playground) and it's also interesting, that it doesn't even trigger on the example from the lint description.
So, I think this sounds like a false positive. It probably makes sense to first look at the code of that lint, and see why it doesn't trigger on the example from the lint description and the example from this issue description.
cast_lossless shouldn't trigger on usize→u64 casts as there is no corresponding Into implementation. This means there isn't an alternative other than try_into, which is not a lossless conversion.
cast_possible_truncation ignores the case because there aren't any platforms where usize is larger than u64, so triggering on those cases just ends up being noise.
@Jarcho Thank you for the input. So, would you recommend adding a new lint?
I think a restriction lint for all integer casts would be the best way. It feels strange to make a lint for only the one cast and the catch-all lint could be made to not trigger when any of the other related lints do in order to avoid the duplicate messages.
We could also add the case to cast_possible_truncation since it technically fits there even though it's unlikely to ever truncate.
So cast_lossless doesn't warn since the cast is not lossless, and cast_possible_truncation doesn't warn since the cast cannot truncate (i.e., it is lossless)? That does not sound very consistent. ;)
If the lang team wants to limit the maximum size of usize to 64 bits then the cast would really be lossless for reals. It would be nice to be able to at least opt-in to that constraint since anything using an as cast there is already making that assumption.
Bonus points in the far off future when someone who hopefully isn't me gets to curse the idiots who couldn't conceive of the need for more 64 bits worth of byte-addressable storage.
If we assume that we keep adding more larger usize, usize as N could possibly truncate for any integer type N. It's not clear how useful cast_possible_truncation firing on every case would be, but also I expect there to be few places using usize as N.
Having an integer type with no static upper bound means that reasoning about it for conversions is difficult.
