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.

Sign up for GitHub

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

Jump to bottom

black_box docs correctness claim incorrect #107957

Closed
gonzalobg opened this issue Feb 12, 2023 · 5 comments · Fixed by #108088 or #108416
Closed

black_box docs correctness claim incorrect #107957

gonzalobg opened this issue Feb 12, 2023 · 5 comments · Fixed by #108088 or #108416
Assignees
@pat-nel87
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@gonzalobg
Copy link

gonzalobg commented Feb 12, 2023 •
edited by rustbot
Loading

Location

https://doc.rust-lang.org/stable/std/hint/fn.black_box.html

Summary

The documentation of black_box states:

Programs cannot rely on black_box for correctness in any way.

but this is not accurate since programs can rely on it behaving like the identity function.
It would be better to reword this as

Programs cannot rely on black_box for correctness beyond it behaving as the identity function.

or similar, e.g.,

Programs cannot rely on the optimisation-inhibiting effects of black_box for correctness.
@gonzalobg gonzalobg added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Feb 12, 2023
@jyn514
Copy link
Member

jyn514 commented Feb 15, 2023

@gonzalobg are you interested in making a PR to fix this? :)

@jyn514 jyn514 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 15, 2023
@gonzalobg
Copy link
Author

Can't, sorry.

@callumio
Copy link
Contributor

@rustbot claim

@callumio callumio mentioned this issue Feb 15, 2023
Merged
@bors bors closed this as completed in 863cd15 Feb 16, 2023
@gonzalobg
Copy link
Author

It seems that there are some other parts of the docs that are still incorrect. For example, in the How to use it section, the incorrect claim continues to be repeated:

First and foremost: black_box does not guarantee any exact behavior

I think the whole document needs a pass to fix this.

@jyn514 jyn514 reopened this Feb 16, 2023
@pat-nel87
Copy link
Contributor

@rustbot claim

@rustbot rustbot assigned pat-nel87 and unassigned callumio Feb 23, 2023
pat-nel87 added a commit to pat-nel87/rust that referenced this issue Feb 24, 2023
@pat-nel87
black_box doc corrections - Issue rust-lang#107957
6f7ca32
@pat-nel87 pat-nel87 mentioned this issue Feb 24, 2023
Merged
@jyn514 jyn514 mentioned this issue Apr 26, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 26, 2023
@bors
Auto merge of rust-lang#110839 - jyn514:rollup-uikilwm, r=jyn514
ae3ab14 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#108416 (black_box doc corrections for clarification - Issue rust-lang#107957)
 - rust-lang#109379 (Replace `yes` command by `while-echo` in test `tests/ui/process/process-sigpipe.rs`)
 - rust-lang#110266 (Update documentation wording on path 'try_exists' functions)
 - rust-lang#110329 (Improve tests for rust-lang#110138)
 - rust-lang#110418 (Spelling rustdoc)
 - rust-lang#110587 (Fix `std` compilation error for wasi+atomics)
 - rust-lang#110594 (`rustc --help` add `--cfg` SPEC declaration.)
 - rust-lang#110792 (Use the standard macOS CI runner)
 - rust-lang#110817 (Add regression tests for const-generic inherent associated types)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in ab7e01e Apr 26, 2023
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jul 18, 2023
@jyn514
Rollup merge of #108416 - pat-nel87:Issue-107957-black_box_docs, r=jy…
5f4894b 
…n514

black_box doc corrections for clarification - Issue #107957

Made a complete pass through the docs to help resolve rust-lang/rust#107957

No code changes, just documentation

`@rustbot` label +T-libs-api -T-libs
spikespaz pushed a commit to spikespaz/dotwalk-rs that referenced this issue Aug 29, 2024
@jyn514
Rollup merge of #108416 - pat-nel87:Issue-107957-black_box_docs, r=jy…
c1a85b9 
…n514

black_box doc corrections for clarification - Issue #107957

Made a complete pass through the docs to help resolve rust-lang/rust#107957

No code changes, just documentation

`@rustbot` label +T-libs-api -T-libs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pat-nel87 pat-nel87

Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
4 participants
@callumio @jyn514 @gonzalobg @pat-nel87