Tracking Issue for inherent unchecked integer methods #85122

clarfonthey · 2021-05-09T20:25:01Z

This is a tracking issue for the unchecked_* methods on integers.

`unchecked_math` (stable as of #122520)

impl uN {
    pub const unsafe fn unchecked_add(self, rhs: uN) -> uN;
    pub const unsafe fn unchecked_sub(self, rhs: uN) -> uN;
    pub const unsafe fn unchecked_mul(self, rhs: uN) -> uN;
}
impl iN {
    pub const unsafe fn unchecked_add(self, rhs: iN) -> iN;
    pub const unsafe fn unchecked_sub(self, rhs: iN) -> iN;
    pub const unsafe fn unchecked_mul(self, rhs: iN) -> iN;
}

Steps / History

`unchecked_neg`

impl iN {
    pub const unsafe fn unchecked_neg(self) -> iN;
}

Steps / History

`unchecked_shifts`

impl uN {
    pub const unsafe fn unchecked_shl(self, rhs: u32) -> uN;
    pub const unsafe fn unchecked_shr(self, rhs: u32) -> uN;
}
impl iN {
    pub const unsafe fn unchecked_shl(self, rhs: u32) -> iN;
    pub const unsafe fn unchecked_shr(self, rhs: u32) -> iN;
}

Steps / History

`unchecked_*` (aggregate)

impl uN {
    pub const unsafe fn unchecked_add(self, rhs: uN) -> uN;
    pub const unsafe fn unchecked_sub(self, rhs: uN) -> uN;
    pub const unsafe fn unchecked_mul(self, rhs: uN) -> uN;
    pub const unsafe fn unchecked_shl(self, rhs: u32) -> uN;
    pub const unsafe fn unchecked_shr(self, rhs: u32) -> uN;
}
impl iN {
    pub const unsafe fn unchecked_add(self, rhs: iN) -> iN;
    pub const unsafe fn unchecked_sub(self, rhs: iN) -> iN;
    pub const unsafe fn unchecked_mul(self, rhs: iN) -> iN;
    pub const unsafe fn unchecked_shl(self, rhs: u32) -> iN;
    pub const unsafe fn unchecked_shr(self, rhs: u32) -> iN;
    pub const unsafe fn unchecked_neg(self) -> iN;
}

Steps / History

Rework the std::iter::Step trait #69659 added unchecked_{add,sub,mul}
Make unchecked_{add,sub,mul} inherent methods unstably const #85096
Add inherent unchecked_shl, unchecked_shr to integers #85703
unchecked_{shl|shr} should use u32 as the RHS #103456
Clean up unchecked_math, separate out unchecked_shifts #115626 added unchecked_neg
Stabilize unchecked_{add,sub,mul} #122520
Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods #121571
Final commenting period (FCP)
Stabilization PR

Unresolved Questions

Should the other unchecked intrinsics like exact_div also get inherent versions?
From Add inherent unchecked_shl, unchecked_shr to integers #85703, consider if distinguishing the different kinds of ub could be useful (UB from overflow like MIN/-1 or MAX+1, LLVM's n[us]w; UB from input range like x/0 or x << -1; UB from lossy like 2/4 or 3 >> 1, LLVM's exact)

Resolved unresolved questions:

~~Is unchecked_* the best naming for these?~~ We stabilised unchecked_{add,sub,mul} already, so, yes.

The text was updated successfully, but these errors were encountered:

Add inherent unchecked_shl, unchecked_shr to integers Tracking issue: rust-lang#85122. Adding more of these methods, since these are missing.

jordens · 2021-11-22T16:50:15Z

Thanks for this! We are seeing some noticeably faster code in on several embedded DSP applications in cortex-m7 with unchecked_sh[lr]. The additional masking has always bothered me as unnecessary in our case.

jordens · 2021-11-22T18:44:44Z

One question on consistency and API choice though:

#85703 adds unchecked_sh[rl](self, rhs: Self) while all the other take a u32 as rhs: {overflowing,wrapping,checked}_sh[rl](self, rhs: u32). Why is that?

cmpute · 2022-04-26T20:00:11Z

I vote for adding exact_div into the library! It could be heavily used in integer factorization and other applications. I'm also developing a checked version of exact division in my crate. I think unchecked_exact_div or unchecked_div_exact will be good names. (I personally prefer div_exact over exact_div as it follows the pattern of div_euclid)

Besides, I'm also thinking about a shorter name for unchecked_, something like raw_add, raw_shl may be easier to type?

clarfonthey · 2022-05-01T20:48:41Z

I don't think that "raw" is a good naming scheme, since in general, you should not be using these primitives directly. In general, they should be hidden behind other methods that are safe to call. So, I would say that the "unchecked" naming is, at best, not descriptive enough, and that "raw" would be further from the goal of being descriptive.

scottmcm · 2022-10-24T00:36:27Z

@jordens Agreed the different RHS type is incorrect. I've sent PR #103456.

…-ou-se `unchecked_{shl|shr}` should use `u32` as the RHS The other shift methods, such as https://doc.rust-lang.org/nightly/std/primitive.u64.html#method.checked_shr and https://doc.rust-lang.org/nightly/std/primitive.i16.html#method.wrapping_shl, use `u32` for the shift amount. That's consistent with other things, like `count_ones`, which also always use `u32` for a bit count, regardless of the size of the type. This PR changes `unchecked_shl` and `unchecked_shr` to also use `u32` for the shift amount (rather than Self). cc rust-lang#85122, the `unchecked_math` tracking issue

…cottmcm `unchecked_{shl|shr}` should use `u32` as the RHS The other shift methods, such as https://doc.rust-lang.org/nightly/std/primitive.u64.html#method.checked_shr and https://doc.rust-lang.org/nightly/std/primitive.i16.html#method.wrapping_shl, use `u32` for the shift amount. That's consistent with other things, like `count_ones`, which also always use `u32` for a bit count, regardless of the size of the type. This PR changes `unchecked_shl` and `unchecked_shr` to also use `u32` for the shift amount (rather than Self). cc rust-lang#85122, the `unchecked_math` tracking issue

jdahlstrom · 2022-11-22T19:11:29Z

I was looking for an unchecked_div the other day, as I was trying to get rid of a superfluous zero test, and was surprised that one doesn't exist even under this feature. Any particular reason it's not included (as in, why is it an unresolved question whether there should be one?) Sure, one can use NonZero* as a divisor even on stable to get a safe unchecked div, but still.

scottmcm · 2022-11-22T23:14:45Z

I think that if there's a reason, it's that there two kinds of unchecked-ness in signed unchecked_div, and it'd be very easy to trigger the INT_MIN / -1 UB when you only remembered to prove that the denominator is non-zero.

I definitely prefer an unsafe type invariant to an unsafe operation where possible, so I think I like the Div<NonZeroTTT> better. After all, with #100428 I think it's x / NonZero::new_unchecked(y), which is very competitive with x.unchecked_div(y), especially if the producer of y can just change to reflect the non-zeroness of its result.

But it might fine to add unchecked_div under this gate and leave it a stabilization question to figure it out if it's worth keeping. We'll see what libs-api has to say.

(Notably, the "the addition overflows" kind of UB is the only kind of UB for unchecked_add, and it's also something that can't be feasibly pre-checked and thus put into a type invariant, the way "it's not zero" can be.)

ojeda · 2022-11-23T16:22:56Z

For the signed cases, I think it is good to have it for consistency with the other operations and to avoid having to play with unreachable_unchecked() (and hope it gets optimized away) or intrinsics (unstable). Or is there another way to do it in stable?

For the unsigned cases, I think it would still be good to offer it for consistency with the signed case and with the other operations, even if one can use NonZero* (perhaps documenting that as an alternative).

RalfJung · 2023-08-17T06:09:34Z

Are there any objections to stabilizing at least unchecked_{add,sub,mul}? For those there shouldn't be any surprises about what their safety requirements are.

clarfonthey · 2023-08-17T19:57:01Z

A unary unchecked_neg could probably also be added to that list.

scottmcm · 2023-08-18T04:10:06Z

@RalfJung That seems entirely reasonable to me. a.unchecked_add(b) is semantically a.checked_add(b).unwrap_or_else(hint::unreachable_unchecked), so I don't think there's any lang issues with it -- it's not a newly expressible concept, not even in const -- so I think a libs-api FCP and it'd be good to go.

Nominating it on behalf of Ralf to get libs-api's opinion. Would you want the FCP the whole thing, or FCP a PR that just does a subset? Or is there still skepticism about exposing these?

m-ou-se · 2023-09-05T14:16:22Z

We discussed this in a recent libs-api meeting. We mostly felt positive about stabilizing this, but it'd be helpful to have a clear overview of the exact APIs this tracking issue is tracking.

RalfJung · 2023-09-06T07:20:48Z

Great; does someone want to prepare a stabilization PR? I think that can cover everything that is currently under the unchecked_math feature gate; an unchecked_div function does not seem to exist (it only exists as an intrinsic, and we also have the exact_div intrinsic, but those will obviously not get stabilized).

A unary unchecked_neg could probably also be added to that list.

Such a function doesn't exist yet, so it would have to be added (in a separate PR) before it can be stabilized. This requires an ACP since it's a new public function. None of this is insurmountable but I think if someone wants to push for this they can do this in parallel to the effort of stabilizing the existing functions in the unchecked_math feature gate; there's no reason they have to all be stabilized at once.

clarfonthey · 2024-02-12T15:06:28Z

As an aside to specifically mention the status of adding unsafe checks to these methods, #117494 was stalled because of compiler errors I couldn't resolve, which lead to me focusing on #117571 to attempt to provide more info to said errors so I could debug them.

I haven't done any work on these PRs recently, but I'll try to rectify that this week, and if anyone else is interested in taking a look to maybe help out, I would appreciate it. I'm very unfamiliar with compiler internals, so, it's been a bit of a learning curve for me.

I'll also take a look about editing the docs while I'm at it.

clarfonthey · 2024-02-23T20:01:58Z

More up-to-date update: I decided to get around the compiler lint errors for now, and while it did require a bit of excursion for the shift operations in particular, #117494 should be ready to merge and will panic in debug mode when the unsafe assertions are ignored. That should hopefully help substantially with the concerns that people are using these methods incorrectly, since users without any additional setup to disable these assertions (who should not be doing that unless they know what they're doing) will run into errors in their tests when using these incorrectly.

I also don't think that we should care about users who are running into issues but not testing their code, because… well, yeah, that's how you verify your code is working!

…ions, r=<try> Add debug_assert_nounwind to unchecked_{add,sub,neg,mul,shl,shr} methods This ensures that these preconditions are actually checked in debug mode, and hopefully should let people know if they messed up. I've also replaced the calls (I could find) in the code that use these intrinsics directly with those that use these methods, so that the asserts actually apply. More discussions on people misusing these methods in the tracking issue: rust-lang#85122.

…ions, r=<try> Add debug_assert_nounwind to unchecked_{add,sub,neg,mul,shl,shr} methods (Old PR is haunted, opening a new one. See rust-lang#117494 for previous discussion.) This ensures that these preconditions are actually checked in debug mode, and hopefully should let people know if they messed up. I've also replaced the calls (I could find) in the code that use these intrinsics directly with those that use these methods, so that the asserts actually apply. More discussions on people misusing these methods in the tracking issue: rust-lang#85122.

the8472 · 2024-03-13T19:42:59Z

Unchecked variants of the {signed_integer}::checked_{add,sub}_unsigned functions and vice-versa could be useful if those properties can be conveyed to LLVM.

See #122420 and #122461

scottmcm · 2024-03-14T07:13:58Z

@the8472 If we get https://discourse.llvm.org/t/rfc-add-nowrap-flags-to-trunc/77453?u=scottmcm then I think it'd be possible to communicate it -- we could do sext + zext + add nuw nsw + trunc nsw -- but I suspect that it would end up getting optimized back down to a normal wrapping add.

Might help for smaller types, though, if it encourages LLVM to use a larger IV where the flags could work.

Though for usize+isize, there's actually a way: ptr::invalid+wrapping_offset+bare_addr.

(Dunno if that's anywhere close to being a good idea, though.)

scottmcm · 2024-03-14T23:03:45Z

FYI: I put up a PR (like Ralf mentioned in #85122 (comment)) to propose stabilizing just unchecked_{add,sub,mul}: #122520

(Along with some extra documentation text in them.)

EDIT: ...and it's in FCP! Thanks, Amanieu.

…sics, r=jhpratt Stabilize `unchecked_{add,sub,mul}` Tracking issue: rust-lang#85122 I think we might as well just stabilize these basic three. They're the ones that have `nuw`/`nsw` flags in LLVM. Notably, this doesn't include the potentially-more-complex or -more-situational things like `unchecked_neg` or `unchecked_shr` that are under different feature flags. To quote Ralf rust-lang#85122 (comment), > Are there any objections to stabilizing at least `unchecked_{add,sub,mul}`? For those there shouldn't be any surprises about what their safety requirements are. *Semantially* these are [already available on stable, even in `const`, via](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=bdb1ff889b61950897f1e9f56d0c9a36) `checked_*`+`unreachable_unchecked`. So IMHO we might as well just let people write them directly, rather than try to go through a `let Some(x) = x.checked_add(y) else { unsafe { hint::unreachable_unchecked() }};` dance. I added additional text to each method to attempt to better describe the behaviour and encourage `wrapping_*` instead. r? rust-lang/libs-api

…hpratt Stabilize `unchecked_{add,sub,mul}` Tracking issue: #85122 I think we might as well just stabilize these basic three. They're the ones that have `nuw`/`nsw` flags in LLVM. Notably, this doesn't include the potentially-more-complex or -more-situational things like `unchecked_neg` or `unchecked_shr` that are under different feature flags. To quote Ralf rust-lang/rust#85122 (comment), > Are there any objections to stabilizing at least `unchecked_{add,sub,mul}`? For those there shouldn't be any surprises about what their safety requirements are. *Semantially* these are [already available on stable, even in `const`, via](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=bdb1ff889b61950897f1e9f56d0c9a36) `checked_*`+`unreachable_unchecked`. So IMHO we might as well just let people write them directly, rather than try to go through a `let Some(x) = x.checked_add(y) else { unsafe { hint::unreachable_unchecked() }};` dance. I added additional text to each method to attempt to better describe the behaviour and encourage `wrapping_*` instead. r? rust-lang/libs-api

…ions, r=<try> Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods (Old PR is haunted, opening a new one. See rust-lang#117494 for previous discussion.) This ensures that these preconditions are actually checked in debug mode, and hopefully should let people know if they messed up. I've also replaced the calls (I could find) in the code that use these intrinsics directly with those that use these methods, so that the asserts actually apply. More discussions on people misusing these methods in the tracking issue: rust-lang#85122.

…hpratt Stabilize `unchecked_{add,sub,mul}` Tracking issue: #85122 I think we might as well just stabilize these basic three. They're the ones that have `nuw`/`nsw` flags in LLVM. Notably, this doesn't include the potentially-more-complex or -more-situational things like `unchecked_neg` or `unchecked_shr` that are under different feature flags. To quote Ralf rust-lang/rust#85122 (comment), > Are there any objections to stabilizing at least `unchecked_{add,sub,mul}`? For those there shouldn't be any surprises about what their safety requirements are. *Semantially* these are [already available on stable, even in `const`, via](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=bdb1ff889b61950897f1e9f56d0c9a36) `checked_*`+`unreachable_unchecked`. So IMHO we might as well just let people write them directly, rather than try to go through a `let Some(x) = x.checked_add(y) else { unsafe { hint::unreachable_unchecked() }};` dance. I added additional text to each method to attempt to better describe the behaviour and encourage `wrapping_*` instead. r? rust-lang/libs-api

…ions, r=<try> Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods (Old PR is haunted, opening a new one. See rust-lang#117494 for previous discussion.) This ensures that these preconditions are actually checked in debug mode, and hopefully should let people know if they messed up. I've also replaced the calls (I could find) in the code that use these intrinsics directly with those that use these methods, so that the asserts actually apply. More discussions on people misusing these methods in the tracking issue: rust-lang#85122.

clarfonthey · 2024-05-22T22:46:05Z

Cleaned up this issue a bit, separating out the various feature flags better and including more PRs that were omitted from the list. #121571 should hopefully be merged soon to ensure that all of these have unsafe preconditions checked in debug mode.

For now, we can keep this issue as an aggregate of all the unchecked methods, but if someone feels it'd be more useful separated into separate issues per feature flag, feel free to do so.

…ions, r=saethlin Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods (Old PR is haunted, opening a new one. See rust-lang#117494 for previous discussion.) This ensures that these preconditions are actually checked in debug mode, and hopefully should let people know if they messed up. I've also replaced the calls (I could find) in the code that use these intrinsics directly with those that use these methods, so that the asserts actually apply. More discussions on people misusing these methods in the tracking issue: rust-lang#85122.

clarfonthey · 2024-05-25T21:35:19Z

Worth an extra update: the unsafe preconditions PR is finally merged! Won't ping everyone who helped get this over the line here since there were quite a few, but this means that in the next nightly all unchecked arithmetic using the methods (not the intrinsics) should be checked in debug mode.

…aethlin Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods (Old PR is haunted, opening a new one. See #117494 for previous discussion.) This ensures that these preconditions are actually checked in debug mode, and hopefully should let people know if they messed up. I've also replaced the calls (I could find) in the code that use these intrinsics directly with those that use these methods, so that the asserts actually apply. More discussions on people misusing these methods in the tracking issue: rust-lang/rust#85122.

clarfonthey added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels May 9, 2021

clarfonthey mentioned this issue May 26, 2021

Add inherent unchecked_shl, unchecked_shr to integers #85703

Merged

the8472 mentioned this issue Oct 11, 2021

Missed optimization: codegen test repeat-trusted-len.rs fails when stdlib is compiled with overflow-checks=on #72549

Closed

JakobDegen mentioned this issue Dec 2, 2021

#![feature(unchecked_math)] emits slow asm #91449

Closed

utaal mentioned this issue Jan 26, 2022

Integer overflow panics in debug mode but is allowed by the verifier verus-lang/verus#45

Closed

dtolnay mentioned this issue Apr 20, 2022

Tracking Issue for nonzero_ops #84186

Open

8 tasks

iago-lito mentioned this issue May 30, 2022

Partial stabilization of "nonzero_checked_ops". #97547

Merged

3 tasks

scottmcm mentioned this issue Oct 24, 2022

unchecked_{shl|shr} should use u32 as the RHS #103456

Merged

scottmcm added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Aug 18, 2023

Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 5, 2023

geofmureithi mentioned this issue Sep 6, 2023

Math contract eigerco/nebula#55

Closed

ojeda mentioned this issue Feb 12, 2024

Disabling particular unsafe precondition checks under -Cdebug-assertions=y #120969

Open

clarfonthey mentioned this issue Feb 24, 2024

Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods #121571

Merged

scottmcm mentioned this issue Mar 14, 2024

Stabilize unchecked_{add,sub,mul} #122520

Merged

scottmcm mentioned this issue Apr 24, 2024

Pre-RFC: Unchecked arithmetic rust-lang/rfcs#2508

Closed

sffc mentioned this issue Jul 26, 2024

Remove _manual_slice constructors unicode-org/icu4x#5312

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tracking Issue for inherent unchecked integer methods #85122

Tracking Issue for inherent unchecked integer methods #85122

clarfonthey commented May 9, 2021 •

edited

Loading

jordens commented Nov 22, 2021

jordens commented Nov 22, 2021 •

edited

Loading

cmpute commented Apr 26, 2022 •

edited

Loading

clarfonthey commented May 1, 2022

scottmcm commented Oct 24, 2022

jdahlstrom commented Nov 22, 2022 •

edited

Loading

scottmcm commented Nov 22, 2022

ojeda commented Nov 23, 2022

RalfJung commented Aug 17, 2023 •

edited

Loading

clarfonthey commented Aug 17, 2023

scottmcm commented Aug 18, 2023 •

edited

Loading

m-ou-se commented Sep 5, 2023

RalfJung commented Sep 6, 2023

clarfonthey commented Feb 12, 2024 •

edited

Loading

clarfonthey commented Feb 23, 2024 •

edited

Loading

the8472 commented Mar 13, 2024 •

edited

Loading

scottmcm commented Mar 14, 2024 •

edited

Loading

scottmcm commented Mar 14, 2024 •

edited

Loading

clarfonthey commented May 22, 2024 •

edited

Loading

clarfonthey commented May 25, 2024

Tracking Issue for inherent unchecked integer methods #85122

Tracking Issue for inherent unchecked integer methods #85122

Comments

clarfonthey commented May 9, 2021 • edited Loading

unchecked_math (stable as of #122520)

Steps / History

unchecked_neg

Steps / History

unchecked_shifts

Steps / History

unchecked_* (aggregate)

Steps / History

Unresolved Questions

jordens commented Nov 22, 2021

jordens commented Nov 22, 2021 • edited Loading

cmpute commented Apr 26, 2022 • edited Loading

clarfonthey commented May 1, 2022

scottmcm commented Oct 24, 2022

jdahlstrom commented Nov 22, 2022 • edited Loading

scottmcm commented Nov 22, 2022

ojeda commented Nov 23, 2022

RalfJung commented Aug 17, 2023 • edited Loading

clarfonthey commented Aug 17, 2023

scottmcm commented Aug 18, 2023 • edited Loading

m-ou-se commented Sep 5, 2023

RalfJung commented Sep 6, 2023

clarfonthey commented Feb 12, 2024 • edited Loading

clarfonthey commented Feb 23, 2024 • edited Loading

the8472 commented Mar 13, 2024 • edited Loading

scottmcm commented Mar 14, 2024 • edited Loading

scottmcm commented Mar 14, 2024 • edited Loading

clarfonthey commented May 22, 2024 • edited Loading

clarfonthey commented May 25, 2024

clarfonthey commented May 9, 2021 •

edited

Loading

`unchecked_math` (stable as of #122520)

`unchecked_neg`

`unchecked_shifts`

`unchecked_*` (aggregate)

jordens commented Nov 22, 2021 •

edited

Loading

cmpute commented Apr 26, 2022 •

edited

Loading

jdahlstrom commented Nov 22, 2022 •

edited

Loading

RalfJung commented Aug 17, 2023 •

edited

Loading

scottmcm commented Aug 18, 2023 •

edited

Loading

clarfonthey commented Feb 12, 2024 •

edited

Loading

clarfonthey commented Feb 23, 2024 •

edited

Loading

the8472 commented Mar 13, 2024 •

edited

Loading

scottmcm commented Mar 14, 2024 •

edited

Loading

scottmcm commented Mar 14, 2024 •

edited

Loading

clarfonthey commented May 22, 2024 •

edited

Loading