Skip to content

Conversation

@scottmcm
Copy link
Member

@scottmcm scottmcm commented Dec 10, 2025

Inspired by #general > `Source` link for `core` items is often inscrutable @ 💬 I wanted to add some more examples of the actual wrapping as well as update the documentation to emphasize that the behaviour is unusual.

In particular, now that unbounded_sh[lr] is stable, point people trying to avoid panics to that instead, since it behaves less weirdly.

Rendered example:
image

@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 Dec 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 10, 2025

r? @workingjubilee

rustbot has assigned @workingjubilee.
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

@scottmcm scottmcm force-pushed the wrapping-shift-docs branch from fb36731 to 812137c Compare December 10, 2025 07:54
@scottmcm scottmcm force-pushed the wrapping-shift-docs branch from 812137c to ff43366 Compare December 10, 2025 08:23
#[doc = concat!("assert_eq!(42_", stringify!($SelfT), ".wrapping_shl(", stringify!($BITS), "), 42);")]
#[doc = concat!("assert_eq!(42_", stringify!($SelfT), ".wrapping_shl(1).wrapping_shl(", stringify!($BITS_MINUS_ONE), "), 0);")]
#[doc = concat!("assert_eq!((-1_", stringify!($SelfT), ").wrapping_shl(128), -1);")]
#[doc = concat!("assert_eq!(5_", stringify!($SelfT), ".wrapping_shl(1025), 10);")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to do some of these examples in binary (0b) or hex (0x), so one can see the bits move better

Copy link
Member

@workingjubilee workingjubilee Dec 10, 2025

Choose a reason for hiding this comment

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

Yeah, actually, demonstrating the shift from 0b0101 to 0b1010 sounds nice. Don't have to do it for all of them, ofc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call; I added a couple more basic examples to show the bits before the extreme examples.

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

/// then truncating as needed. The behaviour matches what shift instructions
/// do on many processors, and is what the `<<` operator does when overflow
/// checks are disabled, but numerically it's weird. Consider, instead,
/// using [`Self::unbounded_shl`] which has nicer behaviour.
Copy link
Member

Choose a reason for hiding this comment

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

Wait, hm.

Is "nicer" quite right? "More numerical" behavior? "As an integer"? "More logical"? "Is more similar to the mul-by-pow-2 operation you were looking for"?

@workingjubilee
Copy link
Member

Hmm, I think I've unsold myself on changing "nicer". We aren't that shy from opinion here, and docs contributors can wage that war as they see fit.

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 14, 2025

📌 Commit f9b830c has been approved by workingjubilee

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 Dec 14, 2025
@workingjubilee
Copy link
Member

@bors rollup

@scottmcm
Copy link
Member Author

Sounds good; happy to leave "nicer" there for now :)

Yeah, "weird" and "nicer" are admittedly kinda vague. Hopefully the new examples elaborate sufficiently, since I couldn't find a succinct way to say something like "mean that large shifts work like you'd expect from doing a bunch of smaller shifts that add up".

(I'll ponder doing a follow-up PR to say more in unbounded_sh[lr] too.)

bors added a commit that referenced this pull request Dec 14, 2025
Rollup of 8 pull requests

Successful merges:

 - #146794 (std: reorganize pipe implementations)
 - #148490 (dangling pointer from temp cleanup)
 - #149837 (Update `wrapping_sh[lr]` docs and examples)
 - #149864 (std: Don't use `linkat` on the `wasm32-wasi*` targets)
 - #149885 (replace addr_of_mut with &raw mut in maybeuninit docs)
 - #149949 (Cleanup of attribute parsing errors)
 - #149969 (don't use no_main and no_core to test IBT)
 - #149998 (miri subtree update)

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

Successful merges:

 - #146794 (std: reorganize pipe implementations)
 - #148490 (dangling pointer from temp cleanup)
 - #149837 (Update `wrapping_sh[lr]` docs and examples)
 - #149864 (std: Don't use `linkat` on the `wasm32-wasi*` targets)
 - #149885 (replace addr_of_mut with &raw mut in maybeuninit docs)
 - #149949 (Cleanup of attribute parsing errors)
 - #149969 (don't use no_main and no_core to test IBT)
 - #149998 (miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a8460d0 into rust-lang:main Dec 15, 2025
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 15, 2025
@scottmcm scottmcm deleted the wrapping-shift-docs branch December 15, 2025 07:04
rust-timer added a commit that referenced this pull request Dec 15, 2025
Rollup merge of #149837 - scottmcm:wrapping-shift-docs, r=workingjubilee

Update `wrapping_sh[lr]` docs and examples

Inspired by [#general > &#96;Source&#96; link for &#96;core&#96; items is often inscrutable @ 💬][zulip-thread] I wanted to add some more examples of the actual wrapping as well as update the documentation to emphasize that the behaviour is unusual.

In particular, now that `unbounded_sh[lr]` is stable, point people trying to avoid panics to that instead, since it behaves less weirdly.

[zulip-thread]: https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.60Source.60.20link.20for.20.60core.60.20items.20is.20often.20inscrutable/near/562774474
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