(Solved) Problems using closures inside a loop

#1

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!

#2

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

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.

3 Likes
#3

Just to “soothe” @KrishnaSannasi’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).

2 Likes
#4

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

1 Like
#5

Thank you both, I read your article @KrishnaSannasi 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!

1 Like
#6

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);

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

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