rust-lang / rust-lang/rust-clippy

mul_add struggling with long expressions

Open
#4,735 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

C-bug I-suggestion-causes-error
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.