Improve the documentation of black_box

[tgross35]

There don't seem to be many great resources on how black_box should be used, so I added some information here

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon.

Please see the contribution instructions for more information.

[rustbot]

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 @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. 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:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[tgross35]

@rustbot label +A-docs

[Mark-Simulacrum]

I think this might be understating a little - I know there was some discussion prior to stabilization/on the release blog post about emphasizing more of the lack of guarantees (cc @nagisa @saethlin).

I think at minimum, we shouldn't try to phrase it as "not stopping all optimizations", which makes it sound like some are guaranteed.

[tgross35]

Would something like this be better?

A word on guarantees

It is important to reiterate the above: black_box does not guarantee with any certainty what optimizations may or may not happen. Variations such as different compiler backends, different targets,
or even just corner case code may mean it has no effects at all. The Rust team attempts to make sure this doesn't happen, but it cannot be guaratnteed: for that reason, black_box should never be relied on for controlling behavior outside of test cases or benchmarks.

For the most performance-critical cases, there is no substitute for validating the generated assembly and using performance tools (e.g. perf or callgrind) to analyze your code. That being said, black_box is suitable for creating fairly accurate benchmarks in most cases, for most users.

I don't think I know enough about the specific context to know how accurate that is, so open to suggestions (didn't see too much on #64102)

[saethlin]

I would prefer changing the wording of the text you're proposing adding, as opposed to adding another section. Documentation here should emphasize everywhere that black_box having any effect at all on a program is entirely best-effort, it may do nothing at all, and therefore this is only for benchmarks. The are already people who think it is for writing cryptography code, and that is wrong.

[saethlin]

In practice this function is pretty reliable, but it is absolutely NOT completely reliable, and even if it were, documenting what it actually accomplishes if it were reliable sounds like a nightmare.

[tgross35]

I've rearranged this somewhat and now that bit is at the beginning: https://github.com/rust-lang/rust/blob/79cf860f397aff83d283ac8de0ac77057ec59147/library/core/src/hint.rs#L225-L233

[saethlin]

I really like the changes you've made. The documentation as-written now answers all my concerns, I've just left some grammar/phrasing tweaks.

[tgross35]

Thank you, I've applied your changes & rebased

[tgross35]

I don't think there are further changes (let me know if so) so

r? Mark-Simulacrum

(Not sure if you can merge saethlin)

[Mark-Simulacrum]

@bors r+ rollup

[bors]

📌 Commit 13e25b8 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[tgross35]

Thanks!