Skip to content

Conversation

@vrtgs
Copy link
Contributor

@vrtgs vrtgs commented Oct 14, 2025

Rationale

Let x = self and
m = (((1 as $Int) << (<$Int>::BITS - 1)).wrapping_shr(self.leading_zeros()))
Then the previous code computed NonZero::new_unchecked(x & m).
Since m has exactly one bit set (the most significant 1-bit of x), (x & m) == m.
Therefore, the masking step was redundant.

The shift is safe and does not need wrapping because:

  • self.leading_zeros() < $Int::BITS because self is non-zero.
  • The result of unchecked_shr is non-zero, satisfying the NonZero invariant. if wrapping happens we would be violating NonZero invariants.

why this micro optimization?
the old code was suboptimal it duplicated $Int’s isolate_highest_one logic instead of delegating to it. Since the type already wraps $Int, either delegation should be used for clarity or, if keeping a custom implementation, it should be optimized as above.

@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 Oct 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 14, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@joboet
Copy link
Member

joboet commented Oct 31, 2025

Thanks!
r=me after squashing commits
@bors delegate+
r? joboet

@rustbot rustbot assigned joboet and unassigned Mark-Simulacrum Oct 31, 2025
@bors
Copy link
Collaborator

bors commented Oct 31, 2025

✌️ @vrtgs, you can now approve this pull request!

If @joboet told you to "r=me" after making some further change, please make that change, then do @bors r=@joboet

@vrtgs vrtgs force-pushed the non-zero-isolate branch 2 times, most recently from 3cd7230 to 6d0a979 Compare November 7, 2025 21:21
@vrtgs
Copy link
Contributor Author

vrtgs commented Nov 7, 2025

@bors r=@joboet

@bors
Copy link
Collaborator

bors commented Nov 7, 2025

📌 Commit 057127c has been approved by joboet

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 7, 2025

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 7, 2025
@rust-log-analyzer
Copy link
Collaborator

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
spellchecking files
building external tool typos from package typos-cli@1.38.1
finished building tool typos
npm WARN deprecated puppeteer@22.15.0: < 24.10.2 is no longer supported
npm ERR! code 127
npm ERR! git dep preparation failed
npm ERR! command /node/bin/node /node/lib/node_modules/npm/bin/npm-cli.js install --force --cache=/home/user/.npm --prefer-offline=false --prefer-online=false --offline=false --no-progress --no-save --no-audit --include=dev --include=peer --include=optional --no-package-lock-only --no-dry-run
npm ERR! npm WARN using --force Recommended protections disabled.
npm ERR! npm WARN deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm ERR! npm WARN deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm ERR! npm WARN deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm ERR! npm WARN deprecated readdir-scoped-modules@1.1.0: This functionality has been moved to @npmcli/fs
npm ERR! npm WARN deprecated debuglog@1.0.1: Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.
npm ERR! npm WARN deprecated read-package-json@2.1.2: This package is no longer supported. Please use @npmcli/package-json instead.
npm ERR! npm WARN deprecated read-installed@4.0.3: This package is no longer supported.
npm ERR! npm ERR! code 127
npm ERR! npm ERR! path /home/user/.npm/_cacache/tmp/git-cloneXXXXXXbAD7op/node_modules/rollup
npm ERR! npm ERR! command failed
npm ERR! npm ERR! command sh -c patch-package
npm ERR! npm ERR! sh: 1: patch-package: not found
npm ERR! 
npm ERR! npm ERR! A complete log of this run can be found in: /home/user/.npm/_logs/2025-11-07T21_29_44_495Z-debug-0.log

npm ERR! A complete log of this run can be found in: /home/user/.npm/_logs/2025-11-07T21_29_36_831Z-debug-0.log
npm install did not exit successfully
tidy [extra_checks]: IO error: npm install returned exit code exit status: 127
tidy [extra_checks]: FAIL
tidy: The following check failed: extra_checks
Command `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools-bin/rust-tidy /checkout /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo /checkout/obj/build 4 /node/bin/npm --extra-checks=py,cpp,js,spellcheck` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:1549:23
Executed at: src/bootstrap/src/core/build_steps/test.rs:1279:29

Command has failed. Rerun with -v to see more details.
Bootstrap failed while executing `test src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck`
Build completed unsuccessfully in 0:03:06
  local time: Fri Nov  7 21:30:05 UTC 2025
  network time: Fri, 07 Nov 2025 21:30:05 GMT
##[error]Process completed with exit code 1.

jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 8, 2025
update isolate_highest_one for NonZero<T>

## Rationale
Let `x = self` and
`m = (((1 as $Int) << (<$Int>::BITS - 1)).wrapping_shr(self.leading_zeros()))`
Then the previous code computed `NonZero::new_unchecked(x & m)`.
Since `m` has exactly one bit set (the most significant 1-bit of `x`), `(x & m) == m`.
Therefore, the masking step was redundant.

The shift is safe and does not need wrapping because:
* `self.leading_zeros() < $Int::BITS` because `self` is non-zero.
* The result of `unchecked_shr` is non-zero, satisfying the `NonZero` invariant. if wrapping happens we would be violating `NonZero` invariants.

why this micro optimization?
the old code was suboptimal it duplicated `$Int`’s isolate_highest_one logic instead of delegating to it. Since the type already wraps `$Int`, either delegation should be used for clarity or, if keeping a custom implementation, it should be optimized as above.
bors added a commit that referenced this pull request Nov 8, 2025
Rollup of 15 pull requests

