rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.97k stars 12.53k forks source link

[ER] Suggest X::from() instead of "as" where possible #90467

Open leonardo-m opened 2 years ago

leonardo-m commented 2 years ago

This is a diagnostic enhancement request in two similar parts. Currently (1.58.0-nightly ff0e14829 2021-10-31) this gives no errors nor warnings:

#![allow(dead_code, unused_variables)]
fn main() {
    let x = 1_u32;
    let y = x as u64;
    const X: u32 = 1;
    const Y: u64 = X as _;
}

But I'd like a help message that suggests to replace the "as" with a from, nudging the programmer to write:

#![allow(dead_code, unused_variables)]
fn main() {
    let x = 1_u32;
    let y = u64::from(x);
    const X: u32 = 1;
    const Y: u64 = X as _;
}

And eventually once the two features are stable, nudging to write:

#![feature(const_num_from_num, const_trait_impl)]
#![allow(dead_code, unused_variables)]
fn main() {
    let x = 1_u32;
    let y = u64::from(x);
    const X: u32 = 1;
    const Y: u64 = u64::from(X);
}
euclio commented 2 years ago

Note that this is already partially implemented in clippy as clippy::cast_lossless. It doesn't catch the const case.

leonardo-m commented 2 years ago

Note that this is already partially implemented in clippy as

I think type conversions warnings/suggestions about built-in types should be in the core compiler because minimizing the usage of 'as' is sufficiently important.

thomcc commented 2 years ago

I think this is too opinionated to be in the core compiler, and is better off in clippy.

And there are a lot of #[allow(clippy::cast_lossless)] in the wild, and this is presumably in code that deliberately made a decision to turn it on (so you know they care about it some), because it's allowed by default.

leonardo-m commented 2 years ago

Rust is in its good place also because its designers were "opinionated", with good coding taste, to avoid hundreds of un-opinionated parts of C/C++/D languages. My informed opinion is that the current design of "as" casts in Rust is one of the few design mistakes of Rust, and getting away from it is a good thing. Because you have a language where x+y correctly gives you run-time errors in debug builds (and in release builds when you use -C overflow-checks), but where one "as" casts breaks your code because it doesn't perform an overflow test. Currently the compiler suggest to use try_into and similar functions to avoid this source of bugs. So using into() or X::from() is a way to be sure the compiler isn't throwing away some bits of data. If using from/into is too much heavy for you, then others shorter solutions need to be found. I use a to!{} macro that calls try_from only in debug builds.