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

nightly SliceIndex::get_unchecked, get_unchecked_mut docs #122234

Closed
ratmice opened this issue Mar 9, 2024 · 3 comments · Fixed by #122302
Closed

nightly SliceIndex::get_unchecked, get_unchecked_mut docs #122234

ratmice opened this issue Mar 9, 2024 · 3 comments · Fixed by #122302
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. E-help-wanted Call for participation: Help is requested to fix this issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@ratmice
Copy link
Contributor

ratmice commented Mar 9, 2024

Location

https://doc.rust-lang.org/std/slice/trait.SliceIndex.html#tymethod.get_unchecked
https://doc.rust-lang.org/std/slice/trait.SliceIndex.html#tymethod.get_unchecked_mut

Summary

The documentation for these methods state respectively:

Returns a shared reference to the output at this location

Returns a mutable reference to the output at this location

However these clearly return pointers rather than references.
Later on they state the following restrictions, which make sense for references but it isn't clear to me that this restriction is necessary for pointers.

Calling this method with an out-of-bounds index or a dangling slice pointer is undefined behavior even if the resulting reference is not used.

At the very least the s/reference/pointer/ change needs to be made there too.

@ratmice ratmice added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Mar 9, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 9, 2024
@jieyouxu jieyouxu added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 9, 2024
@workingjubilee
Copy link
Member

These are nightly-only traits that aren't particularly on-track for stabilization anytime soon, but yes, changing them to be correct on those counts would be fine.

@workingjubilee workingjubilee added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels Mar 10, 2024
@ratmice
Copy link
Contributor Author

ratmice commented Mar 10, 2024

Indeed, while it's nightly only I wasn't actually going to call it from nightly, but was looking because of it's use in other stable functions, e.g. Slice::get_unchecked_mut.

Would make a patch, but I wasn't sure whether to leave the undefined behavior line in or whether it should be removed.

@workingjubilee
Copy link
Member

@ratmice It's still UB, yeah! This is partly because it still has to fulfill the safety conditions of pointer::add. Obviously, that isn't a full justification: there is the question of adding until the pointer is offset to the "last element", which would not trigger "language UB" via that implementation detail...

But the library defines it as UB, which means the implementation may include code that would cause an optimizer to see that it is "impossible" for the defined-UB result to happen. And the question of whether the "surface functions" have this obligation was settled in #117039 so yes, the same conclusion applies.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 10, 2024
docs: Correct ptr/ref verbiage in SliceIndex docs.

Fixes rust-lang#122234
@bors bors closed this as completed in dd2cda7 Mar 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 11, 2024
Rollup merge of rust-lang#122302 - ratmice:issue122234, r=cuviper

docs: Correct ptr/ref verbiage in SliceIndex docs.

Fixes rust-lang#122234
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. E-help-wanted Call for participation: Help is requested to fix this issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants