Mutable borrows in closures should be allowed multiple times

[Nokel81]

I know that the title is a bit strange but I couldn't think of something better.

The following code fails to compile but the second one succeeds. I think that both should be fine.

1.

fn load() -> io::Result<Vec<usize>> {
    ...

    for dir in [<paths>....] {
        store_first_err_ret_ok(&mut first_error, fs::read_dir(dir)).map(|dir| {
            dir
                .into_iter()
                .filter_map(|entry| store_first_err_ret_ok(&mut first_error, entry))
                .map(|e| File::open(e.path()))
                .filter_map(|entry| store_first_err_ret_ok(&mut first_error, entry))
                .for_each(|entry| res.push(entry.len()))
        });
    }

    ...
}

error:

error[E0499]: cannot borrow `first_error` as mutable more than once at a time
   |
   |             .filter_map(|entry| store_first_err_ret_ok(&mut first_error, entry))
   |                         -------                             ----------- first borrow occurs due to use of `first_error` in closure
   |                         |
   |                         first mutable borrow occurs here
   |             .map(|e| File::open(e.path()))
   |             .filter_map(|entry| store_first_err_ret_ok(&mut first_error, entry))
   |              ---------- ^^^^^^^                             ----------- second borrow occurs due to use of `first_error` in closure
   |              |          |
   |              |          second mutable borrow occurs here
   |              first borrow later used by call

2.

fn load() -> io::Result<Vec<usize>> {
    ...

    for dir in [<paths>....] {
        store_first_err_ret_ok(&mut first_error, fs::read_dir(dir)).map(|dir| {
            let inter = dir
                .into_iter()
                .filter_map(|entry| store_first_err_ret_ok(&mut first_error, entry))
                .collect();
            inter
                .into_iter()
                .map(|e| File::open(e.path()))
                .filter_map(|entry| store_first_err_ret_ok(&mut first_error, entry))
                .for_each(|entry| res.push(entry.len()))
        });
    }

    ...
}

I guess my quandry

[CAD97]

The problem here is that the compiler has no actual guarantee that the two filter_map don't run concurrently; in fact, with rayon, they could, with effectively the exact same API.

I agree ideally this should work, but I have no idea how it could work.

[Nokel81]

Could using !Send work. That would tell the compiler that none of the inputs will be sent across threads?

[CAD97]

Though the possibility of running the closures concurrently is the practical reason this specific API cannot support this, the mechanical reason is more complicated than that.

The actual reason that this errors is that both FilterMap (created by Iterator::filter_map)'s closures own a unique reference to first_error. These two points in the chain cannot both hold a reference at the same time.

What I've done personally is typically to keep threading Result through my pipeline, as painful as it can sometimes be. This requires a "blob" error type that can treat every error from the pipeline identically. Then the very end / the outer driver of the pipeline can tee the successes and errors into two collections. This is often aided by a transform of the form (T -> Result<U, E>) -> (Result<T, E> -> Result<U, E>) (this is roughly a monadic lift, I believe?) so that the inner layers don't have to explicitly acknowledge the outer errors.

[vincent163]

In such complicated cases you'll have to actually iterate them with for and handle them one by one manually instead of relying on combinators.

This is exactly why async/await was invented in favor of future combinators - people found that in order to borrow &mut references to multiple closures, they had to wrap them with Arc<Mutex<_>> which was ugly and low in performance.