Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lints IMPLICIT_SATURATING_MUL and IMPLICIT_SATURATING_DIV #9606

Open
alex-semenyuk opened this issue Oct 8, 2022 · 3 comments
Open

Add lints IMPLICIT_SATURATING_MUL and IMPLICIT_SATURATING_DIV #9606

alex-semenyuk opened this issue Oct 8, 2022 · 3 comments
Labels
A-lint Area: New lints

Comments

@alex-semenyuk
Copy link
Contributor

alex-semenyuk commented Oct 8, 2022

What it does

Add lint similar to IMPLICIT_SATURATING_SUB and IMPLICIT_SATURATING_ADD, but for MUL and DIV.

Lint Name

IMPLICIT_SATURATING_MUL and IMPLICIT_SATURATING_DIV

Category

pedantic

Advantage

As mentioned at clippy::implicit_saturating_sub and clippy::implicit_saturating_add, the built-in function is more readable and may be faster.

Drawbacks

No response

Example

 let mut i: u32 = 3;
 i *= 2;

let mut i: u32 = 6;
i /= 2;

Could be written as:

let mut i: u32 = 3;
i = i.saturating_mul(2);

let mut i: u32 = 6;
i = i.saturating_div(2);
@alex-semenyuk alex-semenyuk added the A-lint Area: New lints label Oct 8, 2022
@llogiq
Copy link
Contributor

llogiq commented Oct 8, 2022

I don't see anything saturating in your "bad" examples. Why should we lint assigning ops that multiply or divide?

@alex-semenyuk
Copy link
Contributor Author

Thanks, fill the section about Advantage

@llogiq
Copy link
Contributor

llogiq commented Oct 8, 2022

I might misunderstand something, but not all multiplication and division should saturate by default.

Implementing saturating multiplication manually is quite complex. Division is far easier (one only needs to consider the divide by zero case, but even then the sign needs to be applied), but still non-trivial.

So what pattern should the lint check for?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants