Skip to content

Reduce clippy's complaints about valid decimals #7691

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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

Reduce clippy's complaints about valid decimals #7691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions clippy_lints/src/float_literal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,19 @@ impl<'tcx> LateLintPass<'tcx> for FloatLiteral {
}
}

/// The maximum number of significant decimal digits a float can contain.
/// f32: 23+1 bit significand, storing 6 to 9 decimal digits
/// f64: 52+1 bit significand, storing 15 to 17 decimal digits
/// Above the range's lower bound, precision in the decimal string may be lost,
/// but it is recommended to use the maximum number of digits, not the minimum,
/// when feeding these to a parser, as it preserves the accuracy in binary.
/// In addition, suggesting someone truncate an 8 decimal digit string
/// may change the result of the program if they apply it.
#[must_use]
fn max_digits(fty: FloatTy) -> u32 {
match fty {
FloatTy::F32 => f32::DIGITS,
FloatTy::F64 => f64::DIGITS,
FloatTy::F32 => 9,
FloatTy::F64 => 17,
}
}

Expand Down
34 changes: 19 additions & 15 deletions tests/ui/excessive_precision.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@ fn main() {
const GOOD32_SM: f32 = 0.000_000_000_1;
const GOOD32_DOT: f32 = 10_000_000_000.0;
const GOOD32_EDGE: f32 = 1.000_000_8;
// the Kahan Krew wuz here
const GOOD32_IEEE754_1: f32 = 0.123_456_789_f32;
const GOOD32_IEEE754_2: f32 = 0.123_456_789;
const GOOD32_IEEE754_3: f32 = 1.000_000_9;

const BAD32_1: f32 = 0.1;
const BAD32_2: f32 = 0.123_456_79;

const GOOD64: f64 = 0.123_456_789_012;
const GOOD64_SM: f32 = 0.000_000_000_000_000_1;
const GOOD64_DOT: f32 = 10_000_000_000_000_000.0;
const GOOD64_IEEE754_1: f64 = 0.123_456_789_012_345_67f64;
const GOOD64_IEEE754_2: f64 = 0.123_456_789_012_345_67;

const BAD32_1: f32 = 0.123_456_79_f32;
const BAD32_2: f32 = 0.123_456_79;
const BAD32_3: f32 = 0.1;
const BAD32_EDGE: f32 = 1.000_001;

const BAD64_1: f64 = 0.123_456_789_012_345_66_f64;
const BAD64_2: f64 = 0.123_456_789_012_345_66;
const BAD64_3: f64 = 0.1;
const BAD64_1: f64 = 0.1;
const BAD64_2: f64 = 0.123_456_789_012_345_68;

// Literal as param
println!("{:?}", 8.888_888_888_888_89);
Expand All @@ -37,16 +41,16 @@ fn main() {
let bad32_suf: f32 = 1.123_456_8_f32;
let bad32_inf = 1.123_456_8_f32;

let bad64: f64 = 0.123_456_789_012_345_66;
let bad64_suf: f64 = 0.123_456_789_012_345_66_f64;
let bad64_inf = 0.123_456_789_012_345_66;
let bad64: f64 = 0.123_456_789_012_345_67;
let bad64_suf: f64 = 0.123_456_789_012_345_67f64;
let bad64_inf = 0.123_456_789_012_345_67;

// Vectors
let good_vec32: Vec<f32> = vec![0.123_456];
let good_vec64: Vec<f64> = vec![0.123_456_789];

let good_vec32: Vec<f32> = vec![0.123_456_789];
let bad_vec32: Vec<f32> = vec![0.123_456_79];
let bad_vec64: Vec<f64> = vec![0.123_456_789_123_456_78];

let good_vec64: Vec<f64> = vec![0.123_456_789_012_345_67];
let bad_vec64: Vec<f64> = vec![0.123_456_789_012_345_68];

// Exponential float notation
let good_e32: f32 = 1e-10;
Expand Down
28 changes: 16 additions & 12 deletions tests/ui/excessive_precision.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@ fn main() {
const GOOD32_SM: f32 = 0.000_000_000_1;
const GOOD32_DOT: f32 = 10_000_000_000.0;
const GOOD32_EDGE: f32 = 1.000_000_8;
// the Kahan Krew wuz here
const GOOD32_IEEE754_1: f32 = 0.123_456_789_f32;
const GOOD32_IEEE754_2: f32 = 0.123_456_789;
const GOOD32_IEEE754_3: f32 = 1.000_000_9;

const BAD32_1: f32 = 0.100_000_000_000_1;
const BAD32_2: f32 = 0.123_456_789_012;

const GOOD64: f64 = 0.123_456_789_012;
const GOOD64_SM: f32 = 0.000_000_000_000_000_1;
const GOOD64_DOT: f32 = 10_000_000_000_000_000.0;
const GOOD64_IEEE754_1: f64 = 0.123_456_789_012_345_67f64;
const GOOD64_IEEE754_2: f64 = 0.123_456_789_012_345_67;

const BAD32_1: f32 = 0.123_456_789_f32;
const BAD32_2: f32 = 0.123_456_789;
const BAD32_3: f32 = 0.100_000_000_000_1;
const BAD32_EDGE: f32 = 1.000_000_9;

const BAD64_1: f64 = 0.123_456_789_012_345_67f64;
const BAD64_2: f64 = 0.123_456_789_012_345_67;
const BAD64_3: f64 = 0.100_000_000_000_000_000_1;
const BAD64_1: f64 = 0.100_000_000_000_000_000_1;
const BAD64_2: f64 = 0.123_456_789_012_345_678_9;

// Literal as param
println!("{:?}", 8.888_888_888_888_888_888_888);
Expand All @@ -42,11 +46,11 @@ fn main() {
let bad64_inf = 0.123_456_789_012_345_67;

// Vectors
let good_vec32: Vec<f32> = vec![0.123_456];
let good_vec64: Vec<f64> = vec![0.123_456_789];
let good_vec32: Vec<f32> = vec![0.123_456_789];
let bad_vec32: Vec<f32> = vec![0.123_456_789_012];

let bad_vec32: Vec<f32> = vec![0.123_456_789];
let bad_vec64: Vec<f64> = vec![0.123_456_789_123_456_789];
let good_vec64: Vec<f64> = vec![0.123_456_789_012_345_67];
let bad_vec64: Vec<f64> = vec![0.123_456_789_012_345_678_9];

// Exponential float notation
let good_e32: f32 = 1e-10;
Expand Down
84 changes: 24 additions & 60 deletions tests/ui/excessive_precision.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,112 +1,76 @@
error: float has excessive precision
--> $DIR/excessive_precision.rs:15:26
|
LL | const BAD32_1: f32 = 0.123_456_789_f32;
| ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_79_f32`
Comment on lines -1 to -5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change is faithful to the purpose of this lint. Why would we not suggest replacing 0.123_456_789 with 0.123_456_79, which eliminates the extraneous precision and still represents the identical floating point quantity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I have been wondering about that a little to myself, on reading the code at first I was kind of stunned and confused by the naming of the vars and fn so I didn't examine it closely enough to find all the hidden catches. So let me explain my reasoning more thoroughly. Part of it is somewhat esoteric and may be a better case for recommending this be altered in category, from a style lint to a restriction lint.

It is actually considered "good style" in many situations to use 9 or 17 significant digits. It is necessary for encoding the longer sequences, and faithful replication of the binary value with those 9 or 17 digits is what IEEE754 guarantees. This also matters for, well, the reason that I was alarmed at just using the constant directly: it is common for the character strings of numbers to be blithely copied from language to language. Indeed, most of Rust's float-related constants seem to be lifted from C's <float.h>.

While most language's atoi or equivalent is fairly faithful, some languages have less correct float parsers than Rust's, especially at the edges. So it is possible to introduce floating point error due to variations in parsing, rounding, and formatting between compilers and runtimes. As a general guard against this problem, "over" specifying by a few chars is common, or at least using all 9 or 17 digits. For instance, consider the string values used in Boost, Rust, and OpenCL for pi:

let boost_pi = 3.141592653589793238462643383279502884e+0; // triggers excessive_precision
let rust_pi = 3.14159265358979323846264338327950288f64; // triggers excessive_precision also!
let opencl_pi = 3.141592653589793115998; // triggers excessive_precision AND approx_constant!

// They of course are the same binary value.
assert_eq!(rust_pi, boost_pi);
assert_eq!(rust_pi, opencl_pi);
assert_eq!(rust_pi, std::f64::consts::PI);

Thus, I suppose my disagreement is with the lint per se, or at least what it currently encourages and allows to be machine-applied, since it would likely be preferable to upgrade the type to f64 for many f32 triggers of this lint.

|
= note: `-D clippy::excessive-precision` implied by `-D warnings`

error: float has excessive precision
--> $DIR/excessive_precision.rs:16:26
|
LL | const BAD32_2: f32 = 0.123_456_789;
| ^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_79`

error: float has excessive precision
--> $DIR/excessive_precision.rs:17:26
|
LL | const BAD32_3: f32 = 0.100_000_000_000_1;
LL | const BAD32_1: f32 = 0.100_000_000_000_1;
| ^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.1`

error: float has excessive precision
--> $DIR/excessive_precision.rs:18:29
|
LL | const BAD32_EDGE: f32 = 1.000_000_9;
| ^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.000_001`
= note: `-D clippy::excessive-precision` implied by `-D warnings`

error: float has excessive precision
--> $DIR/excessive_precision.rs:20:26
--> $DIR/excessive_precision.rs:17:26
|
LL | const BAD64_1: f64 = 0.123_456_789_012_345_67f64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66_f64`
LL | const BAD32_2: f32 = 0.123_456_789_012;
| ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_79`

error: float has excessive precision
--> $DIR/excessive_precision.rs:21:26
--> $DIR/excessive_precision.rs:25:26
|
LL | const BAD64_2: f64 = 0.123_456_789_012_345_67;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66`
LL | const BAD64_1: f64 = 0.100_000_000_000_000_000_1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.1`

error: float has excessive precision
--> $DIR/excessive_precision.rs:22:26
--> $DIR/excessive_precision.rs:26:26
|
LL | const BAD64_3: f64 = 0.100_000_000_000_000_000_1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.1`
LL | const BAD64_2: f64 = 0.123_456_789_012_345_678_9;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_68`

error: float has excessive precision
--> $DIR/excessive_precision.rs:25:22
--> $DIR/excessive_precision.rs:29:22
|
LL | println!("{:?}", 8.888_888_888_888_888_888_888);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `8.888_888_888_888_89`

error: float has excessive precision
--> $DIR/excessive_precision.rs:36:22
--> $DIR/excessive_precision.rs:40:22
|
LL | let bad32: f32 = 1.123_456_789;
| ^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8`

error: float has excessive precision
--> $DIR/excessive_precision.rs:37:26
--> $DIR/excessive_precision.rs:41:26
|
LL | let bad32_suf: f32 = 1.123_456_789_f32;
| ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8_f32`

error: float has excessive precision
--> $DIR/excessive_precision.rs:38:21
--> $DIR/excessive_precision.rs:42:21
|
LL | let bad32_inf = 1.123_456_789_f32;
| ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8_f32`

error: float has excessive precision
--> $DIR/excessive_precision.rs:40:22
|
LL | let bad64: f64 = 0.123_456_789_012_345_67;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66`

error: float has excessive precision
--> $DIR/excessive_precision.rs:41:26
|
LL | let bad64_suf: f64 = 0.123_456_789_012_345_67f64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66_f64`
Comment on lines -75 to -79
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cases like this are fixed a different way by #7722.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. That change does make me considerably happier with the lint.


error: float has excessive precision
--> $DIR/excessive_precision.rs:42:21
|
LL | let bad64_inf = 0.123_456_789_012_345_67;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66`

error: float has excessive precision
--> $DIR/excessive_precision.rs:48:36
--> $DIR/excessive_precision.rs:50:36
|
LL | let bad_vec32: Vec<f32> = vec![0.123_456_789];
| ^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_79`
LL | let bad_vec32: Vec<f32> = vec![0.123_456_789_012];
| ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_79`

error: float has excessive precision
--> $DIR/excessive_precision.rs:49:36
--> $DIR/excessive_precision.rs:53:36
|
LL | let bad_vec64: Vec<f64> = vec![0.123_456_789_123_456_789];
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_123_456_78`
LL | let bad_vec64: Vec<f64> = vec![0.123_456_789_012_345_678_9];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_68`

error: float has excessive precision
--> $DIR/excessive_precision.rs:53:24
--> $DIR/excessive_precision.rs:57:24
|
LL | let bad_e32: f32 = 1.123_456_788_888e-10;
| ^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8e-10`

error: float has excessive precision
--> $DIR/excessive_precision.rs:56:27
--> $DIR/excessive_precision.rs:60:27
|
LL | let bad_bige32: f32 = 1.123_456_788_888E-10;
| ^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8E-10`

error: aborting due to 18 previous errors
error: aborting due to 12 previous errors