Skip to content

excessive_precision inconsistency with exponential notation #6341

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
EndilWayfare opened this issue Nov 18, 2020 · 6 comments · Fixed by #14824
Closed

excessive_precision inconsistency with exponential notation #6341

EndilWayfare opened this issue Nov 18, 2020 · 6 comments · Fixed by #14824
Labels
A-documentation Area: Adding or improving documentation C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@EndilWayfare
Copy link

EndilWayfare commented Nov 18, 2020

I love the idea of the excessive_precision lint. I'm working on a parser for a 3rd-party, closed-source file format, and they must have weird float-to-string serialization code. The value in the file doesn't match the one in the UI (e.g. 6.93 serialized to 6.9299999999999997E+00. I know floats are hard and weird, and 6.93 might not be an exact value, but to serialize out a string that's beyond double-precision? Maybe it's technically correct. lexical-core doesn't think so, though). It's really handy to know that my unit test values aren't being silently truncated.

Most cases work just fine. However, I came across a case where using decimal and exponential literals causes clippy to disagree with itself.

#[test]
fn clippy_excessive_precision() {
    let decimal: f64 = 0.004886506780521244;

    // This trips `clippy::excessive_precision`
    let exponential: f64 = 4.886506780521244E-03;

    // Truncated `exponential`
    let clippy: f64 = 4.88650678052124E-03;

    // NOTE: `clippy::float_cmp` warns about the `assert_eq`s, but I'm evaluating binary identity...
    //       so I think this is appropriate?

    assert_eq!(decimal, exponential);

    // `clippy` seems to suggest these should be equal, but they're not
    assert_ne!(exponential, clippy);
    // ... but they ARE within the error margin. Platform issue, maybe?
    assert!((exponential - clippy).abs() < f64::EPSILON);
}

Meta

  • cargo clippy -V: clippy 0.0.212 (18bf6b4f0 2020-10-07)
  • rustc -Vv:
    rustc 1.47.0 (18bf6b4f0 2020-10-07)
    binary: rustc
    commit-hash: 18bf6b4f01a6feaf7259ba7cdae58031af1b7b39
    commit-date: 2020-10-07
    host: x86_64-pc-windows-msvc
    release: 1.47.0
    LLVM version: 11.0
    
@EndilWayfare EndilWayfare added the C-bug Category: Clippy is not doing the correct thing label Nov 18, 2020
@EndilWayfare
Copy link
Author

OH WAIT. Maybe this is actually/additionally a documentation issue?

I used rust-analyzer's auto-fix thing, and it just inserted underscores without truncating. As if it tripped clippy::unreadable_literal instead? clippy no longer complains.

#[test]
fn clippy_excessive_precision() {
    let decimal: f64 = 0.004886506780521244;

    // This trips `clippy::excessive_precision`
    let exponential: f64 = 4.886506780521244E-03;

    // Auto-fixed `exponential`
    let clippy: f64 = 4.886_506_780_521_244E-3;

    assert_eq!(decimal, exponential);

    // These are equal now, as expected
    assert_eq!(exponential, clippy);
}

The documentation for clippy::excessive_precision doesn't say anything about underscores. The only "Why is this bad" is silent truncation. Also, it doesn't complain about decimal.

@giraffate
Copy link
Contributor

@rustbot modify labels: +L-documentation -L-bug

@rustbot rustbot added A-documentation Area: Adding or improving documentation and removed C-bug Category: Clippy is not doing the correct thing labels Nov 19, 2020
@flip1995 flip1995 added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. C-bug Category: Clippy is not doing the correct thing labels Dec 6, 2020
@flip1995
Copy link
Member

flip1995 commented Dec 6, 2020

So if you look at the error message carefully, you'll see, that this lint doesn't complain about the fractional part, but about the exponent part for some reason:

warning: float has excessive precision
 --> src/lib.rs:5:28
  |
5 |     let exponential: f64 = 4.886506780521244E-03;
  |                            ^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `4.886_506_780_521_244E-3`
  |
  = note: `#[warn(clippy::excessive_precision)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision

If you change ...E-03 to ...E-3 it stops complaining. This is definitely a bug, because this lint should not get triggered by the exponential part.

@EndilWayfare
Copy link
Author

OOOH, you're totally right. I was so fixated on the underscores that I didn't notice the zero.

I imagine it's because "precision" is being measured by "count all digit characters in literal after the decimal point"?

@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 18, 2020
@flip1995
Copy link
Member

I imagine it's because "precision" is being measured by "count all digit characters in literal after the decimal point"?

I think we parse the literal and then compare their string representations or something like that, yeah.

@ajtribick
Copy link

Probably related to this, it also doesn't like the plus sign being used in exponents. E.g. trying to assign the following to an f64:

let x: f64 = 1.995_840_309_534_719_6e+292; // 0x1.fffffffffffffp+970

results in

let x: f64 = 1.995_840_309_534_719_6e+292; // 0x1.fffffffffffffp+970
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.995_840_309_534_719_6e292`

I personally prefer including the sign as it makes it more obvious where the separation between the sections is. Even if this is something that's worth linting, it's not a precision issue or a case where changing the type is appropriate, and the suggested fix should not be "truncation" (which at least for me suggests cutting off the end of the representation).

github-merge-queue bot pushed a commit that referenced this issue May 17, 2025
#14824)

Fixes #6341.

changelog: [`excessive_precision`] no longer triggers on an exponent
with leading zeros
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants