Some accuracy lints for floating point operations

[thiagoarrais]

This will add some lints for accuracy on floating point operations suggested by @clarfon in #2040 (fixes #2040).

These are the remaining lints:

• x.powi(2) => x * x
• x.logN() / y.logN() => x.logbase(y)
• x.logbase(E) => x.log()
• x.logbase(10) => x.log10()
• x.logbase(2) => x.log2().
• x * PI / 180 => x.to_radians()
• x * 180 / PI => x.to_degrees()
• (x + 1).log() => x.log_1p()
• sqrt(x * x + y * y) => x.hypot(y)

changelog: Included some accuracy lints for floating point operations

[flip1995]

LGTM. Please read up in #4897 on the discussion of naming these lints and which lint should apply to which float expression.

[thiagoarrais]

I'll make a note to review which lints goes into IMPRECISE_FLOPS or SUBOPTIMAL_FLOPS before removing the WIP marker. I'm not really sure which one goes where right now to be honest.

About naming, I assume I'm ok if I stick to those two names?

[flip1995]

You can just put the different cases where you think they fit best (and note that in the PR body for easier review please). The names SGTM.

[thiagoarrais]

I'm trying to make sqrt(x * x + y * y) => x.hypot(y) but bumping into the error "could not replace range (...) maybe parts of it were already replaced". AFAIK, that is happening because x * x + y * y already gets fixed into x.mul_add(x, x * y).

Any tips on how to avoid that? Should I change the mul_add lint to skip that case?

[flip1995]

I think making mul_add skip the case is the best solution here.

[thiagoarrais]

Still working on this. Quick question: how do you folks usually deal with cross lint errors?

I've got a problem because the lint for x.powi(2) + y produces x * x + y, which is rejected by the mul_add lint. It wants the expression to be x.mul_add(x, y).

[flip1995]

If a lint suggests code that again gets linted by another lint, that is not a problem.

Only if 2 lints trigger on the same code simultaneously is a problem. In those cases one lint should take preference.

[thiagoarrais]

Should I do anything differently here, then? I'm getting the error below when I run TESTNAME=floating_point_powi cargo uitest with my current code:

error: failed to compile fixed code
status: exit code: 1
command: "/data/rust-clippy/target/debug/clippy-driver" "tests/ui/floating_point_powi.fixed" "-L" "/data/rust-clippy/target/debug/test_build_base" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/data/rust-clippy/target/debug/test_build_base/floating_point_powi.stage-id" "-L" "/data/rust-clippy/target/debug/deps" "-L" "/data/rust-clippy/target/debug/deps" "-Dwarnings" "-Zui-testing" "--extern" "regex=/data/rust-clippy/target/debug/deps/libregex-c986894c984c8eac.rlib" "--extern" "serde=/data/rust-clippy/target/debug/deps/libserde-bdb8a48e18d84a1f.rlib" "--extern" "clippy_lints=/data/rust-clippy/target/debug/deps/libclippy_lints-0033f303daa204f4.rlib" "-L" "/data/rust-clippy/target/debug/test_build_base/floating_point_powi.stage-id.aux"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
{"message":"multiply and add expressions can be calculated more efficiently and accurately","code":{"code":"clippy::suboptimal_flops","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/floating_point_powi.fixed","byte_start":192,"byte_end":203,"line_start":11,"line_end":11,"column_start":13,"column_end":24,"is_primary":true,"text":[{"text":"    let _ = (x * x + y).sqrt();","highlight_start":13,"highlight_end":24}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"`-D clippy::suboptimal-flops` implied by `-D warnings`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"consider using","code":null,"level":"help","spans":[{"file_name":"tests/ui/floating_point_powi.fixed","byte_start":192,"byte_end":203,"line_start":11,"line_end":11,"column_start":13,"column_end":24,"is_primary":true,"text":[{"text":"    let _ = (x * x + y).sqrt();","highlight_start":13,"highlight_end":24}],"label":null,"suggested_replacement":"x.mul_add(x, y)","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: multiply and add expressions can be calculated more efficiently and accurately\n  --> tests/ui/floating_point_powi.fixed:11:13\n   |\nLL |     let _ = (x * x + y).sqrt();\n   |             ^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)`\n   |\n   = note: `-D clippy::suboptimal-flops` implied by `-D warnings`\n\n"}
{"message":"multiply and add expressions can be calculated more efficiently and accurately","code":{"code":"clippy::suboptimal_flops","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/floating_point_powi.fixed","byte_start":224,"byte_end":235,"line_start":12,"line_end":12,"column_start":13,"column_end":24,"is_primary":true,"text":[{"text":"    let _ = (x + y * y).sqrt();","highlight_start":13,"highlight_end":24}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"consider using","code":null,"level":"help","spans":[{"file_name":"tests/ui/floating_point_powi.fixed","byte_start":224,"byte_end":235,"line_start":12,"line_end":12,"column_start":13,"column_end":24,"is_primary":true,"text":[{"text":"    let _ = (x + y * y).sqrt();","highlight_start":13,"highlight_end":24}],"label":null,"suggested_replacement":"y.mul_add(y, x)","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: multiply and add expressions can be calculated more efficiently and accurately\n  --> tests/ui/floating_point_powi.fixed:12:13\n   |\nLL |     let _ = (x + y * y).sqrt();\n   |             ^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)`\n\n"}
{"message":"aborting due to 2 previous errors","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to 2 previous errors\n\n"}

[flip1995]

Just allow the other lint in this test file (or just for this test case).

[flip1995]

FYI: since this is a big PR, which seems to be under active development, I won't review every change here (At least as long as there is WIP in the title). @thiagoarrais if you have questions or want to have a review anyway, feel free to ask anytime though.

[thiagoarrais]

No problem! Thanks for the feedback!

[thiagoarrais]

I'm removing the WIP marker as this is ready for review.

Some of the proposed lints were already covered in master:

• x.logbase(E) => x.log()
• x.logbase(10) => x.log10()
• x.logbase(2) => x.log2()
• (x + 1).log() => x.log_1p()

And here is a summary of which lint went into each group. It wasn't really clear to me what should go where. Please let me know if any of these are in the wrong drawer.

Lint	Lint group
x.powi(2) => x * x	imprecise_flops
x.logN() / y.logN() => x.logbase(y)	suboptimal_flops
x * PI / 180 => x.to_radians()	imprecise_flops
x * 180 / PI => x.to_degrees()	imprecise_flops
sqrt(x * x + y * y) => x.hypot(y)	imprecise_flops

[thiagoarrais]

Added TODO comment and rebased. This should be good to go.

[flip1995]

@bors r+

[bors]

📌 Commit 3065201 has been approved by flip1995

[bors]

⌛ Testing commit 3065201 with merge 3749ea4...

[flip1995]

@bors rollup=iffy

[flip1995]

@bors retry (yeet)

[bors]

⌛ Testing commit 3065201 with merge 52f7940...

[flip1995]

@bors retry (yeet yeet)

[bors]

⌛ Testing commit 3065201 with merge 4b87008...

[bors]

☔ The latest upstream changes (presumably #5792) made this pull request unmergeable. Please resolve the merge conflicts.