Skip to content

add exact bitshifts #144342

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Qelxiros
Copy link
Contributor

@Qelxiros Qelxiros commented Jul 23, 2025

Tracking issue: #144336

cc @lolbinarycat

@rustbot
Copy link
Collaborator

rustbot commented Jul 23, 2025

r? @tgross35

rustbot has assigned @tgross35.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 23, 2025
@kennytm
Copy link
Member

kennytm commented Jul 23, 2025

Commenting here to avoid cluttering the closed ACP or the tracking issue.

Regarding the names, we preferred calling these "exact" shifts just like exact_div

impl iX/uX {
    // Shift succeeds if `shift < Self::BITS` and right shifting the result returns the original value.
    fn exact_shl(self, shift: u32) -> Option<Self>;
    unsafe fn exact_shl_unchecked(self, shift: u32) -> Self;

If we're naming the methods after exact_div shouldn't there be 3 functions:

  • checked_exact_shl for the version returning Option (ref 1)
  • exact_shl for panicking (ref 2)
  • unchecked_exact_shl for the unchecked unsafe version (ref 3)

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. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants