Add slice::{split_,}{first,last}_chunk{,_mut}

[clarfonthey]

This adds to the existing tracking issue for slice::array_chunks (#74985) under a separate feature, slice_get_chunk.

Currently, we have the existing first/last API for slices:

impl[T]{pubconstfnfirst(&self) -> Option<&T>;pubconstfnfirst_mut(&mutself) -> Option<&mutT>;pubconstfnlast(&self) -> Option<&T>;pubconstfnlast_mut(&mutself) -> Option<&mutT>;pubconstfnsplit_first(&self) -> Option<(&T,&[T])>;pubconstfnsplit_first_mut(&mutself) -> Option<(&mutT,&mut[T])>;pubconstfnsplit_last(&self) -> Option<(&T,&[T])>;pubconstfnsplit_last_mut(&mutself) -> Option<(&mutT,&mut[T])>;}

This augments it with a first_chunk/last_chunk API that allows retrieving multiple elements at once:

impl[T]{pubconstfnfirst_chunk<constN:usize>(&self) -> Option<&[T;N]>;pubconstfnfirst_chunk_mut<constN:usize>(&mutself) -> Option<&mut[T;N]>;pubconstfnlast_chunk<constN:usize>(&self) -> Option<&[T;N]>;pubconstfnlast_chunk_mut<constN:usize>(&mutself) -> Option<&mut[T;N]>;pubconstfnsplit_first_chunk<constN:usize>(&self) -> Option<(&[T;N],&[T])>;pubconstfnsplit_first_chunk_mut<constN:usize>(&mutself) -> Option<(&mut[T;N],&mut[T])>;pubconstfnsplit_last_chunk<constN:usize>(&self) -> Option<(&[T;N],&[T])>;pubconstfnsplit_last_chunk_mut<constN:usize>(&mutself) -> Option<(&mut[T;N],&mut[T])>;}

The code is based off of a copy of the existing API, with the documentation and examples properly modified. Currently, the most common way to perform these kinds of lookups with the existing methods is via slice.as_chunks::<N>().0[0] or the worse slice.as_chunks::<N>().0[slice.len() - N], which is substantially less readable than slice.first_chunk::<N>() or slice.last_chunk::<N>().

ACP: rust-lang/libs-team#69

[rust-highfive]

r? @yaahc

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

[scottmcm]

I've definitely written a bunch of examples that did .as_chunks().0[0], so 👍 to having something in this area.

I don't know if this is how I'd expect it to look, though. For example, the indexing could be left to subslicing, like a[i..].prefix_chunk() (instead of .get_chunk(i)) or a[..i].suffix_chunk() (instead of .get_chunk(i - N)).

Of course, the other thing that comes to mind is wanting split_at_*-style APIs for &[T] -> (&[T; N], &[T]) and &[T] -> (&[T]; &[T; N]). Or maybe chunk versions of the take_{first/last} APIs?

[clarfonthey]

Hmm, you do have a point about subslicing; it might be more natural to have slice[idx..].first_chunk::<N>() and slice[..idx].last_chunk::<N>(). However, in this case, it would make sense to return Option<[T; N]> which offloads the bounds checking to the user.

Essentially, we'd mimic the first and last APIs but with chunky semantics, accepting const generics. I'm down to make these changes if that seems like the most natural way forward.

[scottmcm]

I don't know what the best approach would be here. Might be worth seeing if libs-api has any specific preferences.

The whole "add const generic versions of most of the slice stuff" is a big project, but seems useful, so maybe they could have some generalized guidance of how they'd like to see it done? If that existed then I'd be happy to sign off on a bunch of individual PRs for adding the various pieces.

[clarfonthey]

That's incredibly fair. Since I was feeling particularly motivated, I decided to go ahead and implement the first/last chunk semantics that I mentioned and maybe that can help explain how this would work out.

I personally find the result rather intuitive, although the chunk naming will probably need work before stabilisation. Hopefully seeing how this would work in code will help iron out that process.

EDIT: And I finished modifying the PR description to properly summarise the new API and the use cases it allows.

[clarfonthey]

I genuinely have no idea how this wasn't required before, since the existing implementation of split_at requires split_at_unchecked ??

Anyway, I decided to reuse this code for these methods and needed this.

[scottmcm]

These look great to me, but apparently I was blind looking at the rustdoc, as I forgot about https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.split_array_mut from #90091 (comment) .

I personally like these better, because they solve the Option-ness question.

[clarfonthey]

Yeah, it's always a challenge to figure out what's there when there are so many methods and they're not really sorted in any logical way. But, I guess, that's a challenge for another day.

Before merge, we can maybe decide which tracking issue this should go in, since, in hindsight, it makes the most sense for it to have its own tracking issue.

[jethrogb]

I don't think chunk sufficiently conveys these APIs deal with constant-size arrays, give the existing chunks API.

[bors]

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

[Dylan-DPC]

r? @rust-lang/libs-api

[scottmcm]

Nominating for libs-api to try to unblock this. (Since it just came up on URLO.)

Should we just merge to nightly as fine for unstable? Do you have strong feelings about the direction to take for "get arrays from slices" APIs? Does this need an ACP/RFC now? (Of course ACPs didn't exist when this PR was opened.)

[bors]

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

[GKFX]

With the APIs proposed, I don't see a nice way to extract a subslice at arbitrary offset (and fixed length) as an array and get an Option without panicking. s[a..].first_chunk::<N>() will panic for s.len() < a, and s.get(a..).and_then(|x| x.first_chunk::<N>()) is verbose. Rather than having to add another API at a later date, I would replace first_chunk and first_chunk_mut with get_array[_mut], by analogy to plain slice::get[_mut]:

impl[T]{pubconstfnget_array<constN:usize>(&self,start_idx:usize) -> Option<&[T;N]>;pubconstfnget_array_mut<constN:usize>(&mutself,start_idx:usize) -> Option<&mut[T;N]>;}

I think it's acceptable to have to specify a zero to get the first chunk: s.get_array(0) is no longer than s.first_chunk().

We discussed this in today's @rust-lang/libs-api meeting. We felt that the chunks and rchunks and chunks_mut and rchunks_mut methods (and the array_ versions) address the half of these that don't involve split.

We'd be interested to see if the iterator types returned by these methods could address the split variants of these by having a method on that iterator type. For instance, a remaining method, or a split method that returns the pair of next and the remaining items.

The methods array_chunks etc. do not permit zero-length arrays, so using array_chunks in place of first_chunk would have an unnecessary panic (or compile error) in that case. An edge case, but could be awkward.

[scottmcm]

Looks like the ACP was accepted! 🎉

@clarfonthey Can you rebase and resolve conflicts and such please?

[clarfonthey]

(Going to wait on final CI run to pass the torch, but things should be rebased now. The biggest change I had to make was using split_at_unchecked instead of get_unchecked now that all existing uses of const traits have been purged from libstd to make way for keyword generics. I don't think this should negatively affect performance.)

[clarfonthey]

@rustbot ready

[scottmcm]

Hmm, I would have expected this to be -> Option<(&[T], &[T; N])>, but this is consistent with split_last, so I guess it's fine for now. I'll put a note in the tracking issue.

[clarfonthey]

Yeah, there are arguments for both but honestly, I don't feel too swayed either way. I think that the potential issues here are greater because of how the types are only nominally different and can both be coerced to be the same, although I don't think there's a whole much that can be done about that than encourage folks to pay more attention.

[scottmcm]

Yeah, I'm worried that someone might try something that boils down to let (head, tail) = x.split_last_chunk(); frobble(tail[0]) and being confused when it compiles (as you said) but doesn't do what they expected.

[WaffleLapkin]

I think that should probably be mentioned on the tracking issue, because I would write something like that... 😅

[scottmcm]

@WaffleLapkin Is the note in #111774 (comment) not sufficient?

[WaffleLapkin]

It is sufficient, thanks

[scottmcm]

Thanks! I'm excited to have these.

@bors r+ rollup

[bors]

📌 Commit d58dd10 has been approved by scottmcm

It is now in the queue for this repository.