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

ACP: Provide TryFrom<&[T]> implementation for &[[T; N]] #201

Open
mina86 opened this issue Apr 1, 2023 · 9 comments
Open

ACP: Provide TryFrom<&[T]> implementation for &[[T; N]] #201

mina86 opened this issue Apr 1, 2023 · 9 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@mina86
Copy link

mina86 commented Apr 1, 2023

Proposal

Problem statement

Provide TryFrom<&[T]> implementation for &[[T; N]] as a more convenient wrapper around [T]::as_chunks.

Motivation, use-cases

Unstable [T]::as_chunks method allows converting &[T] into &[[T; N]]. However, to handle slices whose length doesn’t divide by N the method returns (chunks, remainder) tuple. If one wants to assert an empty remainder they may need to resort to destructuring the tuple and introducing if statement. Similarly, it doesn’t mesh nicely with method chaining. A chain of methods needs to be stopped to verify the second element of the returned tuple before proceeding.

Introducing TryFrom<&[T]> implementation for &[[T; N]] offers a Result-based interface for chunking a slice. Result integrates with many interfaces of the language which results in more concise code.

Compare:

// Before:

fn pairs(slice: &[u8]) -> Result<&[[u8; 2]]> {
    let (chunks, remainder) = slice.as_chunks::<2>();
    if remainder.is_empty() {
        Ok(chuns)
    } else {
        Err(SomeError)
    }
}

// Alternatively with let-else:

fn pairs(slice: &[u8]) -> Result<&[[u8; 2]]> {
    let (chunks, []) = slice.as_chunks::<2>() else {
        return Err(SomeError);
    };
    Ok(chunks)
}

// After, more concise with a single expression:

fn pairs(slice: &[u8]) -> Result<&[[u8; 2]]> {
    slice.try_into().map_err(|_| SomeError)
}
// Before, with use of let-else:

let bytes = get_bytes_in_a_vector();
let (chunks, []) = bytes.as_chunks() else {
    return Err(SomeError.into());
};
let u64s = chunks
    .into_iter()
    .map(|chunk| u64::from_le_bytes(*bytes))
    .collect::<Vec<_>>()

// After, single statement with no need for temporary variables:

let u64s = get_bytes_in_a_vector()
    .as_slice()
    .try_into::<&[[u8; 8]]>(bytes)
    .map_err(|_| SomeError)?
    .into_iter()
    .map(|chunk| u64::from_le_bytes(*bytes))
    .collect::<Vec<_>>();

Solution sketches

Solution is implemented in rust-lang/rust#105316 and has the form:

impl<'a, T, const N: usize> TryFrom<&'a [T]> for &'a [[T; N]] {
    type Error = TryFromSliceError;

    fn try_from(slice: &'a [T]) -> Result<Self, Self::Error> {
        let (chunks, remainder) = slice.as_chunks::<N>();
        if remainder.is_empty() {
            Ok(chunks)
        } else {
            Err(TryFromSliceError(()))
        }
    }
}
@mina86 mina86 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Apr 1, 2023
@mina86 mina86 changed the title (My API Change Proposal) ACP: Provide TryFrom<&[T]> implementation for &[[T; N]] Apr 1, 2023
@dead-claudia
Copy link

Not sure why this is more useful than the method that returns both. At least with as_chunks, you can iterate over the chunks that do exist, even if it doesn't fit evenly.

@mina86
Copy link
Author

mina86 commented Apr 6, 2023

It’s useful if length of the slice not being divisible by the length of a chunk is an error. If you want to go through all the available chunks regardless of remainder than indeed you wouldn’t use this.

@scottmcm
Copy link
Member

scottmcm commented Apr 6, 2023

How do you propose that this impl handles the N == 0 case?

@mina86
Copy link
Author

mina86 commented Apr 6, 2023

Ideally it wouldn’t compile (which also makes me realise that suggested solution doesn’t currently do that but that can be addressed with a simple compile-time assert).

@joshtriplett
Copy link
Member

joshtriplett commented Mar 5, 2024

@mina86 We reviewed this in today's @rust-lang/libs-api meeting, and there were objections to adding a TryFrom impl like this that changes the "shape" of the array, but we were generally happy with the idea of adding a named method for this (e.g. returning Option).

@joshtriplett
Copy link
Member

Also, separately, array_chunks might help.

@m-ou-se
Copy link
Member

m-ou-se commented Mar 5, 2024

This is the objection I shared in the meeting:

I don't think we should have (Try)From implementations that change the "shape" of a data structure like this. Such conversions imply that nested [ and ] are just "noise" that can be ignored, rather than a significant and meaningful part of the data structure.

(If we accept this ACP, would we also have to add all kind of other (Try)From impls that do various forms of flattening and (re-)grouping?)

Having a method for this, however, sounds good to me!

we decided

To clarify, this wasn't a full team decision, but additions like these require team consensus, which cannot be reached when there are objections from team members.

Adding and stabilizing a method for this still requires a team decision (through FCP), but based on the temperature check in the meeting, it seems likely that'd be accepted.

@scottmcm
Copy link
Member

scottmcm commented Mar 5, 2024

The meeting notes mention the possibility of as_chunks_exact, which sounds good to me. It could go under rust-lang/rust#74985 with the rest of the things there, and has the same "needs an answer for N == 0" problem that the rest of them do.

(It's also nice in that it can talk about how as_chunks and as_rchunks give the same results if as_chunks_exact would succeed.)


And +1 to not being fond of this as a trait. I think under the https://doc.rust-lang.org/std/convert/trait.From.html#when-to-implement-from vernacular, it's not "value-preserving" because the "conceptual kind" of the answer is different.

@the8472
Copy link
Member

the8472 commented Mar 5, 2024

The _exact variant does not necessarily suffer from the N==0 case if it returns an option because we can always return None in that case.

It would make the pairs example even simpler

fn pairs(slice: &[u8]) -> Option<&[[u8; 2]]> {
    slice.as_chunks_exact()
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

6 participants