unchecked_{shl|shr} should use u32 as the RHS#103456
Conversation
rustbot
added
the
T-libs
|
(rust-highfive has picked a reviewer for you, use r? to override) |
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
rust-highfive
added
the
S-waiting-on-review
src/test/codegen/unchecked_shifts.rs
Outdated
There was a problem hiding this comment.
annot: the LLVM instruction always gives poison by default for too-high shift amounts (https://llvm.org/docs/LangRef.html#shl-instruction), so no extra flags are expected in the IR.
library/core/src/num/int_macros.rs
Outdated
There was a problem hiding this comment.
annot: this was previously using the intrinsic on stable, so the rustc_allow_const_fn_unstable isn't exposing anything new to stable here.
library/core/src/num/int_macros.rs
Outdated
There was a problem hiding this comment.
annot: the .ok() looks silly here, but Result::unwrap_unchecked isn't const, whereas Option::unwrap_unchecked is const.
scottmcm
force-pushed
the
fix-unchecked-shifts
branch
from
debadbc to
3b16c04
Compare
scottmcm
added
T-libs-api
scottmcm
added
the
I-libs-api-nominated
|
Hi libs-api! I'm nominating this one to get a 👍/👎 on the signature change, in hopes of then being able to move it to a less-busy reviewer once the API decision is made. |
|
@bors r+ |
bors
added
S-waiting-on-bors
m-ou-se
removed
the
I-libs-api-nominated
…-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
|
@bors rollup=iffy (failed in a rollup once, but with the update that config worked in CI) Failed in rollup https://github.com/rust-lang-ci/rust/actions/runs/3477142652/jobs/5813017785 |
bors
added
S-waiting-on-author
rustbot
added
the
A-testsuite
rustbot
added
the
T-infra
scottmcm
force-pushed
the
fix-unchecked-shifts
branch
2 times, most recently
from
1e783ac to
9d4b1f9
Compare
|
Did the test-in-CI trick, and it doesn't fail that config any more: https://github.com/rust-lang/rust/actions/runs/3484161910/jobs/5828408349 @bors r+ |
|
🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened. |
bors
added
S-waiting-on-bors
…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
…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
…earth Rollup of 8 pull requests Successful merges: - rust-lang#102977 (remove HRTB from `[T]::is_sorted_by{,_key}`) - rust-lang#103378 (Fix mod_inv termination for the last iteration) - rust-lang#103456 (`unchecked_{shl|shr}` should use `u32` as the RHS) - rust-lang#103701 (Simplify some pointer method implementations) - rust-lang#104047 (Diagnostics `icu4x` based list formatting.) - rust-lang#104338 (Enforce that `dyn*` coercions are actually pointer-sized) - rust-lang#104498 (Edit docs for `rustc_errors::Handler::stash_diagnostic`) - rust-lang#104556 (rustdoc: use `code-header` class to format enum variants) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
6 participants
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
u32for the shift amount. That's consistent with other things, likecount_ones, which also always useu32for a bit count, regardless of the size of the type.This PR changes
unchecked_shlandunchecked_shrto also useu32for the shift amount (rather than Self).cc #85122, the
unchecked_mathtracking issue