Skip to content
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

Clean up unchecked_math, separate out unchecked_shifts #115626

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

clarfonthey
Copy link
Contributor

Tracking issue: #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.

@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2023

r? @thomcc

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

@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 Sep 7, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2023

The Miri subtree was changed

cc @rust-lang/miri

@RalfJung
Copy link
Member

RalfJung commented Sep 7, 2023

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.

@clarfonthey
Copy link
Contributor Author

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
Copy link
Member

RalfJung commented Sep 7, 2023

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

@clarfonthey
Copy link
Contributor Author

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

@RalfJung
Copy link
Member

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

@thomcc
Copy link
Member

thomcc commented Nov 1, 2023

@bors r+ rollup

sorry for the delay

@bors
Copy link
Contributor

bors commented Nov 1, 2023

📌 Commit 91405ab has been approved by thomcc

It is now in the queue for this repository.

@bors 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 added a commit to matthiaskrgr/rust that referenced this pull request Nov 1, 2023
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.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 1, 2023
…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
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 1, 2023
…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
@bors bors merged commit 260e07b into rust-lang:master Nov 1, 2023
11 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Nov 1, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 1, 2023
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.
@clarfonthey clarfonthey deleted the unchecked-math branch November 1, 2023 16:16
bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Nov 2, 2023
80: Automated pull from upstream `master` r=tshepang a=github-actions[bot]


This PR pulls the following changes from the upstream repository:

* rust-lang/rust#117498
  * rust-lang/rust#117488
  * rust-lang/rust#117441
  * rust-lang/rust#117373
  * rust-lang/rust#117298
* rust-lang/rust#117029
* rust-lang/rust#117289
* rust-lang/rust#117307
* rust-lang/rust#114208
* rust-lang/rust#117482
  * rust-lang/rust#117475
  * rust-lang/rust#117401
  * rust-lang/rust#117397
  * rust-lang/rust#115626
* rust-lang/rust#117436
* rust-lang/rust#115356
* rust-lang/rust#117422
* rust-lang/rust#116692



Co-authored-by: David CARLIER <devnexen@gmail.com>
Co-authored-by: Taiki Endo <te316e89@gmail.com>
Co-authored-by: ltdk <usr@ltdk.xyz>
Co-authored-by: Ryan Mehri <ryan.mehri1@gmail.com>
zhassan-aws added a commit to model-checking/kani that referenced this pull request Nov 6, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

5 participants