(Solved) Problems using closures inside a loop

guillegalor · April 2, 2019, 2:36pm

Hello!

First of all, sorry about my English. I've been struggling with a compilation problem related to using a closure inside a while loop. This is a simplification of my real code to show the problem.

fn test(rng: &mut StdRng) {
    let num_of_indexes = 20;
    let mut some_value: f32 = 0.0;
    let normal = Normal::new(0.0, 0.3);

    // Initialize vector of indexes and shuffles it
    let mut indexes: Vec<usize> = (0..num_of_indexes).collect();
    indexes.shuffle(rng);

    // Closure that fills and shuffle indexes
    let mut fill_and_shuffle = || {
        indexes = (0..num_of_indexes).collect();
        indexes.shuffle(rng);
    };

    // If i call the closure outside the loop it works perfectly
    fill_and_shuffle();
    fill_and_shuffle();

    for _ in 0..3 {
        // In real code I have to do this multiple times
        if indexes.is_empty() {
            indexes = (0..num_of_indexes).collect();
            indexes.shuffle(rng);
        }

        // If I uncomment this, it brokes
        // fill_and_shuffle();

        let index = indexes.pop().expect("El vector está vacio");

        // Mutation
        some_value += normal.sample(rng) as f32;
    }
}

It works fine if I use it outside the loop, but if I call it inside the loop, it doesn't compile and return this error message:

