Skip to content

Conversation

@alex-semenyuk
Copy link
Member

@alex-semenyuk alex-semenyuk commented Apr 21, 2025

Close #14652

changelog: [integer_division]: fix false negative for NonZero denominators

@rustbot
Copy link
Collaborator

rustbot commented Apr 21, 2025

r? @llogiq

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

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

rustbot commented May 2, 2025

Failed to set assignee to Clippy: cannot assign: response: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/assignees#add-assignees-to-an-issue","status":"404"}

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@alex-semenyuk
Copy link
Member Author

r? clippy

@rustbot rustbot assigned blyxyas and unassigned llogiq May 2, 2025
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

Great bugfix, althought I have some light comments =^w^= 🐱

//~^ integer_division

let x = 1. / 2.0;

Copy link
Member

Choose a reason for hiding this comment

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

Just for future resilience, could you also add a test for NonZero / <integer> and NonZero / NonZero

Copy link
Member Author

Choose a reason for hiding this comment

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

Testing I see the following

error[E0369]: cannot divide std::num::NonZero<u32> by std::num::NonZero<u32>

the foreign item type std::num::NonZero<u32> doesn't implement std::ops::Div

and

error[E0369]: cannot divide std::num::NonZero<u32> by u32

 the foreign item type std::num::NonZero<u32> doesn't implement std::ops::Div<u32>

so I adjusted condition as well

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, although I guess that makes sense because the only scenario where absolutely making sure that an integer is non-zero would be in the denominator.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would also be unintuitive (and non-composable), because NonZero(1) / NonZero(2) is zero, so the returned type would always have to be the underlying integer.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 5, 2025
@alex-semenyuk alex-semenyuk force-pushed the integer_division_on_non_zero branch from 74a1494 to b940e64 Compare May 8, 2025 11:03
@alex-semenyuk alex-semenyuk force-pushed the integer_division_on_non_zero branch from b940e64 to 1fbfbb5 Compare May 8, 2025 11:12
@alex-semenyuk alex-semenyuk requested a review from blyxyas May 8, 2025 11:26
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 8, 2025
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks ❤️

@blyxyas blyxyas added this pull request to the merge queue May 9, 2025
Merged via the queue into rust-lang:master with commit 16fd2a8 May 9, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

integer_division false negative for NonZero denominators

5 participants