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

Add suggestion for mul_add #4602

Merged
merged 1 commit into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,7 @@ Released 2018-09-13
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
[`manual_mul_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 319 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 320 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ pub mod misc_early;
pub mod missing_const_for_fn;
pub mod missing_doc;
pub mod missing_inline;
pub mod mul_add;
pub mod multiple_crate_versions;
pub mod mut_mut;
pub mod mut_reference;
Expand Down Expand Up @@ -604,6 +605,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
reg.register_late_lint_pass(box inherent_to_string::InherentToString);
reg.register_late_lint_pass(box trait_bounds::TraitBounds);
reg.register_late_lint_pass(box comparison_chain::ComparisonChain);
reg.register_late_lint_pass(box mul_add::MulAddCheck);

reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
arithmetic::FLOAT_ARITHMETIC,
Expand Down Expand Up @@ -836,6 +838,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
misc_early::UNNEEDED_FIELD_PATTERN,
misc_early::UNNEEDED_WILDCARD_PATTERN,
misc_early::ZERO_PREFIXED_LITERAL,
mul_add::MANUAL_MUL_ADD,
mut_reference::UNNECESSARY_MUT_PASSED,
mutex_atomic::MUTEX_ATOMIC,
needless_bool::BOOL_COMPARISON,
Expand Down Expand Up @@ -1173,6 +1176,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
methods::OR_FUN_CALL,
methods::SINGLE_CHAR_PATTERN,
misc::CMP_OWNED,
mul_add::MANUAL_MUL_ADD,
mutex_atomic::MUTEX_ATOMIC,
redundant_clone::REDUNDANT_CLONE,
slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
Expand Down
106 changes: 106 additions & 0 deletions clippy_lints/src/mul_add.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;

use crate::utils::*;

declare_clippy_lint! {
/// **What it does:** Checks for expressions of the form `a * b + c`
/// or `c + a * b` where `a`, `b`, `c` are floats and suggests using
/// `a.mul_add(b, c)` instead.
///
/// **Why is this bad?** Calculating `a * b + c` may lead to slight
/// numerical inaccuracies as `a * b` is rounded before being added to
/// `c`. Depending on the target architecture, `mul_add()` may be more
/// performant.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// # let a = 0_f32;
/// # let b = 0_f32;
/// # let c = 0_f32;
/// let foo = (a * b) + c;
/// ```
///
/// can be written as
///
/// ```rust
/// # let a = 0_f32;
/// # let b = 0_f32;
/// # let c = 0_f32;
/// let foo = a.mul_add(b, c);
/// ```
pub MANUAL_MUL_ADD,
perf,
"Using `a.mul_add(b, c)` for floating points has higher numerical precision than `a * b + c`"
}

declare_lint_pass!(MulAddCheck => [MANUAL_MUL_ADD]);

fn is_float<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr) -> bool {
cx.tables.expr_ty(expr).is_floating_point()
}

