Skip to content

feat: detect == 0 on unsigned types as a manual_clamp lower bound - #16800

Merged
rustbot merged 1 commit into
rust-lang:masterfrom
LesnyRumcajs:clamp-clippy-unsigned
Jul 18, 2026
Merged

feat: detect == 0 on unsigned types as a manual_clamp lower bound#16800
rustbot merged 1 commit into
rust-lang:masterfrom
LesnyRumcajs:clamp-clippy-unsigned

Conversation

@LesnyRumcajs

Copy link
Copy Markdown
Contributor

Closes #16792

For unsigned types, if x == 0 can be expressed as if x < 1. The former was not caught by manual_clamp lint; this PR normalizes it so that cases as described in the linked issue are caught.

changelog: manual_clamp detect == 0 on unsigned types as a clamp lower bound

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 3, 2026
@rustbot

rustbot commented Apr 3, 2026

Copy link
Copy Markdown
Collaborator

r? @dswij

rustbot has assigned @dswij.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@blyxyas

blyxyas commented Jun 14, 2026

Copy link
Copy Markdown
Member

Hello @dswij and @LesnyRumcajs, I hope that you don't mind we using this contribution to teach @medzernik some things about reviewing (they were very interested!).

This is how most Clippy contributions are structured, some code (usually in between 50 - 200 lines), and then some tests are added.

Things to take into account when reviewing:

  • Try to break the contributor's code. It might sound cruel, but think that you're saving time from whoever will have to open the bug report, whoever will have to fix the bug, and whoever reviews that fix, in the future.
  • If you're not very familiarized with some feature of the language (like pattern bindings to wildcards, or lifetime shadowing through function boundaries), you can check the Rust Reference.
  • Remember to always test (or think how it would interact) with weird setups. How would a macro react? And a proc-macro? What about having this expression in an if let? What if the type implemented Copy + Clone but not Send? What about Sync + !Send?

Also, make sure to check the lintcheck results. And remember that you're not the only reviewing! We are fortunate in that, if Clippy fails, nobody dies! So there's no pressure to catch 100% of the bugs on the first try.

I'll also mention you in another 6-12 pull requests. My suggestion is that you go 1 by 1 and let me know if you have any questions.

@medzernik

Copy link
Copy Markdown

r? @medzernik

@rustbot rustbot assigned medzernik and unassigned dswij Jun 14, 2026
@blyxyas

blyxyas commented Jun 14, 2026

Copy link
Copy Markdown
Member

Hello @medzernik, I'm sorry for the misunderstanding, I thought you were talking about assigning me to your pull requests.

In fact, I didn't even know a non-member had permission to override an assignment like that. For this one we'll keep it, but please don't do it again 😅

@blyxyas blyxyas self-assigned this Jun 14, 2026
@medzernik

Copy link
Copy Markdown

Hello @medzernik, I'm sorry for the misunderstanding, I thought you were talking about assigning me to your pull requests.

In fact, I didn't even know a non-member had permission to override an assignment like that. For this one we'll keep it, but please don't do it again 😅

Oh. Oops. I did it on all 5 xD Sorry.. :D

@medzernik medzernik left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the PR looks ok? From what I've tried it works as it should and there are no edge-cases from my testing? :D Approved? :D

View changes since this review

@rustbot

This comment has been minimized.

@rustbot

rustbot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

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.

@rustbot

This comment has been minimized.

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jul 17, 2026
@LesnyRumcajs
LesnyRumcajs force-pushed the clamp-clippy-unsigned branch from e2c0873 to 900aad6 Compare July 17, 2026 15:25
@rustbot rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jul 17, 2026

@blyxyas blyxyas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for addressing this issue! ❤️ ฅ•ω•ฅ
Could you take care of CI before merging?

@LesnyRumcajs
LesnyRumcajs force-pushed the clamp-clippy-unsigned branch 2 times, most recently from 79ef17e to b3915d6 Compare July 17, 2026 15:49
@LesnyRumcajs
LesnyRumcajs force-pushed the clamp-clippy-unsigned branch from b3915d6 to 21a39ad Compare July 17, 2026 15:50
@LesnyRumcajs

Copy link
Copy Markdown
Contributor Author

@blyxyas It should be all green now! I thought there was just a minor import reshuffle conflict, but turned out is_integer_const got replaced with something else.

@blyxyas

blyxyas commented Jul 17, 2026

Copy link
Copy Markdown
Member

@medzernik do you want to merge this yourself?

@rustbot delegate=@medzernik

@rustbot

rustbot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

✌️ @medzernik, you can now merge this pull request!

If @blyxyas told you to merge after making some further change, then please make that change and post @rustbot merge.

@medzernik

Copy link
Copy Markdown

@rustbot merge

@rustbot
rustbot added this pull request to the merge queue Jul 18, 2026
Merged via the queue into rust-lang:master with commit 9b14e55 Jul 18, 2026
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

manual_clamp improvement for unsigned types

5 participants