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

Broken suggestion from excessive_precision lint #5201

Closed
dtolnay opened this issue Feb 20, 2020 · 4 comments · Fixed by #5202
Closed

Broken suggestion from excessive_precision lint #5201

dtolnay opened this issue Feb 20, 2020 · 4 comments · Fixed by #5202
Labels
I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@dtolnay
Copy link
Member

dtolnay commented Feb 20, 2020

This is minimized from some code in serde_json: https://github.com/serde-rs/json/blob/ed479b4656be48760f3d3373d0fe476541e94e0e/src/de.rs#L1025

fn main() {
    let _ = 1e099;
}
error: literal cannot be represented as the underlying type without loss of precision
 --> src/main.rs:2:13
  |
2 |     let _ = 1e099;
  |             ^^^^^ help: consider changing the type or replacing it with: `1e9_9.0`
  |
  = note: `#[deny(clippy::excessive_precision)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision

Clippy's suggestion of writing this as 1e9_9.0 seems wrong and does not work.

error[E0610]: `{float}` is a primitive type and therefore doesn't have fields
 --> src/main.rs:2:19
  |
2 |     let _ = 1e9_9.0;
  |                   ^

Separately, I don't feel that there should be an on-by-default correctness lint against writing 1e099. I understand that this number can't be represented exactly by f64 (the value ends up being 999999999999999967336168804116691273849533185806555472917961779471295845921727862608739868455469056.0) but this seems excessively noisy for a default lint.

Mentioning @krishna-veerareddy @flip1995 because this lint was last touched in #5185.

clippy 0.0.212 (2855b21 2020-02-19)

dtolnay added a commit to serde-rs/json that referenced this issue Feb 20, 2020
dtolnay added a commit to dtolnay/miniserde that referenced this issue Feb 20, 2020
@krishna-veerareddy
Copy link
Contributor

@dtolnay Maybe we could exclude linting literals with exponential notation such as 1e10 or 1E10. Then we would only be linting fully typed out literals. If this isn't ideal either then I can separate out this lint from excessive_precision and make it a new lint with allow by default or remove it from clippy. Either way Ill start looking into the broken suggestion issue but let me know your thoughts.

@dtolnay
Copy link
Member Author

dtolnay commented Feb 20, 2020

Whether or not there is exponential notation, I don't feel that this should be enabled by default. I recommend changing excessive_precision to a "Restriction" lint which is off by default.

@krishna-veerareddy
Copy link
Contributor

krishna-veerareddy commented Feb 20, 2020

Well excessive precision also lints cases such as 0.123_456_789_012_345_67(too many decimal digits) and it has been doing that for a while without issues so we can leave excessive_precision in correctness. I would have to move my recent changes into a new lint under the restriction category.

excessive_precision was under style category before we added my changes and uplifted it to correctness so we can remove my changes and move excessive_precision back to style although that is not the correct category for what that lint is trying to do. @flip1995 Should I leave excessive_precision in correctness after I remove my recent changes?

@krishna-veerareddy
Copy link
Contributor

@flip1995 Hey I reverted the changes I made in #5185 and moved the changes to a new lint under the restriction category. excessive_precision is back in the style category as it was before.

bors added a commit that referenced this issue Feb 21, 2020
…literal-restriction, r=flip1995

Move check for lossy whole-number floats out of `excessive_precision`

changelog: Add new lint `lossy_float_literal` to detect lossy whole number float literals and move it out of `excessive_precision` again.

Fixes #5201
bors added a commit that referenced this issue Feb 21, 2020
…literal-restriction, r=flip1995

Move check for lossy whole-number floats out of `excessive_precision`

changelog: Add new lint `lossy_float_literal` to detect lossy whole number float literals and move it out of `excessive_precision` again.

Fixes #5201
@bors bors closed this as completed in acfcbee Feb 21, 2020
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants