Skip to content

Conversation

eduardosm
Copy link
Contributor

Some masks where defined as

const NONASCII_MASK: usize = 0x80808080_80808080u64 as usize;

where it was assumed that usize is never wider than 64, which is currently true.

To make those constants valid in a hypothetical 128-bit target, these constants have been redefined in an usize-width-agnostic way

const NONASCII_MASK: usize = usize::from_ne_bytes([0x80; size_of::<usize>()]);

There are already some cases where Rust anticipates the possibility of supporting 128-bit targets, such as not implementing From<usize> for u64.

Some masks where defined as
```rust
const NONASCII_MASK: usize = 0x80808080_80808080u64 as usize;
```
where it was assumed that `usize` is never wider than 64, which is currently true.

To make those constants valid in a hypothetical 128-bit target, these constants have been redefined in an `usize`-width-agnostic way
```rust
const NONASCII_MASK: usize = usize::from_ne_bytes([0x80; size_of::<usize>()]);
```

There are already some cases where Rust anticipates the possibility of supporting 128-bit targets, such as not implementing `From<usize>` for `u64`.
@rust-highfive
Copy link
Contributor

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • a stabilization of a library feature
  • introducing new or changes existing unstable library APIs
  • changes to public documentation in ways that create new stability guarantees

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 15, 2022
@rust-highfive
Copy link
Contributor

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 15, 2022
@eduardosm eduardosm changed the title Make some usize-typed masks definition agnostic to the size of usize Make some usize-typed masks definitions agnostic to the size of usize Apr 15, 2022
@yaahc
Copy link
Member

yaahc commented Apr 15, 2022

Sounds good and looks great, thank you for the PR!

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 15, 2022

📌 Commit 93ae6f8 has been approved by yaahc

@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 Apr 15, 2022
@@ -77,6 +77,6 @@ fn is_ascii_align_to_unrolled(bytes: &[u8]) -> bool {

#[inline]
fn contains_nonascii(v: usize) -> bool {
const NONASCII_MASK: usize = 0x80808080_80808080u64 as usize;
const NONASCII_MASK: usize = usize::from_ne_bytes([0x80; core::mem::size_of::<usize>()]);
Copy link
Member

@RalfJung RalfJung Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this use usize::repeat_u8?

EDIT: Oh, I guess that is crate-private.

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#95887 (resolve: Create dummy bindings for all unresolved imports)
 - rust-lang#96023 (couple of clippy::perf fixes)
 - rust-lang#96035 (Update GitHub Actions actions/checkout Version v2 -> v3)
 - rust-lang#96038 (docs: add link from zip to unzip)
 - rust-lang#96047 (:arrow_up: rust-analyzer)
 - rust-lang#96059 (clarify doc(cfg) wording)
 - rust-lang#96081 (Make some `usize`-typed masks definitions agnostic to the size of `usize`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4ed7627 into rust-lang:master Apr 16, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 16, 2022
@eduardosm eduardosm deleted the masks_usize_size_agnostic branch April 16, 2022 17:02
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.

6 participants