diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fe97097fda4..7415c5fa2262 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/README.md b/README.md index 9a75d2ced080..aff8a36eca7c 100644 --- a/README.md +++ b/README.md @@ -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: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4300ed418333..452e4e9787db 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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; @@ -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, @@ -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, @@ -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, diff --git a/clippy_lints/src/mul_add.rs b/clippy_lints/src/mul_add.rs new file mode 100644 index 000000000000..02e403eee180 --- /dev/null +++ b/clippy_lints/src/mul_add.rs @@ -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, + ); + } + } + } + } + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 2a38c2968d20..8793d98f29f6 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -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", @@ -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", diff --git a/tests/ui/mul_add.rs b/tests/ui/mul_add.rs new file mode 100644 index 000000000000..1322e002c641 --- /dev/null +++ b/tests/ui/mul_add.rs @@ -0,0 +1,16 @@ +#![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; + + // 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(); +} diff --git a/tests/ui/mul_add.stderr b/tests/ui/mul_add.stderr new file mode 100644 index 000000000000..92c3b9e03c15 --- /dev/null +++ b/tests/ui/mul_add.stderr @@ -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 + diff --git a/tests/ui/mul_add_fixable.fixed b/tests/ui/mul_add_fixable.fixed new file mode 100644 index 000000000000..4af7c7e3e1a5 --- /dev/null +++ b/tests/ui/mul_add_fixable.fixed @@ -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(); +} diff --git a/tests/ui/mul_add_fixable.rs b/tests/ui/mul_add_fixable.rs new file mode 100644 index 000000000000..8b42f6f184a4 --- /dev/null +++ b/tests/ui/mul_add_fixable.rs @@ -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(); +} diff --git a/tests/ui/mul_add_fixable.stderr b/tests/ui/mul_add_fixable.stderr new file mode 100644 index 000000000000..123ab2ff100a --- /dev/null +++ b/tests/ui/mul_add_fixable.stderr @@ -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 +