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

Methods for extracting 1 element out of an owned slice and dropping the rest #376

Closed
its-the-shrimp opened this issue May 3, 2024 · 6 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@its-the-shrimp
Copy link

Proposal

Make extracting 1 element out of an owned slice and dropping the rest more straightforward by introducing an into_nth method to the 3 main owned slice types: Vec<T>, Box<[T]>, [T; N]

Problem statement

Turning an owned slice into 1 of its elements is in theory a trivial task, yet the APIs of the 3 main owned slice types and the functionality of Index{Mut} traits makes it unjustifiably harder.

There are 3 main ways to do so, but they're more of the "best effort" type of solution:

  • owned_slice.get_mut(n).map(std::mem::take)
    • Doesn't consume the slice, requiring one to mark it as mut even though you don't intend to mutate it;
    • Only works for T: Default;
  • owned_slice.swap_remove(n)
    • Doesn't consume the slice, requiring one to mark it as mut even though you don't intend to mutate it;
    • Only exists for Vec;
  • owned_slice.into_iter().nth(n)
    • Makes the slice irretrievable on failure.

There's another disadvantage that's mutual for all the 3 approaches above and for anything else there might be:
they're not the obvious solution, not the standard one. This means that:

  • They're not beginner-friendly;
  • They can't be suggested as a hint from the compiler to make them more beginner-friendly.

If a standard method for such an extraction were to exist, the following hint could be added (at the end):

error[E0507]: cannot move out of index of `Vec<String>`
 --> src/main.rs:3:1
  |
3 | v[0]
  | ^^^^ move occurs because value has type `String`, which does not implement the `Copy` trait
  |
help: consider cloning the value if the performance cost is acceptable
  |
3 | v[0].clone()
  |     ++++++++
help: if the vector isn't needed anymore, consider using the `into_nth` method
  |
3 | v.into_nth(0).unwrap()
  |

For more information about this error, try `rustc --explain E0507`.

Solution sketch

impl<T> Vec<T> {
    pub fn into_nth(self, n: usize) -> Result<T, Self> { ... }
}

impl<T> [T] {
    pub fn into_nth(self: Box<Self>, n: usize) -> Result<T, Box<Self>> { ... }
}

impl<const N: usize, T> [T; N] {
    pub fn into_nth(self, n: usize) -> Result<T, [T; N]> { ... }
}

Alternatives

The only alternate solution here is a trait for by-value indexing to control the behaviour of an indexing expression, but there are no plans for this as far as I'm aware, this approach would take much longer, and wouldn't be worth the wait since methods can exist just as well in the meantime.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@its-the-shrimp its-the-shrimp added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels May 3, 2024
@its-the-shrimp its-the-shrimp changed the title Methods for consuming a slice & returning 1 element Methods for extracting 1 element out of an owned slice and dropping the rest May 3, 2024
@clarfonthey
Copy link

I don't think that there's anything in particular that's wrong with this API; I'm just struggling to understand how this could be a useful enough API to justify its existence. Isolating a single element of a list and dropping the rest feels like a wasteful operation that shouldn't be done in most cases.

@pitaj
Copy link

pitaj commented May 4, 2024

Yeah, I wonder if there's a more generic solution that would enable this use-case but would enable other use-cases as well. Maybe change the API to return an array instead?

@kennytm
Copy link
Member

kennytm commented May 4, 2024

for [T; N] you can use slice pattern to extract an item near the start or end

let v: [T; N] = ...;
{...}
let [v0, ..] = v;

unfortunately this doesn't work for Box<[T]>.

@clarfonthey
Copy link

Also worth mentioning that for boxed slices, converting to a vec first is relatively painless and requires just one method call, and generally if you plan to do mutation you should keep it as a vec anyway instead of stripping the excess capacity.

@its-the-shrimp
Copy link
Author

Isolating a single element of a list and dropping the rest feels like a wasteful operation that shouldn't be done in most cases.

Ideally it's true, but a lot of times you don't have control over the API, it gives you a vector but in reality you only really care about one element, you might not be able to just not construct the vector/boxed slice/array from the start.

converting to a vec first is relatively painless

Yet you need to come up with it, there's no straightforward answer to extracting 1 element from a boxed slice.

generally if you plan to do mutation you should keep it as a vec anyway

True, but, again, one doesn't always get to change the API.

Pattern matching is yet another way to extract the first/last element, but also very limited; there are multiple approaches and not one if them is universal enough, I think having 1 intended way to do such a trivial indexing operation would make the language more beginner-friendly overall, since having 1 intended method means the compiler can recommend it when it encounters an expression of the form v[n] where the result is a non-Copy value.

@dtolnay
Copy link
Member

dtolnay commented May 19, 2024

We discussed this ACP in this week's standard library API team meeting, and those present were not convinced that this API would be broadly applicable enough to get a dedicated method. We would recommend continuing to use the already available approach using .into_iter().nth(n) (once rust-lang/rust#124097 is available for the Box<[T]> case).

if n < owned_slice.len() {
    Ok(owned_slice.into_iter().nth(n).unwrap())
} else {
    Err(owned_slice)
}

Or for a contiguous range of elements as mentioned in #376 (comment):

if offset + N < owned_slice.len() {
    let mut iter = owned_slice.into_iter().skip(offset);
    Ok([(); N].map(|_| iter.next().unwrap()))
} else {
    Err(owned_slice)
}

If any of Vec<T> or Box<[T]> or [T; N]'s IntoIter do not already have an optimized implementation of nth, I would be prepared to review a PR adding that.

I would also accept a standard library docs PR pointing out the .into_iter().nth(n) approach somewhere that the reader would look for such a thing; maybe https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.get.

@dtolnay dtolnay closed this as completed May 21, 2024
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

5 participants