Auto merge of #4602 - EthanTheMaster:issue-4001, r=flip1995

Add suggestion for mul_add

Issue #4001: Whenever `a*b+c` is found where `a`,`b`, and `c` are floats, a lint is suggested saying to use `a.mul_add(b, c)`. Using `mul_add` may give a performance boost depending on the target architecture and also has higher numerical accuracy as there is no round off when doing `a*b`.

changelog: New lint: `manual_mul_add`

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

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:
 

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,

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,
+                        );
+                    }
+                }
+            }
+        }
+    }
+}

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",

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();
+}

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
+

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();
+}

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();
+}

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
+