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

Tracking Issue for split_array #90091

Open
2 tasks
Tracked by #16
jethrogb opened this issue Oct 20, 2021 · 39 comments · Fixed by #117561
Open
2 tasks
Tracked by #16

Tracking Issue for split_array #90091

jethrogb opened this issue Oct 20, 2021 · 39 comments · Fixed by #117561
Labels
A-array Area: `[T; N]` C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jethrogb
Copy link
Contributor

jethrogb commented Oct 20, 2021

Feature gate: #![feature(split_array)]

This is a tracking issue for splitting slices and arrays into constant-length arrays.

Public API

impl<T, const N: usize> [T; N] {
    pub fn split_array_ref<const M: usize>(&self) -> (&[T; M], &[T]);
    pub fn split_array_mut<const M: usize>(&mut self) -> (&mut [T; M], &mut [T]);

    pub fn rsplit_array_ref<const M: usize>(&self) -> (&[T], &[T; M]);
    pub fn rsplit_array_mut<const M: usize>(&mut self) -> (&mut [T], &mut [T; M]);
}

impl<T> [T] {
    pub fn split_array_ref<const N: usize>(&self) -> (&[T; N], &[T]);
    pub fn split_array_mut<const N: usize>(&mut self) -> (&mut [T; N], &mut [T]);

    pub fn rsplit_array_ref<const N: usize>(&self) -> (&[T], &[T; N]);
    pub fn rsplit_array_mut<const N: usize>(&mut self) -> (&mut [T], &mut [T; N]);
}

Unresolved Questions

impl<T, const N: usize> [T; N] {
    pub fn split_array<const M: usize>(self) -> ([T; M], [T; N - M]);
    pub fn split_array_ref<const M: usize>(&self) -> (&[T; M], &[T; N - M]);
    pub fn split_array_mut<const M: usize>(&mut self) -> (&mut [T; M], &mut [T; N - M]);
}

However, const generics is not powerful enough for this today. See #83233 (comment) and #83233 (comment). The array functions need to be here so the slice functions can be stabilized without forward compatibility concerns.

@jethrogb jethrogb added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 20, 2021
@jdahlstrom
Copy link

Hmm. Panicking if M > slice.len(), rather than returning a Result/Option, seems a bit surprising to me. I guess it's consistent with split_at, but on the other hand inconsistent with eg. split_first.

@scottmcm
Copy link
Member

scottmcm commented Dec 3, 2021

For the slice versions of these, I think it's important that they not only offer -> (&[T; N], &[T]), but also -> (&[T], &[T; N]). (This is basically the same as how there's both as_chunks and as_rchunks, see #78818.)

That might open up new naming possibilities too. To spitball:

  • split_prefix: &[T] -> (&[T; N], &[T]) and
  • split_suffix: &[T] -> (&[T], &[T; N])

@jplatte
Copy link
Contributor

jplatte commented Dec 4, 2021

I agree that should exist, but I think it should be named rsplit_array since there's already a bunch of split functions whose "from the end" variations are all called rsplit*.

@yaahc
Copy link
Member

yaahc commented Dec 10, 2021

I'd argue for local consistency over global consistency. In the case of slice we already have methods following both conventions, but the methods following the _prefix _suffix convention are related to stripping, where as the split methods all currently follow the split rsplit convention, so I lean towards keeping the current names.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 11, 2021
Add rsplit_array variants to slices and arrays

By request: rust-lang#90091 (comment)

r? `@yaahc`
@jethrogb
Copy link
Contributor Author

Should there also be a shorthand API for slice[a..].split_array_ref::<B>() (the array version of &slice[a..(a+B)])?

@pmnoxx
Copy link
Contributor

pmnoxx commented Jan 16, 2022

I think that the result should be an Option. This would allow us to properly handle splitting in case, the number of elements is lesser than the given contant.

