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

Implement split_array and split_array_mut #83233

Merged
merged 1 commit into from
Oct 23, 2021
Merged

Conversation

jethrogb
Copy link
Contributor

@jethrogb jethrogb commented Mar 17, 2021

This implements [T]::split_array::<const N>() -> (&[T; N], &[T]) and [T; N]::split_array::<const M>() -> (&[T; M], &[T]) and their mutable equivalents. These are another few “missing” array implementations now that const generics are a thing, similar to #74373, #75026, etc. Fixes #74674.

This implements [T; N]::split_array returning an array and a slice. Ultimately, this is probably not what we want, we would want the second return value to be an array of length N-M, which will likely be possible with future const generics enhancements. We need to implement the array method now though, to immediately shadow the slice method. This way, when the slice methods get stabilized, calling them on an array will not be automatic through coercion, so we won't have trouble stabilizing the array methods later (cf. into_iter debacle).

An unchecked version of [T]::split_array could also be added as in #76014. This would not be needed for [T; N]::split_array as that can be compile-time checked. Edit: actually, since split_at_unchecked is internal-only it could be changed to be split_array-only.

@rust-highfive
Copy link
Collaborator

r? @sfackler

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 17, 2021
/// ```
// Note: this is not intended to be stabilized in its current form. The
// second element of the return value should instead be `&[T; {N - M}]`.
#[unstable(feature = "array_split_array", reason = "not intended for stabilization", issue = "74674")]
Copy link
Contributor Author

@jethrogb jethrogb Mar 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Need to create tracking issue if this is approved

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to approve, so go ahead and create the tracking issues for each feature being introduced.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@slightlyoutofphase
Copy link
Contributor

slightlyoutofphase commented Mar 22, 2021

For the array version of this, wouldn't it possibly make more sense to have it be entirely value-based so as to make it useful in a different way than the slice version? Something like this for example.

@jethrogb
Copy link
Contributor Author

That's pretty neat. That brings up a question about naming. For arrays you could have

  • split_array
  • split_array_ref
  • split_array_mut

But now array::split_array and slice::split_array are even more different.

@slightlyoutofphase
Copy link
Contributor

That's pretty neat. That brings up a question about naming. For arrays you could have

  • split_array
  • split_array_ref
  • split_array_mut

But now array::split_array and slice::split_array are even more different.

Well, like, arrays already have easy direct access to all the slice methods anyways via as_slice() and as_mut_slice() (and via the classic direct &arr[..] and &mut arr[..] syntax of course). I'd say overall the best API might ultimately actually be something like just having:

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

That way all three types of functionality are available where it's appropriate for them to be, and there's also no name confusion.

@jethrogb
Copy link
Contributor Author

You can't have [T]::split_array_ref<const M: usize>(&self) -> (&[T; M], &[T; N - M])

@slightlyoutofphase
Copy link
Contributor

slightlyoutofphase commented Mar 23, 2021

You can't have [T]::split_array_ref<const M: usize>(&self) -> (&[T; M], &[T; N - M])

Sorry, I got mixed up with my copy-pasting. It should have been:

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

So the second two are your existing slice ones, but with the immutable version called split_array_ref instead.

On a side note, I realized the value-based array one I gave as an example before can actually be made const, which is cool IMO.

@jethrogb
Copy link
Contributor Author

I think we'd still want the exact-remainder versions on array references. That's still possible with the names you suggest, but for slices now there's a weird naming difference between split_at and split_array_ref.

@slightlyoutofphase
Copy link
Contributor

I think we'd still want the exact-remainder versions on array references. That's still possible with the names you suggest, but for slices now there's a weird naming difference between split_at and split_array_ref.

That's fair enough. I personally don't actually care too much what the names end up being TBH. Primarily my point was just that I think the value based version would be useful to also have for arrays (regardless of what it's called).

@slightlyoutofphase
Copy link
Contributor

slightlyoutofphase commented Mar 23, 2021

Ok, here's a complete, perfectly working, Miri-passing const_trait_impl implementation of all three of the "exact remainder" array methods. There's definitely no technical barrier preventing any of them from being written with the desired signatures right now.

#![allow(incomplete_features)]
#![feature(
    box_syntax,
    const_evaluatable_checked,
    const_generics,
    const_intrinsic_copy,
    const_maybe_uninit_as_ptr,
    const_maybe_uninit_assume_init,
    const_mut_refs,
    const_panic,
    const_precise_live_drops,
    const_ptr_offset,
    const_raw_ptr_deref,
    // This feature should have a more general name considering what it actually does IMO.
    const_refs_to_cell,
    const_slice_from_raw_parts,
    const_trait_impl,
    slice_ptr_get,
)]

use core::mem::{self, MaybeUninit};
use core::ptr;

#[inline(always)]
const unsafe fn ptr_slice_to_array_mut<'a, T, const N: usize>(slice: *mut [T]) -> &'a mut [T; N] {
    &mut *(slice.as_mut_ptr() as *mut [T; N])
}

#[inline(always)]
const unsafe fn ptr_slice_to_array_ref<'a, T, const N: usize>(slice: *const [T]) -> &'a [T; N] {
    &*(slice.as_ptr() as *const [T; N])
}

// This is just a stand-in for `[T; N]`, to simulate how `N` *must* be initially declared somewhere
// entirely outside of `split_array` for it to work as it does below.
pub trait ArrayHelper<T, const N: usize> {
    fn split_array<const M: usize>(self) -> ([T; M], [T; N - M]);
    fn split_array_ref<const M: usize>(&self) -> (&[T; M], &[T; N - M]);
    fn split_array_mut<const M: usize>(&mut self) -> (&mut [T; M], &mut [T; N - M]);
}

impl<T, const N: usize> const ArrayHelper<T, N> for [T; N] {
    // It seems that `const_evaluatable_checked` itself makes these "just work" without even needing
    // a `where` clause.
    #[inline]
    fn split_array<const M: usize>(self) -> ([T; M], [T; N - M]) {
        assert!(M <= N, "Bounds check failure in `[T; N]::split_array'!");
        let mut left = MaybeUninit::<[T; M]>::uninit();
        let mut right = MaybeUninit::<[T; N - M]>::uninit();
        let self_ptr = self.as_ptr();
        let left_ptr = left.as_mut_ptr() as *mut T;
        let right_ptr = right.as_mut_ptr() as *mut T;
        unsafe {
            self_ptr.copy_to_nonoverlapping(left_ptr, M);
            self_ptr.add(M).copy_to_nonoverlapping(right_ptr, N - M);
            mem::forget(self);
            (left.assume_init(), right.assume_init())
        }
    }

    #[inline]
    fn split_array_ref<const M: usize>(&self) -> (&[T; M], &[T; N - M]) {
        assert!(M <= N, "Bounds check failure in `[T; N]::split_array_ref'!");
        let self_ptr = self.as_ptr();
        unsafe {
            // `ptr::slice_from_raw_parts` is a `const fn`. `slice::from_raw_parts` is not.
            let left = ptr::slice_from_raw_parts(self_ptr, M);
            let right = ptr::slice_from_raw_parts(self_ptr.add(M), N - M);
            (ptr_slice_to_array_ref(left), ptr_slice_to_array_ref(right))
        }
    }

    #[inline]
    fn split_array_mut<const M: usize>(&mut self) -> (&mut [T; M], &mut [T; N - M]) {
        assert!(M <= N, "Bounds check failure in `[T; N]::split_array_mut'!");
        let self_ptr = self.as_mut_ptr();
        unsafe {
            // `ptr::slice_from_raw_parts_mut` is a `const fn`. `slice::from_raw_parts_mut` is not.
            let left = ptr::slice_from_raw_parts_mut(self_ptr, M);
            let right = ptr::slice_from_raw_parts_mut(self_ptr.add(M), N - M);
            (ptr_slice_to_array_mut(left), ptr_slice_to_array_mut(right))
        }
    }
}

const X1: ([usize; 2], [usize; 4]) = [1, 2, 3, 4, 5, 6].split_array::<2>();
const Y1: ([usize; 0], [usize; 6]) = [1, 2, 3, 4, 5, 6].split_array::<0>();
const Z1: ([usize; 6], [usize; 0]) = [1, 2, 3, 4, 5, 6].split_array::<6>();
const X2: (&[usize; 2], &[usize; 4]) = [1, 2, 3, 4, 5, 6].split_array_ref::<2>();
const Y2: (&[usize; 0], &[usize; 6]) = [1, 2, 3, 4, 5, 6].split_array_ref::<0>();
const Z2: (&[usize; 6], &[usize; 0]) = [1, 2, 3, 4, 5, 6].split_array_ref::<6>();

fn main() {
    // Boxing everything ensures that Miri will catch anything being done incorrectly.
    let a = [box 1, box 2, box 3, box 4, box 5, box 6].split_array::<2>();
    println!("{:?}", a.0);
    println!("{:?}\n", a.1);
    let b = [box 1, box 2, box 3, box 4, box 5, box 6].split_array::<0>();
    println!("{:?}", b.0);
    println!("{:?}\n", b.1);
    let c = [box 1, box 2, box 3, box 4, box 5, box 6].split_array::<6>();
    println!("{:?}", c.0);
    println!("{:?}\n", c.1);
    let d = [box 1, box 2, box 3, box 4, box 5, box 6];
    let e = d.split_array_ref::<2>();
    println!("{:?}", e.0);
    println!("{:?}\n", e.1);
    let f = d.split_array_ref::<0>();
    println!("{:?}", f.0);
    println!("{:?}\n", f.1);
    let g = d.split_array_ref::<6>();
    println!("{:?}", g.0);
    println!("{:?}\n", g.1);
    let mut h = [box 1, box 2, box 3, box 4, box 5, box 6];
    let i = h.split_array_mut::<2>();
    for (l, r) in i.0.iter_mut().zip(i.1.iter_mut()) {
        *l.as_mut() += *r.as_ref();
        *r.as_mut() += *l.as_ref();
    }
    println!("{:?}", i.0);
    println!("{:?}\n", i.1);
    let j = h.split_array_mut::<0>();
    for (l, r) in j.0.iter_mut().zip(j.1.iter_mut()) {
        *l.as_mut() += *r.as_ref();
        *r.as_mut() += *l.as_ref();
    }
    println!("{:?}", j.0);
    println!("{:?}\n", j.1);
    let k = h.split_array_mut::<6>();
    for (l, r) in k.0.iter_mut().zip(k.1.iter_mut()) {
        *l.as_mut() += *r.as_ref();
        *r.as_mut() += *l.as_ref();
    }
    println!("{:?}", k.0);
    println!("{:?}\n", k.1);
    // Make sure constant usage works properly also
    println!("{:?}", X1.0);
    println!("{:?}\n", X1.1);
    println!("{:?}", Y1.0);
    println!("{:?}\n", Y1.1);
    println!("{:?}", Z1.0);
    println!("{:?}\n", Z1.1);
    println!("{:?}", X2.0);
    println!("{:?}\n", X2.1);
    println!("{:?}", Y2.0);
    println!("{:?}\n", Y2.1);
    println!("{:?}", Z2.0);
    println!("{:?}\n", Z2.1);
}

Playground link.

Edit: Something cool I realized later on yesterday about the full-on "exact remainder" versions above, also, is that the in-function assertions are actually technically not necessary, because they're unreachable. rustc straight-up won't let you call the function to begin with if M > N.

For example, if you do this:

let a = [box 1, box 2, box 3, box 4, box 5, box 6].split_array::<9000>();

you get this:

   Compiling playground v0.0.1 (/playground)
error[E0080]: evaluation of constant value failed
  --> src/main.rs:38:58
   |
38 |     fn split_array<const M: usize>(self) -> ([T; M], [T; N - M]);
   |                                                          ^^^^^ attempt to compute `6_usize - 9000_usize`, which would overflow

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 26, 2021
@bstrie bstrie added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2021
@JohnCSimon JohnCSimon removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2021
@yaahc
Copy link
Member

yaahc commented Oct 21, 2021

I think this is looking good, though I would like to add tests before merging. Here are the relevant tests that I can think of:

  • #[should_panic] test for when M > N
  • M = 0
  • N - M = 0
  • 0 < M < N

And of course feel free to add any other tests you can think of. I'll r+ after the tests are added. Thanks for working on this!

@jethrogb
Copy link
Contributor Author

There is not a single unit test in core. Is it ok to add some or do things work differently here?

@jethrogb
Copy link
Contributor Author

Found some integration tests that look like unit tests, added it there. Together with the doc tests, all those cases are covered now.

@rust-log-analyzer

This comment has been minimized.

@yaahc
Copy link
Member

yaahc commented Oct 22, 2021

I'm surprised to hear that core has no unit tests and I don't know of any restriction against adding them, but doing it as an integration test like you did works perfectly too, thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 22, 2021

📌 Commit 4a43976 has been approved by yaahc

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 22, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 22, 2021
Implement split_array and split_array_mut

This implements `[T]::split_array::<const N>() -> (&[T; N], &[T])` and `[T; N]::split_array::<const M>() -> (&[T; M], &[T])` and their mutable equivalents. These are another few “missing” array implementations now that const generics are a thing, similar to rust-lang#74373, rust-lang#75026, etc. Fixes rust-lang#74674.

This implements `[T; N]::split_array` returning an array and a slice. Ultimately, this is probably not what we want, we would want the second return value to be an array of length N-M, which will likely be possible with future const generics enhancements. We need to implement the array method now though, to immediately shadow the slice method. This way, when the slice methods get stabilized, calling them on an array will not be automatic through coercion, so we won't have trouble stabilizing the array methods later (cf. into_iter debacle).

An unchecked version of `[T]::split_array` could also be added as in rust-lang#76014. This would not be needed for `[T; N]::split_array` as that can be compile-time checked. Edit: actually, since split_at_unchecked is internal-only it could be changed to be split_array-only.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 22, 2021
Implement split_array and split_array_mut

This implements `[T]::split_array::<const N>() -> (&[T; N], &[T])` and `[T; N]::split_array::<const M>() -> (&[T; M], &[T])` and their mutable equivalents. These are another few “missing” array implementations now that const generics are a thing, similar to rust-lang#74373, rust-lang#75026, etc. Fixes rust-lang#74674.

This implements `[T; N]::split_array` returning an array and a slice. Ultimately, this is probably not what we want, we would want the second return value to be an array of length N-M, which will likely be possible with future const generics enhancements. We need to implement the array method now though, to immediately shadow the slice method. This way, when the slice methods get stabilized, calling them on an array will not be automatic through coercion, so we won't have trouble stabilizing the array methods later (cf. into_iter debacle).

An unchecked version of `[T]::split_array` could also be added as in rust-lang#76014. This would not be needed for `[T; N]::split_array` as that can be compile-time checked. Edit: actually, since split_at_unchecked is internal-only it could be changed to be split_array-only.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 23, 2021
…askrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#83233 (Implement split_array and split_array_mut)
 - rust-lang#88300 (Stabilise unix_process_wait_more, extra ExitStatusExt methods)
 - rust-lang#89416 (nice_region_error: Include lifetime placeholders in error output)
 - rust-lang#89468 (Report fatal lexer errors in `--cfg` command line arguments)
 - rust-lang#89730 (add feature flag for `type_changing_struct_update`)
 - rust-lang#89920 (Implement -Z location-detail flag)
 - rust-lang#90070 (Add edition configuration to compiletest)
 - rust-lang#90087 (Sync rustfmt subtree)
 - rust-lang#90117 (Make RSplit<T, P>: Clone not require T: Clone)
 - rust-lang#90122 (CI: make docker cache download and `docker load` time out after 10 minutes)
 - rust-lang#90166 (Add comment documenting why we can't use a simpler solution)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5ea0274 into rust-lang:master Oct 23, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 23, 2021
@lcnr lcnr added the A-const-generics Area: const generics (parameters and arguments) label Dec 11, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request 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 pull request 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
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

const-generic array splitting