How do I avoid incurring in lifetime issues when refactoring a function?

Viewed 95

Playground if you want to jump directly into the code.

Problem

I'm trying to implement a function filter_con<T, F>(v: Vec<T>, predicate: F) that allows concurrent filter on a Vec, via async predicates.

That is, instead of doing:

let arr = vec![...];
let arr_filtered = join_all(arr.into_iter().map(|it| async move {
    if some_getter(&it).await > some_value {
        Some(it)
    } else {
        None
    }
}))
.await
.into_iter()
.filter_map(|it| it)
.collect::<Vec<T>>()

every time I need to filter for a Vec, I want to be able to:

let arr = vec![...];
let arr_filtered = filter_con(arr, |it| async move { 
  some_getter(&it).await > some_value 
}).await

Tentative implementation

I've extracted the function into its own but I am incurring in lifetime issues

async fn filter_con<T, B, F>(arr: Vec<T>, predicate: F) -> Vec<T>
where
    F: FnMut(&T) -> B,
    B: futures::Future<Output = bool>,
{
    join_all(arr.into_iter().map(|it| async move {
        if predicate(&it).await {
            Some(it)
        } else {
            None
        }
    }))
    .await
    .into_iter()
    .filter_map(|p| p)
    .collect::<Vec<_>>()
}
error[E0507]: cannot move out of a shared reference

I don't know what I'm moving out of predicate?

For more details, see the playground.

1 Answers

You won't be able to make the predicate an FnOnce, because, if you have 10 items in your Vec, you'll need to call the predicate 10 times, but an FnOnce only guarantees it can be called once, which could lead to something like this:

let vec = vec![1, 2, 3];
let has_drop_impl = String::from("hello");

filter_con(vec, |&i| async {
  drop(has_drop_impl);
  i < 5
}

So F must be either an FnMut or an Fn. The standard library Iterator::filter takes an FnMut, though this can be a source of confusion (it is the captured variables of the closure that need a mutable reference, not the elements of the iterator).

Because the predicate is an FnMut, any caller needs to be able to get an &mut F. For Iterator::filter, this can be used to do something like this:

let vec = vec![1, 2, 3];
let mut count = 0;

vec.into_iter().filter(|&x| {
  count += 1;  // this line makes the closure an `FnMut`
  x < 2
})

However, by sending the iterator to join_all, you are essentially allowing your async runtime to schedule these calls as it wants, potentially at the same time, which would cause an aliased &mut T, which is always undefined behaviour. This issue has a slightly more cut down version of the same issue https://github.com/rust-lang/rust/issues/69446.

I'm still not 100% on the details, but it seems the compiler is being conservative here and doesn't even let you create the closure in the first place to prevent soundness issues.

I'd recommend making your function only accept Fns. This way, your runtime is free to call the function however it wants. This does means that your closure cannot have mutable state, but this is unlikely to be a problem in a tokio application. For the counting example, the "correct" solution is to use an AtomicUsize (or equivalent), which allows mutation via shared reference. If you're referencing mutable state in your filter call, it should be thread safe, and thread safe data structures generally allow mutation via shared reference.

Given that restriction, the following gives the answer you expect:

async fn filter_con<T, B, F>(arr: Vec<T>, predicate: F) -> Vec<T>
where
    F: Fn(&T) -> B,
    B: Future<Output = bool>,
{
    join_all(arr.into_iter().map(|it| async {
        if predicate(&it).await {
            Some(it)
        } else {
            None
        }
    }))
    .await
    .into_iter()
    .filter_map(|p| p)
    .collect::<Vec<_>>()
}

Playground

Related