Skip to content

Commit 1dc7f5c

Browse files
committed
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`
2 parents 5cb9833 + 327c91f commit 1dc7f5c

File tree

10 files changed

+258
-2
lines changed

10 files changed

+258
-2
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,7 @@ Released 2018-09-13
10631063
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
10641064
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
10651065
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
1066+
[`manual_mul_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
10661067
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
10671068
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
10681069
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

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

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

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

clippy_lints/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ pub mod misc_early;
224224
pub mod missing_const_for_fn;
225225
pub mod missing_doc;
226226
pub mod missing_inline;
227+
pub mod mul_add;
227228
pub mod multiple_crate_versions;
228229
pub mod mut_mut;
229230
pub mod mut_reference;
@@ -604,6 +605,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
604605
reg.register_late_lint_pass(box inherent_to_string::InherentToString);
605606
reg.register_late_lint_pass(box trait_bounds::TraitBounds);
606607
reg.register_late_lint_pass(box comparison_chain::ComparisonChain);
608+
reg.register_late_lint_pass(box mul_add::MulAddCheck);
607609

608610
reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
609611
arithmetic::FLOAT_ARITHMETIC,
@@ -836,6 +838,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
836838
misc_early::UNNEEDED_FIELD_PATTERN,
837839
misc_early::UNNEEDED_WILDCARD_PATTERN,
838840
misc_early::ZERO_PREFIXED_LITERAL,
841+
mul_add::MANUAL_MUL_ADD,
839842
mut_reference::UNNECESSARY_MUT_PASSED,
840843
mutex_atomic::MUTEX_ATOMIC,
841844
needless_bool::BOOL_COMPARISON,
@@ -1173,6 +1176,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
11731176
methods::OR_FUN_CALL,
11741177
methods::SINGLE_CHAR_PATTERN,
11751178
misc::CMP_OWNED,
1179+
mul_add::MANUAL_MUL_ADD,
11761180
mutex_atomic::MUTEX_ATOMIC,
11771181
redundant_clone::REDUNDANT_CLONE,
11781182
slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,

clippy_lints/src/mul_add.rs

+106
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
use rustc::hir::*;
2+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
3+
use rustc::{declare_lint_pass, declare_tool_lint};
4+
use rustc_errors::Applicability;
5+
6+
use crate::utils::*;
7+
8+
declare_clippy_lint! {
9+
/// **What it does:** Checks for expressions of the form `a * b + c`
10+
/// or `c + a * b` where `a`, `b`, `c` are floats and suggests using
11+
/// `a.mul_add(b, c)` instead.
12+
///
13+
/// **Why is this bad?** Calculating `a * b + c` may lead to slight
14+
/// numerical inaccuracies as `a * b` is rounded before being added to
15+
/// `c`. Depending on the target architecture, `mul_add()` may be more
16+
/// performant.
17+
///
18+
/// **Known problems:** None.
19+
///
20+
/// **Example:**
21+
///
22+
/// ```rust
23+
/// # let a = 0_f32;
24+
/// # let b = 0_f32;
25+
/// # let c = 0_f32;
26+
/// let foo = (a * b) + c;
27+
/// ```
28+
///
29+
/// can be written as
30+
///
31+
/// ```rust
32+
/// # let a = 0_f32;
33+
/// # let b = 0_f32;
34+
/// # let c = 0_f32;
35+
/// let foo = a.mul_add(b, c);
36+
/// ```
37+
pub MANUAL_MUL_ADD,
38+
perf,
39+
"Using `a.mul_add(b, c)` for floating points has higher numerical precision than `a * b + c`"
40+
}
41+
42+
declare_lint_pass!(MulAddCheck => [MANUAL_MUL_ADD]);
43+
44+
fn is_float<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr) -> bool {
45+
cx.tables.expr_ty(expr).is_floating_point()
46+
}
47+
48+
// Checks whether expression is multiplication of two floats
49+
fn is_float_mult_expr<'a, 'tcx, 'b>(cx: &LateContext<'a, 'tcx>, expr: &'b Expr) -> Option<(&'b Expr, &'b Expr)> {
50+
if let ExprKind::Binary(op, lhs, rhs) = &expr.kind {
51+
if let BinOpKind::Mul = op.node {
52+
if is_float(cx, &lhs) && is_float(cx, &rhs) {
53+
return Some((&lhs, &rhs));
54+
}
55+
}
56+
}
57+
58+
None
59+
}
60+
61+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MulAddCheck {
62+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
63+
if let ExprKind::Binary(op, lhs, rhs) = &expr.kind {
64+
if let BinOpKind::Add = op.node {
65+
//Converts mult_lhs * mult_rhs + rhs to mult_lhs.mult_add(mult_rhs, rhs)
66+
if let Some((mult_lhs, mult_rhs)) = is_float_mult_expr(cx, lhs) {
67+
if is_float(cx, rhs) {
68+
span_lint_and_sugg(
69+
cx,
70+
MANUAL_MUL_ADD,
71+
expr.span,
72+
"consider using mul_add() for better numerical precision",
73+
"try",
74+
format!(
75+
"{}.mul_add({}, {})",
76+
snippet(cx, mult_lhs.span, "_"),
77+
snippet(cx, mult_rhs.span, "_"),
78+
snippet(cx, rhs.span, "_"),
79+
),
80+
Applicability::MaybeIncorrect,
81+
);
82+
}
83+
}
84+
//Converts lhs + mult_lhs * mult_rhs to mult_lhs.mult_add(mult_rhs, lhs)
85+
if let Some((mult_lhs, mult_rhs)) = is_float_mult_expr(cx, rhs) {
86+
if is_float(cx, lhs) {
87+
span_lint_and_sugg(
88+
cx,
89+
MANUAL_MUL_ADD,
90+
expr.span,
91+
"consider using mul_add() for better numerical precision",
92+
"try",
93+
format!(
94+
"{}.mul_add({}, {})",
95+
snippet(cx, mult_lhs.span, "_"),
96+
snippet(cx, mult_rhs.span, "_"),
97+
snippet(cx, lhs.span, "_"),
98+
),
99+
Applicability::MaybeIncorrect,
100+
);
101+
}
102+
}
103+
}
104+
}
105+
}
106+
}

src/lintlist/mod.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 319] = [
9+
pub const ALL_LINTS: [Lint; 320] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -938,6 +938,13 @@ pub const ALL_LINTS: [Lint; 319] = [
938938
deprecation: None,
939939
module: "loops",
940940
},
941+
Lint {
942+
name: "manual_mul_add",
943+
group: "perf",
944+
desc: "Using `a.mul_add(b, c)` for floating points has higher numerical precision than `a * b + c`",
945+
deprecation: None,
946+
module: "mul_add",
947+
},
941948
Lint {
942949
name: "manual_saturating_arithmetic",
943950
group: "style",

tests/ui/mul_add.rs

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#![warn(clippy::manual_mul_add)]
2+
#![allow(unused_variables)]
3+
4+
fn mul_add_test() {
5+
let a: f64 = 1234.567;
6+
let b: f64 = 45.67834;
7+
let c: f64 = 0.0004;
8+
9+
// Examples of not auto-fixable expressions
10+
let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
11+
let test2 = 1234.567 * 45.67834 + 0.0004;
12+
}
13+
14+
fn main() {
15+
mul_add_test();
16+
}

tests/ui/mul_add.stderr

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
error: consider using mul_add() for better numerical precision
2+
--> $DIR/mul_add.rs:10:17
3+
|
4+
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(a * b + c).mul_add((c + a * b), (c + (a * b) + c))`
6+
|
7+
= note: `-D clippy::manual-mul-add` implied by `-D warnings`
8+
9+
error: consider using mul_add() for better numerical precision
10+
--> $DIR/mul_add.rs:10:17
11+
|
12+
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
13+
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`
14+
15+
error: consider using mul_add() for better numerical precision
16+
--> $DIR/mul_add.rs:10:31
17+
|
18+
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
19+
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`
20+
21+
error: consider using mul_add() for better numerical precision
22+
--> $DIR/mul_add.rs:10:46
23+
|
24+
LL | let test1 = (a * b + c) * (c + a * b) + (c + (a * b) + c);
25+
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`
26+
27+
error: consider using mul_add() for better numerical precision
28+
--> $DIR/mul_add.rs:11:17
29+
|
30+
LL | let test2 = 1234.567 * 45.67834 + 0.0004;
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1234.567.mul_add(45.67834, 0.0004)`
32+
33+
error: aborting due to 5 previous errors
34+

tests/ui/mul_add_fixable.fixed

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::manual_mul_add)]
4+
#![allow(unused_variables)]
5+
6+
fn mul_add_test() {
7+
let a: f64 = 1234.567;
8+
let b: f64 = 45.67834;
9+
let c: f64 = 0.0004;
10+
11+
// Auto-fixable examples
12+
let test1 = a.mul_add(b, c);
13+
let test2 = a.mul_add(b, c);
14+
15+
let test3 = a.mul_add(b, c);
16+
let test4 = a.mul_add(b, c);
17+
18+
let test5 = a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c)) + c;
19+
let test6 = 1234.567_f64.mul_add(45.67834_f64, 0.0004_f64);
20+
}
21+
22+
fn main() {
23+
mul_add_test();
24+
}

tests/ui/mul_add_fixable.rs

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::manual_mul_add)]
4+
#![allow(unused_variables)]
5+
6+
fn mul_add_test() {
7+
let a: f64 = 1234.567;
8+
let b: f64 = 45.67834;
9+
let c: f64 = 0.0004;
10+
11+
// Auto-fixable examples
12+
let test1 = a * b + c;
13+
let test2 = c + a * b;
14+
15+
let test3 = (a * b) + c;
16+
let test4 = c + (a * b);
17+
18+
let test5 = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c;
19+
let test6 = 1234.567_f64 * 45.67834_f64 + 0.0004_f64;
20+
}
21+
22+
fn main() {
23+
mul_add_test();
24+
}

tests/ui/mul_add_fixable.stderr

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error: consider using mul_add() for better numerical precision
2+
--> $DIR/mul_add_fixable.rs:12:17
3+
|
4+
LL | let test1 = a * b + c;
5+
| ^^^^^^^^^ help: try: `a.mul_add(b, c)`
6+
|
7+
= note: `-D clippy::manual-mul-add` implied by `-D warnings`
8+
9+
error: consider using mul_add() for better numerical precision
10+
--> $DIR/mul_add_fixable.rs:13:17
11+
|
12+
LL | let test2 = c + a * b;
13+
| ^^^^^^^^^ help: try: `a.mul_add(b, c)`
14+
15+
error: consider using mul_add() for better numerical precision
16+
--> $DIR/mul_add_fixable.rs:15:17
17+
|
18+
LL | let test3 = (a * b) + c;
19+
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`
20+
21+
error: consider using mul_add() for better numerical precision
22+
--> $DIR/mul_add_fixable.rs:16:17
23+
|
24+
LL | let test4 = c + (a * b);
25+
| ^^^^^^^^^^^ help: try: `a.mul_add(b, c)`
26+
27+
error: consider using mul_add() for better numerical precision
28+
--> $DIR/mul_add_fixable.rs:18:17
29+
|
30+
LL | let test5 = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c;
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c))`
32+
33+
error: consider using mul_add() for better numerical precision
34+
--> $DIR/mul_add_fixable.rs:19:17
35+
|
36+
LL | let test6 = 1234.567_f64 * 45.67834_f64 + 0.0004_f64;
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1234.567_f64.mul_add(45.67834_f64, 0.0004_f64)`
38+
39+
error: aborting due to 6 previous errors
40+

0 commit comments

Comments
 (0)