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

const-generic array splitting #74674

Closed
jplatte opened this issue Jul 23, 2020 · 17 comments · Fixed by #83233
Closed

const-generic array splitting #74674

jplatte opened this issue Jul 23, 2020 · 17 comments · Fixed by #83233
Labels
A-const-generics Area: const generics (parameters and arguments) C-feature-request Category: A feature request, i.e: not implemented / a PR. F-const_generics `#![feature(const_generics)]` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jplatte
Copy link
Contributor

jplatte commented Jul 23, 2020

Creating a new issue about const-generic array splitting methods since it's been discussed on the array_chunks PR, where it really doesn't belong.

We probably want

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

// requires more const generics work to be accepted by rustc AFAIK
impl<T, const N: usize> [T; N] {
    fn split_array<const M: usize>(&self) -> (&[T; M], &[T; {N - M}]) { ... }
}

The possibility of a variadic array splitting function was also mentioned (at least that's my interpretation) but since there's not even an active RFC about variadic generics, I think that's a bit out of scope. The only thing one could do once the second split_array above becomes possible is add split_array_2, split_array_3 and so on.

@leonardo-m
Copy link

We also need two or more different use cases.

@jonas-schievink jonas-schievink added A-const-generics Area: const generics (parameters and arguments) F-const_generics `#![feature(const_generics)]` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jul 23, 2020
@withoutboats
Copy link
Contributor

withoutboats commented Jul 23, 2020

@DutchGhost pointed out that this could be implemented today by taking 2 const params, but the check that they subdivide the array evenly would have to be a runtime assertion.

impl [T; N]
    fn split<N2, N3>(&self) -> (&[T; N2] &[T; N3]) {
        assert_eq!(N, N2 + N3);
        // ...
    }
}

We could add the API on nightly with an awareness that we intend to change the API someday.

@DutchGhost
Copy link
Contributor

DutchGhost commented Jul 23, 2020

I made an implementation that errors at compiletime when you specify wrong array lengths!

use core::mem::size_of;

trait SplitCheck<const N2: usize, const N3: usize> {
    type LHS;
    type RHS;

    const ASSERT: bool;
    const __ASSERT: () = [()][(!Self::ASSERT) as usize];
}

impl<T, const N: usize, const N2: usize, const N3: usize> SplitCheck<{ N2 }, { N3 }> for [T; N] {
    type LHS = [T; N2];
    type RHS = [T; N3];

    const ASSERT: bool = size_of::<Self::LHS>() + size_of::<Self::RHS>() == size_of::<Self>();
}

impl<T, const N: usize> [T; N] {
    fn split_array<const N2: usize, const N3: usize>(&self) -> (&[T; N2], &[T; N3]) {
        let _: () = <Self as SplitCheck<{ N2 }, { N3 }>>::__ASSERT;

        let (lhs, rhs) = self.split_at(N2);

        unsafe {
            let lhs_array = &(*lhs.as_ptr().cast());
            let rhs_array = &(*rhs.as_ptr().cast());

            (lhs_array, rhs_array)
        }
    }
}

Here a playground link for a PoC: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=52d0664c958d9bca65935b8df6086c33

@jethrogb
Copy link
Contributor

use case: Parsing binary data into structs that contain fixed-length arrays

@withoutboats
Copy link
Contributor

I think something that uses traits to impose relational const constraints makes more sense outside of std right now.

@DutchGhost
Copy link
Contributor

DutchGhost commented Jul 23, 2020

I think something that uses traits to impose relational const constraints makes more sense outside of std right now.

yeah, the error message you get when the assert hits is far from pretty and usefull. I'd say its a tradeoff between wanting some ugly compile error whenever the lenghts are wrong, or taking that assertion and finding out the lenghts are wrong during runtime.

One thing to make the error a little bit nicer, is to write the following:

    const __ASSERT: &'static str =
        ["The total length of the splitted arrays does not equal the length of the original!"]
            [(!Self::ASSERT) as usize];

Now, if somehow the values of N, N2 and N3 could be displayed in that message, it would be even nicer

