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

std::hint::unreachable_unchecked: docs note about compiler optimizations being sufficient? #95865

Closed
sharnoff opened this issue Apr 9, 2022 · 1 comment · Fixed by #96154
Closed

Comments

@sharnoff
Copy link
Contributor

sharnoff commented Apr 9, 2022

I don't know whether this is something that the docs team already has a particular policy on, so perhaps this is out of scope, but:

The docs for std::hint::unreachable_unchecked give an example that -- with optimizations -- compiles to the same assembly using plain unwrap instead of unwrap_or_else(|| unsafe { unreachable_unchecked }). (See: https://godbolt.org/z/4ha9K1rGK)

Given that this is likely often the case for simple inputs (particularly for arithemtic operations like the example), I was wondering if it would be good to have some kind of note in the documentation to advising that this function should only be used if it's actually necessary. In particular, this could note that rustc with -C opt-level=1 will produce the same assembly for the example as a version that uses unwrap instead.

Of course several caveats apply (it's not ideal to rely on compiler optimizations in docs, etc) - but IMO it could be worthwhile for the purpose of discouraging unecessary unsafe.

@leonardo-m
Copy link

Also the example could be replaced with a better one where unreachable_unchecked gives an advantage.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 13, 2022
Expand core::hint::unreachable_unchecked() docs

Rework the docs for `unreachable_unchecked`, encouraging deliberate use, and providing a better example for action at a distance.

Fixes rust-lang#95865
@bors bors closed this as completed in 3615cb4 May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants