Clean up unchecked_math, separate out unchecked_shifts #115626

clarfonthey · 2023-09-07T03:29:45Z

Tracking issue: #85122

Changes:

Remove const_inherent_unchecked_arith flag and make const-stability flags the same as the method feature flags. Given the number of other unsafe const fns already stabilised, it makes sense to just stabilise these in const context when they're stabilised.
Move unchecked_shl and unchecked_shr into a separate unchecked_shifts flag, since the semantics for them are unclear and they'll likely be stabilised separately as a result.
Add an unchecked_neg method exclusively to signed integers, under the unchecked_neg flag. This is because it's a new API and probably needs some time to marinate before it's stabilised, and while it would make sense to have a similar version for unsigned integers since checked_neg also exists for those there is absolutely no case where that would be a good idea, IMQHO.

The longer-term goal here is to prepare the unchecked_math methods for an FCP and stabilisation since they've existed for a while, their semantics are clear, and people seem in favour of stabilising them.

rustbot · 2023-09-07T03:29:52Z

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-09-07T03:29:55Z

The Miri subtree was changed

cc @rust-lang/miri

RalfJung · 2023-09-07T07:24:29Z

Move unchecked_shl and unchecked_shr into a separate unchecked_shifts flag, since the semantics for them are unclear and they'll likely be stabilised separately as a result.

What's unclear about them? Seems fairly clear to me: UB if shift amount exceeds bitwidth.

src/tools/miri/tests/fail/intrinsics/unchecked_shl.rs

clarfonthey · 2023-09-07T16:52:02Z

Move unchecked_shl and unchecked_shr into a separate unchecked_shifts flag, since the semantics for them are unclear and they'll likely be stabilised separately as a result.

What's unclear about them? Seems fairly clear to me: UB if shift amount exceeds bitwidth.

So, it wasn't mentioned in the tracking issue, but in the original PR, there was some mention of having a version that actually asserts that no overflow whatsoever occurs, rather than simply masking the bits: #85703 (comment)

RalfJung · 2023-09-07T19:53:12Z

Ah I see. That should concern be recorded in the tracking issue for the new feature as well then.

clarfonthey · 2023-09-07T22:31:14Z

You are right! And it seems to be listed in the description for it, although sneakily only there.

RalfJung · 2023-10-13T06:21:02Z

@thomcc friendly reminder that this is waiting for your review. :)

thomcc · 2023-11-01T01:52:13Z

@bors r+ rollup

sorry for the delay

bors · 2023-11-01T01:52:17Z

📌 Commit 91405ab has been approved by thomcc

It is now in the queue for this repository.

Clean up unchecked_math, separate out unchecked_shifts Tracking issue: rust-lang#85122 Changes: 1. Remove `const_inherent_unchecked_arith` flag and make const-stability flags the same as the method feature flags. Given the number of other unsafe const fns already stabilised, it makes sense to just stabilise these in const context when they're stabilised. 2. Move `unchecked_shl` and `unchecked_shr` into a separate `unchecked_shifts` flag, since the semantics for them are unclear and they'll likely be stabilised separately as a result. 3. Add an `unchecked_neg` method exclusively to signed integers, under the `unchecked_neg` flag. This is because it's a new API and probably needs some time to marinate before it's stabilised, and while it *would* make sense to have a similar version for unsigned integers since `checked_neg` also exists for those there is absolutely no case where that would be a good idea, IMQHO. The longer-term goal here is to prepare the `unchecked_math` methods for an FCP and stabilisation since they've existed for a while, their semantics are clear, and people seem in favour of stabilising them.

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#115626 (Clean up unchecked_math, separate out unchecked_shifts) - rust-lang#117029 (Add FileCheck annotations to MIR-opt inlining tests ) - rust-lang#117397 (Don't emit delayed good-path bugs on panic) - rust-lang#117401 (Refactor: move suggestion functions from demand to suggestions) - rust-lang#117475 (Inline and remove `create_session`.) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#115626 (Clean up unchecked_math, separate out unchecked_shifts) - rust-lang#117397 (Don't emit delayed good-path bugs on panic) - rust-lang#117401 (Refactor: move suggestion functions from demand to suggestions) - rust-lang#117475 (Inline and remove `create_session`.) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#115626 - clarfonthey:unchecked-math, r=thomcc Clean up unchecked_math, separate out unchecked_shifts Tracking issue: rust-lang#85122 Changes: 1. Remove `const_inherent_unchecked_arith` flag and make const-stability flags the same as the method feature flags. Given the number of other unsafe const fns already stabilised, it makes sense to just stabilise these in const context when they're stabilised. 2. Move `unchecked_shl` and `unchecked_shr` into a separate `unchecked_shifts` flag, since the semantics for them are unclear and they'll likely be stabilised separately as a result. 3. Add an `unchecked_neg` method exclusively to signed integers, under the `unchecked_neg` flag. This is because it's a new API and probably needs some time to marinate before it's stabilised, and while it *would* make sense to have a similar version for unsigned integers since `checked_neg` also exists for those there is absolutely no case where that would be a good idea, IMQHO. The longer-term goal here is to prepare the `unchecked_math` methods for an FCP and stabilisation since they've existed for a while, their semantics are clear, and people seem in favour of stabilising them.