impl<T, const N: usize> [T; N] {
    pub fn split_array_ref<const M: usize>(&self) -> Option<(&[T; M], &[T])>;
    pub fn split_array_mut<const M: usize>(&mut self) -> Option<(&mut [T; M], &mut [T])>;

    pub fn rsplit_array_ref<const M: usize>(&self) -> Option<(&[T], &[T; M])>;
    pub fn rsplit_array_mut<const M: usize>(&mut self) -> Option<(&mut [T], &mut [T; M])>;
}

I've been looking an api like that. However, it would be great to add a proper handling of cases, where split can't happen.

Consider

[1,2,3].split_array_ref<4>().is_none(); /// Would make sense if the split fails

/// It's useful for network code, to check if prefix has length at least 4 to decode the message.
/// This code can avoid any unwraps, because `u32::from_le_bytes` need array of 4 elements as input. 
if let Some((left, _) = [1,2,3,4,5].split_array_ref<4>() {
   println!("{}", u32::from_le_bytes(left));
}

@jethrogb
Copy link
Contributor Author

split_at doesn't return an Option.

@pmnoxx
Copy link
Contributor

pmnoxx commented Jan 16, 2022

split_at doesn't return an Option.

Yes, that's unfortunate. I think it would be great to add try_split_at api. This would simplify at lot of code.

impl<T, const N: usize> [T; N] {
   pub fn try_split_at(&self, mid: usize) -> Option<&[T], &[T]>

    pub fn try_split_array_ref<const M: usize>(&self) -> Option<(&[T; M], &[T])>;
    pub fn try_split_array_mut<const M: usize>(&mut self) -> Option<(&mut [T; M], &mut [T])>;

    pub fn try_rsplit_array_ref<const M: usize>(&self) -> Option<(&[T], &[T; M])>;
    pub fn try_rsplit_array_mut<const M: usize>(&mut self) -> Option<(&mut [T], &mut [T; M])>;
}

For example:
Currently, I have to write code like this to use the api:

let ary = [127,0,0,1,55,66];

if ary.len() >= 4 {
   let (ip,_) = ary.split_at(4);
   ///. ...
}

I think it would be much cleaner to write it in a declarative way, which avoid hidden unwraps of getting a slice.

let ary = [127,0,0,1,55,66];

if let Some((ip,_)) = ary.try_split_at(4) {
  /// ...
}

Or even better

let ary = [127,0,0,1,55,66];

if let Some((ip,_)) = ary.try_split_array_ref<4>() {
  /// This makes ip a reference to 4 bytes array.
  /// ...
}

@scottmcm
Copy link
Member

cc #95198, which proposes doing this more like the split_first family of things.

Those naturally would return Option, the same way that first & split_first return Option today.

@faern
Copy link
Contributor

faern commented Apr 7, 2022

I definitely want infallible (fail at compile time) split methods on arrays. I want to be able to cut arrays up into smaller array chunks without any bounds checks at runtime. I know there are issues currently that won't allow this API today. But I think we should aim for a future where we can do that.

Being able to parse the content of arrays without paying more bounds checks or code bloat than needed is definitely desired both for readability and performance. ([0u8; 100]).split_array_ref::<10>().expect("out of bounds, will never happen) does not look nice and has a lot of code paths that we already know won't happen. If it was a compile time error we would not accidentally create bugs where the 10 was made into a too large number.

@faern
Copy link
Contributor

faern commented Apr 7, 2022

impl<T, const N: usize> [T; N] {
    pub fn try_split_array_ref<const M: usize>(&self) -> Option<(&[T; M], &[T])>;
}

This makes no sense IMO 🤔 Because the only time it would ever be None should be found at compile time. I know that's not possible now. But why introduce a large API surface and use up a bunch of good method names for something that's (hopefully)? a temporary limitation.

@Xiretza
Copy link
Contributor

Xiretza commented May 18, 2022

I agree with the comments above that the methods on &[T] should return Options, but as @faern said this makes no sense on arrays, where all size checks are known at compile time. In my opinion the API would ideally look like this (note also the rsplit_ methods on arrays, for completeness):

impl<T, const N: usize> [T; N] {
    pub fn split_array<const M: usize>(self) -> ([T; M], [T; N - M]);

    pub fn split_array_ref<const M: usize>(&self) -> (&[T; M], &[T; N - M]);
    pub fn split_array_mut<const M: usize>(&mut self) -> (&mut [T; M], &mut [T; N - M]);

    pub fn rsplit_array_ref<const M: usize>(&self) -> (&[T; N - M], &[T; M]);
    pub fn rsplit_array_mut<const M: usize>(&mut self) -> (&mut [T; N - M], &mut [T; M]);
}

impl<T> [T] {
    pub fn split_array_ref<const N: usize>(&self) -> Option<(&[T; N], &[T])>;
    pub fn split_array_mut<const N: usize>(&mut self) -> Option<(&mut [T; N], &mut [T])>;

    pub fn rsplit_array_ref<const N: usize>(&self) -> Option<(&[T], &[T; N])>;
    pub fn rsplit_array_mut<const N: usize>(&mut self) -> Option<(&mut [T], &mut [T; N])>;
}

But as noted in the OP, this requires more advanced const generics features. The alternative array API (returning slices for the "rest") would be possible today, but I think it'd be better to wait until the "proper" solution is possible rather than stabilizing something suboptimal.

@Xiretza
Copy link
Contributor

Xiretza commented Jul 15, 2022

Would it be possible to stabilize the slice methods, while the array methods remain unstable and continue to use the temporary API until const generics are more advanced? I don't think method resolution takes stable status and return type into account, right?

This would at least give us the slice methods right now, whereas the necessary const generic work is still quite a ways away from stabilization if I'm not mistaken.

@gcxfd
Copy link

gcxfd commented Jul 25, 2022

maybe can add split_array to Box<[T]> ?

@WaffleLapkin
Copy link
Member

@gcxfd if you want (Box<[T]>, usize) -> (Box<[T]>, Box<[T]>) kind of function, that's impossible without re-allocating the boxes, so I don't think this will be added to std. If you want to get slices ((Box<[T]>, usize) -> (&[T], &[T])), then you can use the methods implemented on slice thanks to the deref coercion.

@Kixunil
Copy link
Contributor

Kixunil commented Oct 22, 2022

Note that it is currently possible to return &[T, (N - M)] on nightly with an ugly warning

How hard would it be to at least allow it for core even if not stabilizing?

@sdroege
Copy link
Contributor

sdroege commented Dec 6, 2022

While this can be implemented with multiple calls to this API, I was wondering if it wouldn't be nice to have some kind of subarray API: [1, 2, 3, 4, 5].subarray::<1, 3>() == &[2, 3, 4] or similar. Was that considered somewhere already?

@Kixunil
Copy link
Contributor

Kixunil commented Dec 6, 2022

@sdroege yeah, that'd be very helpful. I've seen a bunch of code that currently has to use array[0..16].try_into().unwrap(). It'd need to use const generics instead of arguments though.

@kgiebeler-xq-tec
Copy link

I stumbled upon this feature while writing a Stack ADT.

My current implementation looks like that:

    pub fn pop<const N: usize, T: From<[u8; N]>>(&mut self) -> T {
        // TODO switch to `rsplit_array_mut`, once https://github.com/rust-lang/rust/issues/90091 stabilizes
        let bytes = self.bytes.split_off(self.bytes.len() - N);
        T::from(<[u8; N]>::try_from(bytes).unwrap())
    }

Would it make sense to add a split_array_off or split_off_array to the family of function described in this issue?

@Xiretza
Copy link
Contributor

Xiretza commented Mar 12, 2023

I've had these changes lying around for a while and finally pushed them in #109049. I'll take a look at making everything const fn as well.

@Kixunil
Copy link
Contributor

Kixunil commented Mar 12, 2023

@faern sure, the implementation is not that important, I was mainly concerned about API. That being said the optimizer should be smart enough to remove branches; const is a good point but IIUC core can already use experimental const traits.

@Xiretza
Copy link
Contributor

Xiretza commented May 2, 2023

Note that it is currently possible to return &[T, (N - M)] on nightly with an ugly warning

How hard would it be to at least allow it for core even if not stabilizing?

As noted in #109049 (comment), this will not be allowed for now.

I would still like to get an answer to #90091 (comment); if that idea is feasible, I'll implement it.

@safinaskar
Copy link
Contributor

safinaskar commented Jun 1, 2023

I fully agree with @Xiretza's proposal here: #90091 (comment) . Note that parts of it already implemented here: #111774

@safinaskar
Copy link
Contributor

In this proposal (i. e. #90091) [T; N] and [T] will have methods with same names. This is wrong. Because this (thanks to deref coercion) will make notation a.split_array_ref::<1>() look very confusing

@Xiretza
Copy link
Contributor

Xiretza commented Jun 1, 2023

In this proposal [...] [T; N] and [T] will have methods with same names. This is wrong.

It's certainly something that should be given thought to, but I don't think it's necessarily "wrong". The same thing happens whenever a trait is implemented both for a type and its deref target, so it's not like this adds an entirely new level of confusion.

@WaffleLapkin
Copy link
Member

WaffleLapkin commented Jun 1, 2023

As a side note: [T; N] -> [T] is not Deref coercion, it's unsize coercion (which is, I believe, special cased to participate in method resolution (as opposed to, say [T; N] -> dyn Trait which is still unsize coercion but does not get involved in the method resolution)).

I also don't think that's particularly confusing, as it is common in Rust to have different types have methods with the same names. (it is, however, something to think about and be careful wrt stabilization)

@YuriGor
Copy link

YuriGor commented Aug 30, 2023

Hello everyone, how to get to know if this feature going to stay in stable version?

And also please validate my potential use case. I have my internal lightweight implementation of 2d array which stores rows in a single array, so knowing number of columns we can always slide the window to correct offset at any row we want:

pub struct Array2d<T: Default + Copy, const SIZE: usize, const COLUMNS: usize> {
    data: [T; SIZE],
}

impl<T: Default + Copy, const SIZE: usize, const COLUMNS: usize> Array2d<T, SIZE, COLUMNS> {
    const ROWS: usize = SIZE / COLUMNS;

// ...

    pub fn get_row(&self, row: usize) -> Option<&[T]> {
        if row < Self::ROWS {
            Some(&self.data[row * COLUMNS..(row + 1) * COLUMNS])
        } else {
            None
        }
    }

}

// ...

Currently i return slice from get_row. But I always know the size.
Would it make sense to use split_array_ref and rsplit_array_ref to cut off data before/after specified row and return fixed-sized ref to array?
something like (untested):

 pub fn get_row(&self, row: usize) -> Option<&[T; COLUMNS]> {
        if row < Self::ROWS {
            let (row_data,_) = self.data.split_array_ref::<COLUMNS * (row + 1)>(); // drop trailing data after the row
            let (_, final_row_data) = row_data.rsplit_array_ref::<COLUMNS>(); // drop leading data before the row
            Some(final_row_data)
        } else {
            None
        }
    }

Would it have some performance benefits, like compiler would be able to optimize smth knowing fixed size beforehand compared with slices which gave dynamic size?
Or what is main point of using these experimental methods instead of slices?

@tgross35
Copy link
Contributor

tgross35 commented Nov 1, 2023

There has been some ongoing discussion on Zulip, https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Stabilizing.20array-from-slice.20*something*.3F. Copying what I said there, I think we may be able to move forward with something like the following:

  1. Stabilize the API in Tracking Issue for slice_first_last_chunk feature (slice::{split_,}{first,last}_chunk{,_mut}) #111774, [T}::{split_}{first,last}_chunk{_mut}. This provides a fallible way to get an array from a slice, everything returns an Option
  2. Remove the slice API from Tracking Issue for split_array #90091 (this issue). These are all infallible but the discussion has made it very clear that a panicking API is not wanted
  3. Postpone the array API from Tracking Issue for split_array #90091 (this issue) until generic_const_exprs is better off and we can provide a nonpanicking array splitting API
    a. Maybe consider something like the array indexing API [1, 2, 3, 4, 5].subarray::<1, 3>() == &[2, 3, 4] suggested in Tracking Issue for split_array #90091 (comment)

#111774 provides the identical API as this issue except with fallibility, which seems to be what most users want based on discussion here. I also think the naming is nicer - chunk is often used in std to represent a known-length array or slice, array_ref is only used in this method.

@tgross35
Copy link
Contributor

tgross35 commented Nov 4, 2023

...and I did just that in #117561

@scottmcm
Copy link
Member

scottmcm commented Jan 11, 2024

With those methods in FCP (#117561 (comment)), nominating this one for libs-api to consider whether they'd like to see these ones deleted from nightly.

Err, I should read better. https://github.com/rust-lang/rust/pull/117561/files#diff-e8ccaf64ce21f955ccebef33b52158631493a6f0966815a2ebc142d7cd2b5e06L2024 is removing these methods.

@scottmcm scottmcm added I-libs-api-nominated Nominated for discussion during a libs-api team meeting. and removed I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels Jan 11, 2024
@scottmcm scottmcm linked a pull request Jan 11, 2024 that will close this issue
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 19, 2024
Stabilize `slice_first_last_chunk`

This PR does a few different things based around stabilizing `slice_first_last_chunk`. They are split up so this PR can be by-commit reviewed, I can move parts to a separate PR if desired.

This feature provides a very elegant API to extract arrays from either end of a slice, such as for parsing integers from binary data.

## Stabilize `slice_first_last_chunk`

ACP: rust-lang/libs-team#69
Implementation: rust-lang#90091
Tracking issue: rust-lang#111774

This stabilizes the functionality from rust-lang#111774:

```rust
impl [T] {
    pub const fn first_chunk<const N: usize>(&self) -> Option<&[T; N]>;
    pub fn first_chunk_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
    pub const fn last_chunk<const N: usize>(&self) -> Option<&[T; N]>;
    pub fn last_chunk_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
    pub const fn split_first_chunk<const N: usize>(&self) -> Option<(&[T; N], &[T])>;
    pub fn split_first_chunk_mut<const N: usize>(&mut self) -> Option<(&mut [T; N], &mut [T])>;
    pub const fn split_last_chunk<const N: usize>(&self) -> Option<(&[T], &[T; N])>;
    pub fn split_last_chunk_mut<const N: usize>(&mut self) -> Option<(&mut [T], &mut [T; N])>;
}
```

Const stabilization is included for all non-mut methods, which are blocked on `const_mut_refs`. This change includes marking the trivial function `slice_split_at_unchecked` const-stable for internal use (but not fully stable).

## Remove `split_array` slice methods

Tracking issue: rust-lang#90091
Implementation: rust-lang#83233 (review)

This PR also removes the following unstable methods from the `split_array` feature, rust-lang#90091:

```rust
impl<T> [T] {
    pub fn split_array_ref<const N: usize>(&self) -> (&[T; N], &[T]);
    pub fn split_array_mut<const N: usize>(&mut self) -> (&mut [T; N], &mut [T]);

    pub fn rsplit_array_ref<const N: usize>(&self) -> (&[T], &[T; N]);
    pub fn rsplit_array_mut<const N: usize>(&mut self) -> (&mut [T], &mut [T; N]);
}
```

This is done because discussion at rust-lang#90091 and its implementation PR indicate a strong preference for nonpanicking APIs that return `Option`. The only difference between functions under the `split_array` and `slice_first_last_chunk` features is `Option` vs. panic, so remove the duplicates as part of this stabilization.

This does not affect the array methods from `split_array`. We will want to revisit these once `generic_const_exprs` is further along.

## Reverse order of return tuple for `split_last_chunk{,_mut}`

An unresolved question for rust-lang#111774 is whether to return `(preceding_slice, last_chunk)` (`(&[T], &[T; N])`) or the reverse (`(&[T; N], &[T])`), from `split_last_chunk` and `split_last_chunk_mut`. It is currently implemented as `(last_chunk, preceding_slice)` which matches `split_last -> (&T, &[T])`. The first commit changes these to `(&[T], &[T; N])` for these reasons:

- More consistent with other splitting methods that return multiple values: `str::rsplit_once`, `slice::split_at{,_mut}`, `slice::align_to` all return tuples with the items in order
- More intuitive (arguably opinion, but it is consistent with other language elements like pattern matching `let [a, b, rest @ ..] ...`
- If we ever added a varidic way to obtain multiple chunks, it would likely return something in order: `.split_many_last::<(2, 4)>() -> (&[T], &[T; 2], &[T; 4])`
- It is the ordering used in the `rsplit_array` methods

I think the inconsistency with `split_last` could be acceptable in this case, since for `split_last` the scalar `&T` doesn't have any internal order to maintain with the other items.

## Unresolved questions

Do we want to reserve the same names on `[u8; N]` to avoid inference confusion? rust-lang#117561 (comment)

---

`slice_first_last_chunk` has only been around since early 2023, but `split_array` has been around since 2021.

`@rustbot` label -T-libs +T-libs-api -T-libs +needs-fcp
cc `@rust-lang/wg-const-eval,` `@scottmcm` who raised this topic, `@clarfonthey` implementer of `slice_first_last_chunk` `@jethrogb` implementer of `split_array`

Zulip discussion: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Stabilizing.20array-from-slice.20*something*.3F

Fixes: rust-lang#111774
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 20, 2024
Rollup merge of rust-lang#117561 - tgross35:split-array, r=scottmcm

Stabilize `slice_first_last_chunk`

This PR does a few different things based around stabilizing `slice_first_last_chunk`. They are split up so this PR can be by-commit reviewed, I can move parts to a separate PR if desired.

This feature provides a very elegant API to extract arrays from either end of a slice, such as for parsing integers from binary data.

## Stabilize `slice_first_last_chunk`

ACP: rust-lang/libs-team#69
Implementation: rust-lang#90091
Tracking issue: rust-lang#111774

This stabilizes the functionality from rust-lang#111774:

```rust
impl [T] {
    pub const fn first_chunk<const N: usize>(&self) -> Option<&[T; N]>;
    pub fn first_chunk_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
    pub const fn last_chunk<const N: usize>(&self) -> Option<&[T; N]>;
    pub fn last_chunk_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
    pub const fn split_first_chunk<const N: usize>(&self) -> Option<(&[T; N], &[T])>;
    pub fn split_first_chunk_mut<const N: usize>(&mut self) -> Option<(&mut [T; N], &mut [T])>;
    pub const fn split_last_chunk<const N: usize>(&self) -> Option<(&[T], &[T; N])>;
    pub fn split_last_chunk_mut<const N: usize>(&mut self) -> Option<(&mut [T], &mut [T; N])>;
}
```

Const stabilization is included for all non-mut methods, which are blocked on `const_mut_refs`. This change includes marking the trivial function `slice_split_at_unchecked` const-stable for internal use (but not fully stable).

## Remove `split_array` slice methods

Tracking issue: rust-lang#90091
Implementation: rust-lang#83233 (review)

This PR also removes the following unstable methods from the `split_array` feature, rust-lang#90091:

```rust
impl<T> [T] {
    pub fn split_array_ref<const N: usize>(&self) -> (&[T; N], &[T]);
    pub fn split_array_mut<const N: usize>(&mut self) -> (&mut [T; N], &mut [T]);

    pub fn rsplit_array_ref<const N: usize>(&self) -> (&[T], &[T; N]);
    pub fn rsplit_array_mut<const N: usize>(&mut self) -> (&mut [T], &mut [T; N]);
}
```

This is done because discussion at rust-lang#90091 and its implementation PR indicate a strong preference for nonpanicking APIs that return `Option`. The only difference between functions under the `split_array` and `slice_first_last_chunk` features is `Option` vs. panic, so remove the duplicates as part of this stabilization.

This does not affect the array methods from `split_array`. We will want to revisit these once `generic_const_exprs` is further along.

## Reverse order of return tuple for `split_last_chunk{,_mut}`

An unresolved question for rust-lang#111774 is whether to return `(preceding_slice, last_chunk)` (`(&[T], &[T; N])`) or the reverse (`(&[T; N], &[T])`), from `split_last_chunk` and `split_last_chunk_mut`. It is currently implemented as `(last_chunk, preceding_slice)` which matches `split_last -> (&T, &[T])`. The first commit changes these to `(&[T], &[T; N])` for these reasons:

- More consistent with other splitting methods that return multiple values: `str::rsplit_once`, `slice::split_at{,_mut}`, `slice::align_to` all return tuples with the items in order
- More intuitive (arguably opinion, but it is consistent with other language elements like pattern matching `let [a, b, rest @ ..] ...`
- If we ever added a varidic way to obtain multiple chunks, it would likely return something in order: `.split_many_last::<(2, 4)>() -> (&[T], &[T; 2], &[T; 4])`
- It is the ordering used in the `rsplit_array` methods

I think the inconsistency with `split_last` could be acceptable in this case, since for `split_last` the scalar `&T` doesn't have any internal order to maintain with the other items.

## Unresolved questions

Do we want to reserve the same names on `[u8; N]` to avoid inference confusion? rust-lang#117561 (comment)

---

`slice_first_last_chunk` has only been around since early 2023, but `split_array` has been around since 2021.

`@rustbot` label -T-libs +T-libs-api -T-libs +needs-fcp
cc `@rust-lang/wg-const-eval,` `@scottmcm` who raised this topic, `@clarfonthey` implementer of `slice_first_last_chunk` `@jethrogb` implementer of `split_array`

Zulip discussion: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Stabilizing.20array-from-slice.20*something*.3F

Fixes: rust-lang#111774
@tgross35
Copy link
Contributor

This should be reopened, I'm not sure why #117561 closed it. Also the tracking issue can be updated to remove the impl<T> [T] block, #117561 removed it (and stabilized the slice_first_last_chunk in its place, yay!)

@scottmcm
Copy link
Member

Oh, that's my fault -- I saw that it removed split_array_ref and such and thought it was everything.

@scottmcm scottmcm reopened this Jan 20, 2024
@tgross35
Copy link
Contributor

Oh, no I only removed the slice components. I know we want a different API but figured that would just be a rework of this implementation.

But maybe it’s better to start something new with a clean tracking history. I can submit a PR to remove if so.

github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 21, 2024
Stabilize `slice_first_last_chunk`

This PR does a few different things based around stabilizing `slice_first_last_chunk`. They are split up so this PR can be by-commit reviewed, I can move parts to a separate PR if desired.

This feature provides a very elegant API to extract arrays from either end of a slice, such as for parsing integers from binary data.

## Stabilize `slice_first_last_chunk`

ACP: rust-lang/libs-team#69
Implementation: rust-lang/rust#90091
Tracking issue: rust-lang/rust#111774

This stabilizes the functionality from rust-lang/rust#111774:

```rust
impl [T] {
    pub const fn first_chunk<const N: usize>(&self) -> Option<&[T; N]>;
    pub fn first_chunk_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
    pub const fn last_chunk<const N: usize>(&self) -> Option<&[T; N]>;
    pub fn last_chunk_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
    pub const fn split_first_chunk<const N: usize>(&self) -> Option<(&[T; N], &[T])>;
    pub fn split_first_chunk_mut<const N: usize>(&mut self) -> Option<(&mut [T; N], &mut [T])>;
    pub const fn split_last_chunk<const N: usize>(&self) -> Option<(&[T], &[T; N])>;
    pub fn split_last_chunk_mut<const N: usize>(&mut self) -> Option<(&mut [T], &mut [T; N])>;
}
```

Const stabilization is included for all non-mut methods, which are blocked on `const_mut_refs`. This change includes marking the trivial function `slice_split_at_unchecked` const-stable for internal use (but not fully stable).

## Remove `split_array` slice methods

Tracking issue: rust-lang/rust#90091
Implementation: rust-lang/rust#83233 (review)

This PR also removes the following unstable methods from the `split_array` feature, rust-lang/rust#90091:

```rust
impl<T> [T] {
    pub fn split_array_ref<const N: usize>(&self) -> (&[T; N], &[T]);
    pub fn split_array_mut<const N: usize>(&mut self) -> (&mut [T; N], &mut [T]);

    pub fn rsplit_array_ref<const N: usize>(&self) -> (&[T], &[T; N]);
    pub fn rsplit_array_mut<const N: usize>(&mut self) -> (&mut [T], &mut [T; N]);
}
```

This is done because discussion at #90091 and its implementation PR indicate a strong preference for nonpanicking APIs that return `Option`. The only difference between functions under the `split_array` and `slice_first_last_chunk` features is `Option` vs. panic, so remove the duplicates as part of this stabilization.

This does not affect the array methods from `split_array`. We will want to revisit these once `generic_const_exprs` is further along.

## Reverse order of return tuple for `split_last_chunk{,_mut}`

An unresolved question for #111774 is whether to return `(preceding_slice, last_chunk)` (`(&[T], &[T; N])`) or the reverse (`(&[T; N], &[T])`), from `split_last_chunk` and `split_last_chunk_mut`. It is currently implemented as `(last_chunk, preceding_slice)` which matches `split_last -> (&T, &[T])`. The first commit changes these to `(&[T], &[T; N])` for these reasons:

- More consistent with other splitting methods that return multiple values: `str::rsplit_once`, `slice::split_at{,_mut}`, `slice::align_to` all return tuples with the items in order
- More intuitive (arguably opinion, but it is consistent with other language elements like pattern matching `let [a, b, rest @ ..] ...`
- If we ever added a varidic way to obtain multiple chunks, it would likely return something in order: `.split_many_last::<(2, 4)>() -> (&[T], &[T; 2], &[T; 4])`
- It is the ordering used in the `rsplit_array` methods

I think the inconsistency with `split_last` could be acceptable in this case, since for `split_last` the scalar `&T` doesn't have any internal order to maintain with the other items.

## Unresolved questions

Do we want to reserve the same names on `[u8; N]` to avoid inference confusion? rust-lang/rust#117561 (comment)

---

`slice_first_last_chunk` has only been around since early 2023, but `split_array` has been around since 2021.

`@rustbot` label -T-libs +T-libs-api -T-libs +needs-fcp
cc `@rust-lang/wg-const-eval,` `@scottmcm` who raised this topic, `@clarfonthey` implementer of `slice_first_last_chunk` `@jethrogb` implementer of `split_array`

Zulip discussion: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Stabilizing.20array-from-slice.20*something*.3F

Fixes: #111774
@tgross35
Copy link
Contributor

Haven't seen it linked here, the arrayref crate provides a way to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-array Area: `[T; N]` C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.