Successful merges:

 - #147404 (Fix issue with callsite inline attribute not being applied sometimes.)
 - #147534 (Implement SIMD funnel shifts in const-eval/Miri)
 - #147686 (update isolate_highest_one for NonZero<T>)
 - #148020 (Show backtrace on allocation failures when possible)
 - #148204 (Modify contributor email entries in .mailmap)
 - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
 - #148555 (Fix rust-by-example spanish translation)
 - #148556 (Fix suggestion for returning async closures)
 - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability)
 - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute)
 - #148612 (Add note for identifier with attempted hygiene violation)
 - #148613 (Switch hexagon targets to rust-lld)
 - #148644 ([bootstrap] Make `--open` option work with `doc src/tools/error_index_generator`)
 - #148649 (don't completely reset `HeadUsages`)
 - #148675 (Remove eslint-js from npm dependencies)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 8, 2025
update isolate_highest_one for NonZero<T>

## Rationale
Let `x = self` and
`m = (((1 as $Int) << (<$Int>::BITS - 1)).wrapping_shr(self.leading_zeros()))`
Then the previous code computed `NonZero::new_unchecked(x & m)`.
Since `m` has exactly one bit set (the most significant 1-bit of `x`), `(x & m) == m`.
Therefore, the masking step was redundant.

The shift is safe and does not need wrapping because:
* `self.leading_zeros() < $Int::BITS` because `self` is non-zero.
* The result of `unchecked_shr` is non-zero, satisfying the `NonZero` invariant. if wrapping happens we would be violating `NonZero` invariants.

why this micro optimization?
the old code was suboptimal it duplicated `$Int`’s isolate_highest_one logic instead of delegating to it. Since the type already wraps `$Int`, either delegation should be used for clarity or, if keeping a custom implementation, it should be optimized as above.
bors added a commit that referenced this pull request Nov 8, 2025
Rollup of 16 pull requests

Successful merges:

 - #147534 (Implement SIMD funnel shifts in const-eval/Miri)
 - #147686 (update isolate_highest_one for NonZero<T>)
 - #148020 (Show backtrace on allocation failures when possible)
 - #148204 (Modify contributor email entries in .mailmap)
 - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
 - #148279 (rustc_builtin_macros: rename bench parameter to avoid collisions with user-defined function names)
 - #148555 (Fix rust-by-example spanish translation)
 - #148556 (Fix suggestion for returning async closures)
 - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability)
 - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute)
 - #148612 (Add note for identifier with attempted hygiene violation)
 - #148613 (Switch hexagon targets to rust-lld)
 - #148619 (Enable std locking functions on AIX)
 - #148644 ([bootstrap] Make `--open` option work with `doc src/tools/error_index_generator`)
 - #148649 (don't completely reset `HeadUsages`)
 - #148675 (Remove eslint-js from npm dependencies)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Nov 8, 2025
Rollup of 10 pull requests

Successful merges:

 - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro)
 - #147024 (std_detect: Support run-time detection on OpenBSD using elf_aux_info)
 - #147534 (Implement SIMD funnel shifts in const-eval/Miri)
 - #147540 (Stabilise `as_array` in `[_]` and `*const [_]`; stabilise `as_mut_array` in `[_]` and `*mut [_]`.)
 - #147686 (update isolate_highest_one for NonZero<T>)
 - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
 - #148555 (Fix rust-by-example spanish translation)
 - #148556 (Fix suggestion for returning async closures)
 - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability)
 - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Nov 8, 2025
Rollup of 10 pull requests

Successful merges:

 - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro)
 - #147024 (std_detect: Support run-time detection on OpenBSD using elf_aux_info)
 - #147534 (Implement SIMD funnel shifts in const-eval/Miri)
 - #147540 (Stabilise `as_array` in `[_]` and `*const [_]`; stabilise `as_mut_array` in `[_]` and `*mut [_]`.)
 - #147686 (update isolate_highest_one for NonZero<T>)
 - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
 - #148555 (Fix rust-by-example spanish translation)
 - #148556 (Fix suggestion for returning async closures)
 - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability)
 - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Nov 9, 2025
Rollup of 10 pull requests

Successful merges:

 - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro)
 - #147024 (std_detect: Support run-time detection on OpenBSD using elf_aux_info)
 - #147534 (Implement SIMD funnel shifts in const-eval/Miri)
 - #147540 (Stabilise `as_array` in `[_]` and `*const [_]`; stabilise `as_mut_array` in `[_]` and `*mut [_]`.)
 - #147686 (update isolate_highest_one for NonZero<T>)
 - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
 - #148555 (Fix rust-by-example spanish translation)
 - #148556 (Fix suggestion for returning async closures)
 - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability)
 - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9ccff4a into rust-lang:master Nov 9, 2025
10 of 11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 9, 2025
rust-timer added a commit that referenced this pull request Nov 9, 2025
Rollup merge of #147686 - vrtgs:non-zero-isolate, r=joboet

update isolate_highest_one for NonZero<T>

## Rationale
Let `x = self` and
`m = (((1 as $Int) << (<$Int>::BITS - 1)).wrapping_shr(self.leading_zeros()))`
Then the previous code computed `NonZero::new_unchecked(x & m)`.
Since `m` has exactly one bit set (the most significant 1-bit of `x`), `(x & m) == m`.
Therefore, the masking step was redundant.

The shift is safe and does not need wrapping because:
* `self.leading_zeros() < $Int::BITS` because `self` is non-zero.
* The result of `unchecked_shr` is non-zero, satisfying the `NonZero` invariant. if wrapping happens we would be violating `NonZero` invariants.

why this micro optimization?
the old code was suboptimal it duplicated `$Int`’s isolate_highest_one logic instead of delegating to it. Since the type already wraps `$Int`, either delegation should be used for clarity or, if keeping a custom implementation, it should be optimized as above.
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