Fix slice::ChunksMut aliasing

[saethlin]

Fixes #94231, details in that issue.
cc @RalfJung

This isn't done just yet, all the safety comments are placeholders. But otherwise, it seems to work.

I don't really like this approach though. There's a lot of unsafe code where there wasn't before, but as far as I can tell the only other way to uphold the aliasing requirement imposed by __iterator_get_unchecked is to use raw slices, which I think require the same amount of unsafe code. All that would do is tie the len and ptr fields together.

Oh I just looked and I'm pretty sure that ChunksExactMut, RChunksMut, and RChunksExactMut also need to be patched. Even more reason to put up a draft.

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[the8472]

but as far as I can tell the only other way to uphold the aliasing requirement imposed by __iterator_get_unchecked is to use raw slices, which I think require the same amount of unsafe code

But that could be improved by implementing slicing and split_at on them.

[the8472]

We discussed this in the libs team meeting. The general approach looks acceptable, but we'd like the new slice pointer APIs to be split out into a separate PR so they could be reviewed on their own. This PR can then be rebased once that's accepted.

[saethlin]

I think that sounds awesome. If there is someone else willing to do the new slice pointer APIs that would be ideal, my plate is pretty full already so it might take me some time to get through that whole process.

[the8472]

If there is someone else willing to do the new slice pointer APIs that would be ideal

#95594

[saethlin]

Blocked on #95594
@rustbot label: +S-blocked

[bors]

☔ The latest upstream changes (presumably #97632) made this pull request unmergeable. Please resolve the merge conflicts.

[saethlin]

@rustbot label: -S-blocked

[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

[saethlin]

I am stuck. The code works, but I cannot figure out how to write safety comments in most of these cases, because what makes the unsafe code valid is highly nonlocal. Basically I imagine these all as "this is safe because it is an invariant of the type" but that doesn't seem remotely helpful as a safety comment. Help?

Here are where I'm definitely missing safety comments (those that exist may need improving too):

tidy error: /home/ben/rust/library/core/src/slice/iter.rs:1762: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:2032: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:2075: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:2076: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:2728: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:2729: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:2788: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:2789: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:3035: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:3061: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:3087: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:3106: undocumented unsafe
tidy error: /home/ben/rust/library/core/src/slice/iter.rs:3107: undocumented unsafe

[the8472]

I am stuck. The code works, but I cannot figure out how to write safety comments in most of these cases, because what makes the unsafe code valid is highly nonlocal. Basically I imagine these all as "this is safe because it is an invariant of the type" but that doesn't seem remotely helpful as a safety comment. Help?

It's mostly about the new *mut [T]::split_at_mut calls, right?

I think this can be addressed in several steps

• put a general comment on the struct field that gives an overview
  • constructed from a valid &mut [T]
  • splitting always in-bounds, producing exclusively-owned subslices that can be converted back to &mut [T] and that keeps * mut [T]::len valid, which in turn makes the bounds checking in split_at_mut valid.
  • special-case: see __iterator_get_unchecked
• perhaps the split_at_mut docs can be improved? Currently state the general safety requirements, but perhaps they could have a better explanation in which specific circumstances those requirements are automatically fulfilled.

Or perhaps even simpler: We could explain that all uses should be treated as-if it were a &mut [T], which make the whole thing safe except in the case where __iterator_get_unchecked is called, but then the TrustedRandomAccess contract takes over requiring only-once access to each subslice.

[the8472]

@bors r+

[bors]

📌 Commit 746afe8 has been approved by the8472

It is now in the queue for this repository.

[RalfJung]

Is the PR description outdated? With that description (which will now live forever in our git history) it doesn't sound like this was ready to land...

[saethlin]

🤦 it's outdated