@leonardo-m
Copy link

Compile-time errors are better. This means we need to add something to Rust to perform similar static casts in a nicer and shorter way.

@withoutboats
Copy link
Contributor

@leonardo-m Aren't you the same leonardo from the internals forum who created a thread about this exact feature a week ago? I responded there with an explanation of the state of things in regard to that feature.

In any event the "correct" API is not to have two parameters and constrain them, but to derive the length of the second array by subtracting the a single parameter from the length of the main array. (-> (&[T; M], &[T; N - M])). But the current implementation of const generics does not support that.

Probably it would be better to hash out what APIs we could implement now, and whether its worth adding them to nightly, in a zulip thread than a GitHub issue.

@leonardo-m
Copy link

Aren't you the same leonardo from the internals forum who created a thread about this exact feature a week ago?

There I asked for a "where" clause on const integers, while here I've suggested about a more general const_assert that's usable in more/different cases.

@mbartlett21
Copy link
Contributor

mbartlett21 commented Jan 17, 2021

Here is an example implementation:

(EDIT: Updated features)

#![feature(const_refs_to_cell)]
#![feature(const_ptr_read)]
#![feature(const_ptr_offset)]
#![feature(generic_const_exprs)]

use core::mem::ManuallyDrop;

const fn split_arr<T, const N: usize, const M: usize>(arr: [T; N]) -> ([T; M], [T; N - M])
where
    [T; N - M]: ,
{
    let arr = ManuallyDrop::new(arr);
    let start_ptr = &arr as *const _ as *const T;
    let arr_1_ptr = start_ptr as *const [T; M];
    let arr_2_ptr = unsafe { start_ptr.add(M) } as *const [T; N - M];

    unsafe { (arr_1_ptr.read(), arr_2_ptr.read()) }
}

const ARR: [u8; 10] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];

const FIRST_3: [u8; 3] = split_arr(ARR).0;

// This last one doesn't work, since rustc can't infer the constant
// const LAST_7: [u8; 7] = split_arr(ARR).1;

// We just need to specify the generic params...
const LAST_7: [u8; 7] = split_arr::<u8, 10, 3>(ARR).1;

(playground)

@jethrogb
Copy link
Contributor

Is it possible to add [T; N]::split_array now but adjust the signature so that its use is always a compile-time error? I'd like to land the slice version ASAP but I don't want to run into the same problem we had with into_iter.

@jethrogb
Copy link
Contributor

Another use case: converting binary data to integers using {integer}::from_Xe_bytes

@jethrogb
Copy link
Contributor

Is it possible to add [T; N]::split_array now but adjust the signature so that its use is always a compile-time error? I'd like to land the slice version ASAP but I don't want to run into the same problem we had with into_iter.

This seems easy: just keep the array version unstable?

@slightlyoutofphase
Copy link
Contributor

This is 100% doable in current nightly rust, in a way that simply results in an immediate compile-time error if an invalid split index is provided, and is also completely const-compatible. Here's a playground link that does it as a const impl of the following trait (which is just a stand-in for the actual [T; N] type, of course):

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]);
}

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue 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 issue 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 bors closed this as completed in 5ea0274 Oct 23, 2021
@leonardo-m
Copy link

To split an array or slice with this functions I'm using:

let (a1, rest) = data.split_array_ref::<N>();
let (a2, empty) = rest.split_array_ref::<M>();
assert!(empty.is_empty());
(a1, a2)

I guess there isn't a desire for a array method similar to:

fn split_into_two_arrays<T, const N: usize, const M: usize>(data: &[T; N]) -> (&[T; M], &[T; N - M]) {

@jethrogb
Copy link
Contributor

I think I'd use split_array_ref followed by try_from on the rest.

@jethrogb
Copy link
Contributor

jethrogb commented Oct 25, 2021

Check the tracking issue #90091 for more details on exact-length splitting for arrays.

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) C-feature-request Category: A feature request, i.e: not implemented / a PR. F-const_generics `#![feature(const_generics)]` 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.

8 participants