Skip to content

Initializing primitive from literal with cast #3842

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

Closed
gnzlbg opened this issue Mar 4, 2019 · 9 comments
Closed

Initializing primitive from literal with cast #3842

gnzlbg opened this issue Mar 4, 2019 · 9 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Mar 4, 2019

I've seen code like this:

fn main() {
    let _ = 100 as usize;
    let _ = 100 as f64;
}

where clippy currently errors with

warning: casting i32 to f64 may become silently lossy if types change
 --> src/main.rs:3:13
  |
3 |     let _ = 100 as f64;
  |             ^^^^^^^^^^ help: try: `f64::from(100)`
  |
  = note: #[warn(clippy::cast_lossless)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless

I think we should at least suggest if people didn't meant to write:

fn main() {
    let _ = 100_usize;
    let _ = 100_f64;
}

instead. Semantically, 100_i32 as f64 and 100_f64 are not the exact same thing, but in practice..

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages L-suggestion Lint: Improving, adding or fixing lint suggestions labels Mar 5, 2019
@rink1969
Copy link

I'd like to try this issue.
But I'm not sure what is expected output?
Is something like this OK?

warning: casting integer literal to f64 is unnecessary
 --> examples/clippytest.rs:3:13
  |
3 |     let y = 100 as f64;
  |             ^^^^^^^^^^ help: try: `100_f64`
  |
  = note: #[warn(clippy::unnecessary_cast)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Mar 12, 2019

cc @oli-obk I'm not sure whether suggesting 100_f64 only is ok, or whether both should be suggested.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 12, 2019

Yes I think we should be extending the cast_lossless lint to suggest both solutions. This doesn't require a new lint.

@rink1969
Copy link

I think it's unnecessary cast for integer literal.
We can directly define f64 literal like 100_f64.

Another case is 100_i32 as f64, I also think it's unnecessary cast.
Same reason as before.
But this will break many test case.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Mar 12, 2019

Is constructing a floating-point literal exactly the same thing as constructing and integer literal and casting it to a floating point value ? (e.g. are literals truncated according to the same rules as casts?).

@rink1969
Copy link

I checked the llvm ir generated by rustc.
100 as f64 100_f64 and 100_i32 as f64, these three cases got same irs( a floating-point literal).
f64::from(100) has an extra function call.

So I think the cast is unnecessary for integer literal.
But it's ok for var.

@rink1969
Copy link

I've submit a PR.
#3877
So we can see the effect about the fix.

bors added a commit that referenced this issue Mar 14, 2019
casting integer literal to float is unnecessary

fix issue #3842
@rink1969
Copy link

@gnzlbg Do you think we can close this issue?

@flip1995
Copy link
Member

Yes this is implemented in #3877

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

No branches or pull requests

4 participants