Skip to content

Tracking Issue for exact_div #139911

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
1 of 4 tasks
newpavlov opened this issue Apr 16, 2025 · 10 comments
Open
1 of 4 tasks

Tracking Issue for exact_div #139911

newpavlov opened this issue Apr 16, 2025 · 10 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-unimplemented Status: The feature has not been implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@newpavlov
Copy link
Contributor

newpavlov commented Apr 16, 2025

Feature gate: #![feature(exact_div)]

This is a tracking issue for exact division methods (i.e. for division without remainder) on primitive integer types.

Public API

/// Checked integer division without remainder. Computes `self / rhs`,
/// returning `None` if `rhs == 0` or if division remainder is not zero.
pub const fn checked_exact_div(self, rhs: Self) -> Option<Self> { ... }

/// Integer division without remainder. Computes `self / rhs`,
/// panics if `rhs == 0` or if division remainder is not zero.
pub const fn exact_div(self, rhs: Self) -> Self { ... }

/// Integer division without remainder.
/// Unchecked version of `exact_div`.
pub unsafe const fn unchecked_exact_div(self, rhs: Self) -> Self { ... }

Steps / History

Unresolved Questions

@newpavlov newpavlov added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-unimplemented Status: The feature has not been implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 16, 2025
@hanna-kruppe
Copy link
Contributor

How do these handle overflow (iN::MIN / -1)?

@newpavlov
Copy link
Contributor Author

newpavlov commented Apr 16, 2025

They return None/panic/cause UB respectively, see here.

@hanna-kruppe
Copy link
Contributor

For the safe version this makes sense but for unchecked_exact_div it’s not obvious to me that UB is the right choice. Having three distinct preconditions to check (no remainder, no division by zero, no overflow) is not great because it’s easy to forget one, especially the overflow edge case. Even for unsigned types, the precedent in stable Rust is handling division by zero via impl Div<NonZeroT> for T instead of an unsafe method that’s UB on zero RHS. This may be useful in this case as well. And overflow in signed division could panic even if the “no remainder” portion is unchecked under threat of UB.

@newpavlov
Copy link
Contributor Author

The unsafe variant would be a simple wrapper around the exact_div intrinsic.

@hanna-kruppe
Copy link
Contributor

Just because the perma-unstable intrinsic works this way doesn’t mean it’s also the best option for a stable user-facing API. We also have an unchecked_div intrinsic but as noted in my last comment, it’s not exposed as-is.

@newpavlov
Copy link
Contributor Author

Any other behavior would be surprising. You could argue that we don't need unchecked_exact_div methods similarly to how we don't have unchecked_div, but there was an explicit interest in it during previous discussion. Personally, I think we should add unchecked_div methods instead.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Apr 16, 2025

I’m not here to argue about whether the functionality is needed, just point out that there’s other ways to expose it that may be better (easier to use correctly), at least for unsigned integers. Similarly , there’s no need to add unsafe unchecked_div methods for unsigned integers because the safe Div implementations already achieves the same effect (those impls use the unchecked_div intrinsic internally, and callers that need to unsafely assert that the RHS is non-zero can combine it with NonZero::new_unchecked), and unchecked_div for signed integers could also take a NonZero RHS and only require callers to check the “no overflow” condition.

@abgros
Copy link

abgros commented Apr 16, 2025

For the safe version this makes sense but for unchecked_exact_div it’s not obvious to me that UB is the right choice. Having three distinct preconditions to check (no remainder, no division by zero, no overflow) is not great because it’s easy to forget one, especially the overflow edge case.

No, there is only one unsafe precondition: a.exact_div_unchecked(b) is only allowed if there is a number c such that b * c == a (without overflow). The three cases are just a consequence of that.

An "unchecked" method panicking on some inputs would be quite surprising.

@hanna-kruppe
Copy link
Contributor

This “single” precondition leaves a third of the work to the “(without overflow)” and isn’t even correct: a = b = 0 is UB but c = 0 satisfies b * c = a.

In any case: even if there is some clever way to subsume all three conditions in a single one, that’s of little use to unsafe code authors if the generality makes it easy to overlook edge cases or difficult to connect to the concrete facts available at the call site.

@abgros
Copy link

abgros commented Apr 16, 2025

This “single” precondition leaves a third of the work to the “(without overflow)” and isn’t even correct: a = b = 0 is UB but c = 0 satisfies b * c = a.

You're right, it should be a unique number c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-unimplemented Status: The feature has not been implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants