rust-lang / rust-lang/rust-clippy
mul_add struggling with long expressions
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 13.5k
- Forks
- 2.2k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 32
Description
I've recently gotten the clippy lint for mul_add and ran into it with a cubic bezier calculation.
Currently the function looks like this:
(1.0 - x).powi(3) * p0
+ 3.0 * (1.0 - x).powi(2) * x * p1
+ 3.0 * (1.0 - x) * x.powi(2) * p2
+ x.powi(3) * p3
The initial lint for this function alone looks something like this:
warning: consider using mul_add() for better numerical precision
--> src/main.rs:27:5
|
27 | / (1.0 - x).powi(3) * p0
28 | | + 3.0 * (1.0 - x).powi(2) * x * p1
29 | | + 3.0 * (1.0 - x) * x.powi(2) * p2
30 | | + x.powi(3) * p3
| |________________________^
|
= note: `#[warn(clippy::manual_mul_add)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
help: try
|
27 | x.powi(3).mul_add(p3, (1.0 - x).powi(3) * p0
28 | + 3.0 * (1.0 - x).powi(2) * x * p1
29 | + 3.0 * (1.0 - x) * x.powi(2) * p2)
|
warning: consider using mul_add() for better numerical precision
--> src/main.rs:27:5
|
27 | / (1.0 - x).powi(3) * p0
28 | | + 3.0 * (1.0 - x).powi(2) * x * p1
29 | | + 3.0 * (1.0 - x) * x.powi(2) * p2
| |__________________________________________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
help: try
|
27 | 3.0 * (1.0 - x) * x.powi(2).mul_add(p2, (1.0 - x).powi(3) * p0
28 | + 3.0 * (1.0 - x).powi(2) * x * p1)
|
warning: consider using mul_add() for better numerical precision
--> src/main.rs:27:5
|
27 | / (1.0 - x).powi(3) * p0
28 | | + 3.0 * (1.0 - x).powi(2) * x * p1
| |__________________________________________^ help: try: `(1.0 - x).powi(3).mul_add(p0, 3.0 * (1.0 - x).powi(2) * x * p1)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
warning: consider using mul_add() for better numerical precision
--> src/main.rs:27:5
|
27 | / (1.0 - x).powi(3) * p0
28 | | + 3.0 * (1.0 - x).powi(2) * x * p1
| |__________________________________________^ help: try: `3.0 * (1.0 - x).powi(2) * x.mul_add(p1, (1.0 - x).powi(3) * p0)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
Finished dev [unoptimized + debuginfo] target(s) in 0.00s
Since there is a lot of multiplication and adding going on, the output is quite a bit to take in. However, following its suggestions can lead to incorrect code.
The first expression correctly recognizes the whole thing as a single expression (potentially by pure coincidence) and suggests a change that does not break things, since it respects the order of mathematical operations.
However, when looking at more of the output, it seems like the later suggestions only look at part of the operation. Especially when both multiplication and addition is involved at the same time on multiple levels, this can be dangerous. If I look at the output in my terminal and start at the bottom with the last thing output first, then I'll break the code.
The following code would be the result of applying the last suggestion:
// (1.0 - x).powi(3) * p0
// + 3.0 * (1.0 - x).powi(2) * x * p1
3.0 * (1.0 - x).powi(2) * x.mul_add(p1, (1.0 - x).powi(3) * p0)
//
+ 3.0 * (1.0 - x) * x.powi(2) * p2
+ x.powi(3) * p3
The issue with this is that performing a x.mul_add first, means that the other multiplications are done after the addition. With a single parenthesis, which fixes the suggestion, the issue is probably explained best:
// 3.0 * (1.0 - x).powi(2) * x.mul_add(p1, (1.0 - x).powi(3) * p0)
(3.0 * (1.0 - x).powi(2) * x).mul_add(p1, (1.0 - x).powi(3) * p0)
+ 3.0 * (1.0 - x) * x.powi(2) * p2
+ x.powi(3) * p3
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reproducing the cubic Bézier example with the manual_mul_add lint enabled and compare each suggested rewrite with the original operation grouping. Trace how the lint handles nested multiplication and addition, then ensure its suggestions preserve evaluation order and do not move multiplication inside mul_add; rerun the reproduction to verify every emitted suggestion remains equivalent.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100