// Checks whether expression is multiplication of two floats
fn is_float_mult_expr<'a, 'tcx, 'b>(cx: &LateContext<'a, 'tcx>, expr: &'b Expr) -> Option<(&'b Expr, &'b Expr)> {
if let ExprKind::Binary(op, lhs, rhs) = &expr.kind {
if let BinOpKind::Mul = op.node {
if is_float(cx, &lhs) && is_float(cx, &rhs) {
return Some((&lhs, &rhs));
}
}
}

None
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MulAddCheck {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Binary(op, lhs, rhs) = &expr.kind {
if let BinOpKind::Add = op.node {
//Converts mult_lhs * mult_rhs + rhs to mult_lhs.mult_add(mult_rhs, rhs)
if let Some((mult_lhs, mult_rhs)) = is_float_mult_expr(cx, lhs) {
if is_float(cx, rhs) {
span_lint_and_sugg(
cx,
MANUAL_MUL_ADD,
expr.span,
"consider using mul_add() for better numerical precision",
"try",
format!(
"{}.mul_add({}, {})",
snippet(cx, mult_lhs.span, "_"),
snippet(cx, mult_rhs.span, "_"),
snippet(cx, rhs.span, "_"),
),
Applicability::MaybeIncorrect,
);
}
}
//Converts lhs + mult_lhs * mult_rhs to mult_lhs.mult_add(mult_rhs, lhs)
if let Some((mult_lhs, mult_rhs)) = is_float_mult_expr(cx, rhs) {
if is_float(cx, lhs) {
span_lint_and_sugg(
cx,
MANUAL_MUL_ADD,
expr.span,
"consider using mul_add() for better numerical precision",
"try",
format!(
"{}.mul_add({}, {})",
snippet(cx, mult_lhs.span, "_"),
snippet(cx, mult_rhs.span, "_"),
snippet(cx, lhs.span, "_"),
),
Applicability::MaybeIncorrect,
);
}
}
}
}
}
}
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 319] = [
pub const ALL_LINTS: [Lint; 320] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -938,6 +938,13 @@ pub const ALL_LINTS: [Lint; 319] = [
deprecation: None,
module: "loops",
},
Lint {
name: "manual_mul_add",
group: "perf",
desc: "Using `a.mul_add(b, c)` for floating points has higher numerical precision than `a * b + c`",
deprecation: None,
module: "mul_add",
},
Lint {
name: "manual_saturating_arithmetic",
group: "style",
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/mul_add.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![warn(clippy::manual_mul_add)]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#![warn(clippy::manual_mul_add)]
// run-rustfix
#![warn(clippy::manual_mul_add)]

I'm not sure if this will work, because some of the suggestions have to be applied one after another. If that's the case, just split up the test in a run-rustfix file and a non-run-rustfix file.

#![allow(unused_variables)]

fn mul_add_test() {
let a: f64 = 1234.567;
let b: f64 = 45.67834;
let c: f64 = 0.0004;

// Examples of not auto-fixable expressions
let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
let test2 = 1234.567 * 45.67834 + 0.0004;
}

fn main() {
mul_add_test();
}
34 changes: 34 additions & 0 deletions tests/ui/mul_add.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error: consider using mul_add() for better numerical precision
--> $DIR/mul_add.rs:10:17
|
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(a * b + c).mul_add((c + a * b), (c + (a * b) + c))`
|
= note: `-D clippy::manual-mul-add` implied by `-D warnings`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add.rs:10:17
|
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add.rs:10:31
|
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add.rs:10:46
|
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add.rs:11:17
|
LL | let test2 = 1234.567 * 45.67834 + 0.0004;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1234.567.mul_add(45.67834, 0.0004)`

error: aborting due to 5 previous errors

24 changes: 24 additions & 0 deletions tests/ui/mul_add_fixable.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix

#![warn(clippy::manual_mul_add)]
#![allow(unused_variables)]

fn mul_add_test() {
let a: f64 = 1234.567;
let b: f64 = 45.67834;
let c: f64 = 0.0004;

// Auto-fixable examples
let test1 = a.mul_add(b, c);
let test2 = a.mul_add(b, c);

let test3 = a.mul_add(b, c);
let test4 = a.mul_add(b, c);

let test5 = a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c)) + c;
let test6 = 1234.567_f64.mul_add(45.67834_f64, 0.0004_f64);
}

fn main() {
mul_add_test();
}
24 changes: 24 additions & 0 deletions tests/ui/mul_add_fixable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix

#![warn(clippy::manual_mul_add)]
#![allow(unused_variables)]

fn mul_add_test() {
let a: f64 = 1234.567;
let b: f64 = 45.67834;
let c: f64 = 0.0004;

// Auto-fixable examples
let test1 = a * b + c;
let test2 = c + a * b;

let test3 = (a * b) + c;
let test4 = c + (a * b);

let test5 = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c;
let test6 = 1234.567_f64 * 45.67834_f64 + 0.0004_f64;
}

fn main() {
mul_add_test();
}
40 changes: 40 additions & 0 deletions tests/ui/mul_add_fixable.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: consider using mul_add() for better numerical precision
--> $DIR/mul_add_fixable.rs:12:17
|
LL | let test1 = a * b + c;
| ^^^^^^^^^ help: try: `a.mul_add(b, c)`
|
= note: `-D clippy::manual-mul-add` implied by `-D warnings`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add_fixable.rs:13:17
|
LL | let test2 = c + a * b;
| ^^^^^^^^^ help: try: `a.mul_add(b, c)`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add_fixable.rs:15:17
|
LL | let test3 = (a * b) + c;
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add_fixable.rs:16:17
|
LL | let test4 = c + (a * b);
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add_fixable.rs:18:17
|
LL | let test5 = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c))`

error: consider using mul_add() for better numerical precision
--> $DIR/mul_add_fixable.rs:19:17
|
LL | let test6 = 1234.567_f64 * 45.67834_f64 + 0.0004_f64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1234.567_f64.mul_add(45.67834_f64, 0.0004_f64)`

error: aborting due to 6 previous errors