error[E0502]: cannot borrow indexes as immutable because it is also borrowed as mutable
--> src/main.rs:928:12
|
917 | let mut fill_and_shuffle = || {
| -- mutable borrow occurs here
918 | indexes = (0..num_of_indexes).collect();
| ------- first borrow occurs due to use of indexes in closure
...
928 | if indexes.is_empty() {
| ^^^^^^^ immutable borrow occurs here
...
934 | fill_and_shuffle();
| ---------------- mutable borrow used here, in later iteration of loop

error[E0506]: cannot assign to indexes because it is borrowed
--> src/main.rs:929:13
|
917 | let mut fill_and_shuffle = || {
| -- borrow of indexes occurs here
918 | indexes = (0..num_of_indexes).collect();
| ------- borrow occurs due to use in closure
...
929 | indexes = (0..num_of_indexes).collect();
| ^^^^^^^ assignment to borrowed indexes occurs here
...
934 | fill_and_shuffle();
| ---------------- borrow used here, in later iteration of loop

error[E0499]: cannot borrow indexes as mutable more than once at a time
--> src/main.rs:930:13
|
917 | let mut fill_and_shuffle = || {
| -- first mutable borrow occurs here
918 | indexes = (0..num_of_indexes).collect();
| ------- first borrow occurs due to use of indexes in closure
...
930 | indexes.shuffle(rng);
| ^^^^^^^ second mutable borrow occurs here
...
934 | fill_and_shuffle();
| ---------------- first borrow used here, in later iteration of loop

error[E0501]: cannot borrow *rng as mutable because previous closure requires unique access
--> src/main.rs:930:29
|
917 | let mut fill_and_shuffle = || {
| -- closure construction occurs here
918 | indexes = (0..num_of_indexes).collect();
919 | indexes.shuffle(rng);
| --- first borrow occurs due to use of rng in closure
...
930 | indexes.shuffle(rng);
| ^^^ second borrow occurs here
...
934 | fill_and_shuffle();
| ---------------- first borrow used here, in later iteration of loop

error[E0499]: cannot borrow indexes as mutable more than once at a time
--> src/main.rs:936:21
|
917 | let mut fill_and_shuffle = || {
| -- first mutable borrow occurs here
918 | indexes = (0..num_of_indexes).collect();
| ------- first borrow occurs due to use of indexes in closure
...
934 | fill_and_shuffle();
| ---------------- first borrow used here, in later iteration of loop
935 |
936 | let index = indexes.pop().expect("El vector está vacio");
| ^^^^^^^ second mutable borrow occurs here

error[E0501]: cannot borrow *rng as mutable because previous closure requires unique access
--> src/main.rs:939:37
|
917 | let mut fill_and_shuffle = || {
| -- closure construction occurs here
918 | indexes = (0..num_of_indexes).collect();
919 | indexes.shuffle(rng);
| --- first borrow occurs due to use of rng in closure
...
934 | fill_and_shuffle();
| ---------------- first borrow used here, in later iteration of loop
...
939 | some_value += normal.sample(rng) as f32;
| ^^^ second borrow occurs here

error: aborting due to 6 previous errors
I've read the doc and I've done some research, but I didn't find anything that explains me this(I'm new to Rust). Maybe Rust borrows variables when it enters a loop or something like that, I don't know.

Thanks in advance!

RustyYato · April 2, 2019, 3:17pm

This has to do with how closures are desugared, you can read about it in my blog here

https://krishnasannasi.github.io/rust/syntactic/sugar/2019/01/17/Closures-Magic-Functions

Basically, the problem is the same as this

fn test() {
    let mut x = 0;
    let mut change_x = || x += 1;

    x += 1;
    change_x();
}

Which when you read my blog, you will see that it is the same problem as this

fn test() {
    let mut x = 0;
    let y = &mut x;

    x += 1;
    *y += 1;
}

Which is ~~obviously~~ wrong according to Rust's aliasing rules.

Yandros · April 2, 2019, 11:03pm

Just to "soothe" @RustyYato's statement, regarding your code: since the above pattern can lead to a "data race" Rust cannot grant you the usual zero-cost guarantee that it is sound, and will thus fail to compile unless you guard your code with a runtime check, such as RefCell's.

I don't have the time right now, but if you are interested I can write a version of your code that will compile thanks to RefCell.

The other (and better) solution is to feed the vector to the closure as an argument (thus only borrowing it for the duration of each call) instead of letting it be captured (since it is then borrowed for the whole duration of the closure's existence, hence the issue).

RustyYato · April 3, 2019, 1:08am

Heh, looking back on it I should have worded that differently.

guillegalor · April 3, 2019, 1:54pm

Thank you both, I read your article @RustyYato and it was quite explantory! Also I think I'm following @Yandros advice and I'll pass the vector and the rng as arguments. I think I understood what was the problem, thank you both so much!

Yandros · April 3, 2019, 4:44pm

Instead of:

fn test(rng: &mut StdRng) {
    const LEN: usize = 20;

    // Initialize vector of indexes and shuffles it
    let mut indexes: Vec<usize> = (0 .. LEN).collect();
    indexes.shuffle(rng);

    // Closure that fills and shuffle indexes
    let mut fill_and_shuffle = || {
        // no arg but uses &mut indexes and &mut *rng => CAPTURES THEM
        indexes = (0 .. LEN).collect();
        indexes.shuffle(rng);
    }; // => fill_and_shuffle captures &mut indexes, &mut* rng --+
    //                                                           |
    let _ = indexes.pop(); // requires &mut indexes => error     |
    //                                                           |
    fill_and_shuffle(); // <-------------------------------------+ borrow lasts at least until here

you should use

fn test(rng: &mut StdRng) {
    const LEN: usize = 20;

    // Initialize vector of indexes and shuffles it
    let mut indexes: Vec<usize> = (0..num_of_indexes).collect();
    indexes.shuffle(rng);

    // Closure that fills and shuffle indexes
    let fill_and_shuffle = |indexes, rng| {
        // only uses indexes and rng; since both are args, nothing is captured
        indexes = (0 .. LEN).collect();
        indexes.shuffle(rng);
    };

    // proof that it captures nothing: closure can be coerced to a function
    let _: fn(&mut Vec<usize>, &mut StdRng) = fill_and_shuffle;

    let _ = indexes.pop(); // requires &mut indexes => fine

    fill_and_shuffle(&mut indexes, rng); // borrows start and end here "i.e. in place"

Finally, since the closure does not capture anything and can thus be rewritten as a function, here is an example that uses an extension trait (to enable method-like syntax) and reuses the vector's allocation:

fn test (rng: &'_ mut StdRng)
{
    const LEN: usize = 20;

    trait FillAndShuffle { fn fill_and_shuffle (&mut self, _: &mut StdRng); }
    impl FillAndShuffle for Vec<usize> {
        fn fill_and_shuffle (&mut self, rng: &mut StdRng)
        {
            // fill, but reusing the allocation (if any)
            self.clear();
            self.extend(0 .. LEN);

            // shuffle
            self.shuffle(rng);
        }
    }

    // Initialize vector of indexes and shuffle it
    let mut indexes: Vec<usize> = Vec::new();
    indexes.fill_and_shuffle(rng);

    let _ = indexes.pop();

    indexes.fill_and_shuffle(rng);

Playground

(Aside: @guillegalor, if you are more confortable discussing all this in Spanish, you can PM me)

guillegalor · April 4, 2019, 12:55pm

Thank you so much again, I think i got it now!

system · July 3, 2019, 12:55pm

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.

Topic		Replies	Views
Borrow checker logic help	6	564	May 2, 2020
Understanding problem with borrowing help	4	385	January 21, 2022
Strange behaviour with closures help	3	600	April 23, 2022
Reusing a closure help	2	857	August 9, 2022
Lifetimes in closures with captured mutable reference help	10	569	September 1, 2020

(Solved) Problems using closures inside a loop

(Aside: @guillegalor, if you are more confortable discussing all this in Spanish, you can PM me)

Related topics