Update to the latest Rust toolchain (2023-11-06). The relevant changes are: - rust-lang/rust#117507: this required changing the import of `Span` from `rustc_span::source_map::Span` to `rustc_span::Span`. - rust-lang/rust#114208: this changed the data field for the `OffsetOf` variant of `NullOp` from `List<FieldIdx>` to `List<(VariantIdx, FieldIdx)>`, which required updating the relevant code in `rvalue.rs`. - rust-lang/rust#115626: the unchecked shift operators have been separated from the `unchecked_math` feature, so this required changing the feature annotation in `tests/ui/should-panic-attribute/unexpected-failures/test.rs` - Some rustc change (not sure which one) result in a line in `tests/coverage/unreachable/variant/main.rs` getting optimized out. To maintain what this test is testing, I changed the `match` to make it a bit less-prone to optimization. - A change in `cargo` (rust-lang/cargo#12779) resulted in an update to Kani's workspace `Cargo.toml` when `cargo add` is executed inside `tests/script-based-pre/build-cache-bin`. This is apparently intended behavior, so I had to make the `exclude` in the `Cargo.toml` more specific to make sure this doesn't happen (I tried using a glob, but that didn't work, apparently because of rust-lang/cargo#6009. Resolves #2848 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

rustbot assigned thomcc Sep 7, 2023

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 Sep 7, 2023

RalfJung reviewed Sep 7, 2023

View reviewed changes

src/tools/miri/tests/fail/intrinsics/unchecked_shl.rs Show resolved Hide resolved

clarfonthey force-pushed the unchecked-math branch from 7a65982 to 10fb174 Compare September 16, 2023 05:37

Clean up unchecked_math, separate out unchecked_shifts

91405ab

clarfonthey force-pushed the unchecked-math branch from 10fb174 to 91405ab Compare October 13, 2023 06:17

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 1, 2023

matthiaskrgr mentioned this pull request Nov 1, 2023

Rollup of 5 pull requests #117481

Closed

matthiaskrgr mentioned this pull request Nov 1, 2023

Rollup of 4 pull requests #117482

Merged

bors merged commit 260e07b into rust-lang:master Nov 1, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Nov 1, 2023

clarfonthey deleted the unchecked-math branch November 1, 2023 16:16

github-actions bot mentioned this pull request Nov 2, 2023

Automated pull from upstream master ferrocene/ferrocene#80

Merged

zhassan-aws mentioned this pull request Nov 6, 2023

Update Rust toolchain to 2023-11-06 model-checking/kani#2858

Merged

clarfonthey mentioned this pull request May 22, 2024

Tracking Issue for inherent unchecked integer methods #85122

Open

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up unchecked_math, separate out unchecked_shifts #115626

Clean up unchecked_math, separate out unchecked_shifts #115626

clarfonthey commented Sep 7, 2023

rustbot commented Sep 7, 2023

rustbot commented Sep 7, 2023

RalfJung commented Sep 7, 2023

clarfonthey commented Sep 7, 2023

RalfJung commented Sep 7, 2023

clarfonthey commented Sep 7, 2023

RalfJung commented Oct 13, 2023

thomcc commented Nov 1, 2023

bors commented Nov 1, 2023

Clean up unchecked_math, separate out unchecked_shifts #115626

Clean up unchecked_math, separate out unchecked_shifts #115626

Conversation

clarfonthey commented Sep 7, 2023

rustbot commented Sep 7, 2023

rustbot commented Sep 7, 2023

RalfJung commented Sep 7, 2023

clarfonthey commented Sep 7, 2023

RalfJung commented Sep 7, 2023

clarfonthey commented Sep 7, 2023

RalfJung commented Oct 13, 2023

thomcc commented Nov 1, 2023

bors commented Nov 1, 2023