Mutable Borrow Issues within Iterators and Closures

help
1

I've been playing around with Day 5 of advent of code and got everything working.
But I ran into an error when I was doing some debugging.

error[E0502]: cannot borrow `unum` as immutable because it is also borrowed as mutable
  --> day5/src/main.rs:25:17
   |
20 |         .map(|line| {
   |              ------ mutable borrow occurs here
21 |             // im lazy so remember to delete any new lines that come after the update arrays
22 |             unum += 1;
   |             ---- first borrow occurs due to use of `unum` in closure
...
25 |         .filter(|update: &Vec<usize>| {
   |          ------ ^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here
   |          |
   |          mutable borrow later used by call
...
36 |                 .expect(format!("Failed on vec: {:?} @ Line: {unum}", update).as_str());
   |                                                               ---- second borrow occurs due to use of `unum` in closure

I was trying to keep track of the # of update arrays that have been processed and which line caused a panic when unwrapping the first item of the array. This is what unum is used for.

Here we see I have a mutable ref to unum where I am incrementing it for each line.
Then I try to print it, when my expect statement is triggered, on creation of temp.

I know rust borrow rules don't like to have a mut ref alive while an immut ref is.
Yet I don't see how this specific scenario can be problematic, given that it is executed on 1 thread.

The idea here being that unum is incremented for each line, but that ref goes out of scope, once Vec::from_iter(...) finishes. And after that an immutable ref of unum is alive only within the scope of the filter(|update| ...) which finishes before running the next iteration of map.
Shouldn't the &mut unumdie before the &unum is made, and the &unum then dies before the next iteration of &mut unum?

I'm aware that the issue is the &mut unum of unum is being captured within the closure of filter which then uses the &unum to print but I don't know if there is anyway to customize this behavior. I am curious what the rusty way to solve this is. Lmk what you guys think.

    let mut unum = 0 as usize;
    let updates: Vec<Vec<usize>> = i
        .map(|line| {
            // im lazy so remember to delete any new lines that come after the update arrays
            unum += 1;
            Vec::from_iter(line.split(",").filter_map(|item| item.parse().ok()))
        }) // get all the updates from file
        .filter(|update: &Vec<usize>| {
            // filter remove bad arrays
            //for each element of update
            //check if curr elm is a key
            //if last elm is inside the mapping for current key then we have bad news
            // stop checking array once we find a rule break
            // return otherwise
            let mut v = update.iter();
            let mut is_valid: bool = true;
            let mut temp = v
                .next()
                .expect(format!("Failed on vec: {:?} @ Line: {unum}", update).as_str());
            v.for_each(|num| {
                if matches!(
                    page_rules
                        .get(num)
                        .and_then(|rules| Some(rules.contains(temp))),
                    Some(true)
                ) {
                    is_valid = false;
                    return (); // match page rules violated
                }
                temp = num
            });
            is_valid
        })
        .collect();
2

Are you looking for enumerate?

roughly the diff needed here [but untested code!] 
-   let mut unum = 0 as usize;
    let updates: Vec<Vec<usize>> = i
        .map(|line| {
-           // im lazy so remember to delete any new lines that come after the update arrays
-           unum += 1;
            Vec::from_iter(line.split(",").filter_map(|item| item.parse().ok()))
        }) // get all the updates from file
-       .filter(|update: &Vec<usize>| {
+       .enumerate() 
+       .filter(|(&unum, update)| {
            // filter remove bad arrays
            //for each element of update
            //check if curr elm is a key
            //if last elm is inside the mapping for current key then we have bad news
            // stop checking array once we find a rule break
            // return otherwise
            let mut v = update.iter();
            let mut is_valid: bool = true;
            let mut temp = v
                .next()
                .expect(format!("Failed on vec: {:?} @ Line: {unum}", update).as_str());
            v.for_each(|num| {
                if matches!(
                    page_rules
                        .get(num)
                        .and_then(|rules| Some(rules.contains(temp))),
                    Some(true)
                ) {
                    is_valid = false;
                    return (); // match page rules violated
                }
                temp = num
            });
            is_valid
        })
+       .map(|(_, update)| update)
        .collect();
2 Likes
3

Well, UB is defined at the language level, and Rust says an aliased &mut _ is UB. But your question about why the &mut _ sticks around is a sensible question:

You can think of how closures work like so: The compiler creates a struct that has its captures as fields, and then implements some Fn traits[1] so that it can be called like a function. The construction of the struct happens wherever the closure is written. So the same &mut _ capture sticks around for as long as the closure is in use.

Closures do not release their captures at the end of one call and recreate them at the point of the next call.

An aside about single threadeness

"Single threaded" is usually a dead end argument when it comes to relaxing borrow checking. For one, the language either assumes multithreadedness or doesn't know what a thread is or isn't, depending on your POV. And this is correct in the general sense, as the compiler has no way to know if a system call or linked library, etc, spawns threads. For another, a single thread still allows multiple borrows at difference positions on the stack, interrupt handlers that may access global state, etc.

  1. and other traits ↩︎

1 Like
4

Thanks for the deeper explanation on closures. If I am understanding right, I can't extract the closures to functions either because they would still capture their environment at least once to get the unum value.

That makes things more clear!

Also thats a good point about threads relation with Borrow Checker, lots of code can be used in ways outside of its original intended use and can introduce threads, so it makes sense to have a sane default.

5

This is 100% the solution I was going for, can't believe I missed it thanks.

I was planning to go off on the deep end and try to use Cell or some runtime craziness to get around Borrow Checker.

6

I wouldn't consider it off the deep end to use Cell if enumerate didn't make sense for some reason. That is, Cell can be a perfectly reasonable approach (especially for small Copy types like usize). Although it's not the most ergonomic.

-    let unum = 0 as usize;
+    let unum = Cell::new(0);
     let updates: Vec<Vec<usize>> = i
         .map(|line| {
-            unum += 1;
+            unum.set(unum.get() + 1);
             Vec::from_iter(line.split(",").filter_map(|item| item.parse().ok()))
          })
[...]
-.expect(format!("Failed on vec: {:?} @ Line: {unum}", update).as_str());
+.expect(format!("Failed on vec: {:?} @ Line: {}", update, unum.get()).as_str());
1 Like
7

Awesome, I guess great minds think alike. :slight_smile:
The reason why I was hesitant to pull out cell is I am unaware of what performance cost it has, I'll probably get around to benchmarking to learn when its cost is too great.

8

The cost of using Cell is only missing some optimizations that don't apply to this situation.