0

This code doesn't compile:

fn drain_some<'a>(
    vals: &'a mut Vec<Vec<i32>>,
    inds: &'a [usize],
) -> impl Iterator<Item = i32> + 'a {
    inds.iter().flat_map(|i| vals[*i].drain(..))
}

Because (as I understand it) the draining iterator holds a mutable reference (vals) that was captured by a FnMut's, and you can't return that from the FnMut. But it seems like it should be possible, since the closure doesn't actually get called again until the draining iterator is dropped.

Is there another approach to this that works?

Dan
  • 12,409
  • 3
  • 50
  • 87
  • 1
    you need to take your time when asking. If you edit the question then you just make people that is trying to help you miss stuff. It is better that you open a newly related question IMO. – Netwave Nov 13 '21 at 11:28
  • @Netwave Sorry about that, I only realized it was an issue after I saw the answers. – Dan Nov 13 '21 at 11:31

2 Answers2

2

You can replace drain(..) with std::mem::take(), which should have the same effect without requiring multiple mut references into vals at the same time. This compiles:

fn drain_some<'a>(
    vals: &'a mut Vec<Vec<i32>>,
    inds: &'a [usize],
) -> impl Iterator<Item = i32> + 'a {
    inds.iter().flat_map(|i| std::mem::take(&mut vals[*i]))
}

Playground

user4815162342
  • 141,790
  • 18
  • 296
  • 355
  • oh nice, will this keep an empty vec in the original vals vector? Can you explain why take works and drain not? – Netwave Nov 13 '21 at 11:15
  • 1
    @Netwave Correct, `std::mem::take(foo)` is just shorthand for `std::mem::replace(foo, Default::default())`. I believe the difference is that `take()` returns an owned object and breaks the lifetime relationship between the original reference into `vals[i]` and the returned value. `drain()` on the other hand keeps that relationship, and the borrow checker cannot prove that `flat_map()` won't hold on to multiple `mut` references into `vals` at once. (That's at least my mental model, I'm not 100% sure.) – user4815162342 Nov 13 '21 at 11:18
1

You can just collect the drained iterator to take ownership of it:

fn drain_some<'a>(
    vals: &'a mut Vec<Vec<i32>>,
    inds: &'a [usize],
) -> impl Iterator<Item = i32> + 'a {
    inds.iter().flat_map(|i| vals[*i].drain(..).collect::<Vec<_>>())
}

Playground

Netwave
  • 40,134
  • 6
  • 50
  • 93
  • `remove()` has the downside of being O(n), which would be an issue if `val` contains a large number of small inner vectors. – user4815162342 Nov 13 '21 at 11:15
  • @user4815162342, yeah, I didnt knew if they wanted to keep and empty vector or just clear it from the origianl vector though. Im tracking your answer which is pretty interesting :D – Netwave Nov 13 '21 at 11:16
  • @user4815162342, I updated it. A simple solution is taking ownership of the drained iterator, which is the same idea behind `take` BUT probably worst, since it will create a new vector and fill it instead of an empty vector and taking the old one. Am I right? – Netwave Nov 13 '21 at 11:26
  • The OP's code seemed to intend leaving behind a non-empty outer vector of empty inner vectors, which my answer (and your updated answer) replicates. I believe your updated answer is algorithmically correct, but unnecessarily allocates and copies data from inner vectors into freshly allocated ones. (`` is optimized to reuse the storage if you collect `vec.into_iter()` into a `Vec` without ever advancing the iterator, but that can't be done for `vec.drain(..)` because it is supposed to retain the original vector's storage.) – user4815162342 Nov 13 '21 at 11:33
  • @user4815162342, makes sense, It was my guess. Thanks for the detailed explanation :) – Netwave Nov 13 '21 at 11:36