Skip to content

feat(manual_saturating_arithmetic): lint x.checked_sub(y).unwrap_or_default() #15845

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(manual_saturating_arithmetic): lint x.checked_sub(y).unwrap_or_default() #15845

wants to merge 2 commits into from

Conversation

ada4a
Copy link
Contributor

@ada4a ada4a commented Oct 8, 2025 •
edited
Loading

Resolves #15655

changelog: [manual_saturating_arithmetic]: lint x.checked_sub(y).unwrap_or_default()

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 8, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 8, 2025

r? @Alexendoo

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@ada4a ada4a force-pushed the 15655-manual_saturating_arithmetic branch from ff8d1fc to 583af98 Compare October 8, 2025 16:00
@ada4a ada4a marked this pull request as draft October 8, 2025 16:12
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 8, 2025
@ada4a ada4a marked this pull request as ready for review October 8, 2025 16:13
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 8, 2025
Alexendoo
Alexendoo reviewed Oct 20, 2025
View reviewed changes
clippy_lints/src/methods/manual_saturating_arithmetic.rs Outdated
Comment on lines 85 to 86
use self::MinMax::{Max, Min};
use CheckedArith::{Add, Mul, Sub};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say move these upwards rather than duplicating

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to add a block just to keep the scoping a bit cleaner.. But maybe it'd be better not to bother

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I wouldn't say the block is needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, removed it

@Alexendoo
Copy link
Member

The commit/PR title & description should be updated to say x.checked_sub(y).unwrap_or_default() rather than x.saturating_sub(y).unwrap_or_default()

@ada4a ada4a changed the title feat(manual_saturating_arithmetic): lint x.saturating_sub(y).unwrap_or_default() feat(manual_saturating_arithmetic): lint x.checked_sub(y).unwrap_or_default() Oct 20, 2025
@ada4a ada4a force-pushed the 15655-manual_saturating_arithmetic branch from 583af98 to 04502af Compare October 20, 2025 18:21
@rustbot
Copy link
Collaborator

rustbot commented Oct 20, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@ada4a ada4a force-pushed the 15655-manual_saturating_arithmetic branch from 04502af to 95834c9 Compare October 20, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Alexendoo Alexendoo Alexendoo left review comments

Assignees

@Alexendoo Alexendoo

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

suboptimal suggestion for match of checked math

3 participants

@ada4a @rustbot @Alexendoo