Explanation of Restricted Borrowing

The following code:

#[derive(Debug, thiserror::Error)]
enum Error<'p, 'v> {
    #[error("Remaining parameters are invalid: {remaining:?}")]
    InvalidRemaining { remaining: &'p str },

    #[error("Parameter name is too long: {parameter:?}")]
    NameTooLong { parameter: &'p str },
}

fn read_cmdline_while<const N: usize>(
    cmdline: &mut Peekable<Chars>,
    buffer: &mut heapless::String<N>,
    mut predicate: impl FnMut(char) -> bool,
) -> Result<usize, CapacityError> {
    buffer.clear();

    while let Some(c) = cmdline.next_if(|c| predicate(*c)) {
        buffer.push(c)?;
    }

    Ok(buffer.len())
}

fn read_parameter_name<'p, 'v>(
    cmdline: &mut Peekable<Chars>,
    name_buffer: &'p mut heapless::String::<128>,
) -> Result<(), Error<'p, 'v>> {
    read_cmdline_while(cmdline, name_buffer, |c| c.is_alphanumeric() || c == '_')
        .map_err(|_| Error::NameTooLong {
            parameter: name_buffer.as_str(),
        })
        .and_then(|char_count_read| {
            // Check if there were no valid parameter name chars.
            if char_count_read > 0 || cmdline.peek().is_none() {
                Ok(())
            } else {
                read_cmdline_while(cmdline, name_buffer, |_| true).ok();

                Err(Error::InvalidRemaining {
                    remaining: name_buffer.as_str(),
                })
            }
        })
}

results in the following error:

error[E0500]: closure requires unique access to `*name_buffer` but it is already borrowed
   --> src/params.rs:147:19
    |
139 |   fn read_parameter_name<'p, 'v>(
    |                          -- lifetime `'p` defined here
...
143 | /     read_cmdline_while(cmdline, name_buffer, |c| c.is_alphanumeric() || c == '_')
144 | |         .map_err(|_| Error::NameTooLong {
    | |                  --- borrow occurs here
145 | |             parameter: name_buffer.as_str(),
    | |                        ----------- first borrow occurs due to use of `*name_buffer` in closure
146 | |         })
147 | |         .and_then(|char_count_read| {
    | |                   ^^^^^^^^^^^^^^^^^ closure construction occurs here
...   |
152 | |                 read_cmdline_while(cmdline, name_buffer, |_| true).ok();
    | |                                             ----------- second borrow occurs due to use of `*name_buffer` in closure
...   |
158 | |         })
    | |__________- returning this value requires that `*name_buffer` is borrowed for `'p`

I do not understand how this borrowing failure is occurring. In no circumstance will name_buffer be borrowed in the .and_then closure, as it is only borrowed in the case of the call to Result::<(), heapless::CapacityError>::map_err—in which case, the .and_then closure is never called.

If anyone could elucidate the failure in my understanding—and potentially explain a solution to the above issue—I would be very grateful.

This is a wonderful assessment from the people who created the borrow checker. Thanks.

Well… to understand what is happening you first need to take an assumption that compiler “thinks about what would happen at runtime” and then decides whether to accept your program or not — and flush it down the toilet. Forget it.

It's simply mathematically impossible to create such a compiler. In principle. It just could never work.

So… forget all that “advanced thinkings” about “this may only happen that doesn't happen”. That simply not how compiler work. Once again: it's not that we are lazy and haven't done that. We simply couldn't do that.

But… what could we do? We could look on the semantic out of the window we are left with syntax.

And syntactically nothing in definition map_err and and_then tells the compiler that only one of them may ever call the closure. Worse: both closures have to exist till the moment where all remporary objects are dropped (that happens at ; here) — and that's blatant violation of Rules.

And yes, once again: these Rules that borrow checker uses are just an imprecise approximation of what happens at runtime… that's fine, that's the only way it may ever work.

P.S. Thinking about what would happen at runtime is still a good idea, of course, that's how programs that does something useful are created. Just don't forget that compiler may never do that step directly: what this program does at runtime is Ok → ergo this program should be accepted. There are always an intermediate step.

2 Likes

The solution to this problem, therefore, is to write syntax that does guarantee that only one is run: use match instead of .map_err().and_then(). In particular, whenever you have any enum, including Result, one of the first things you should consider is match:

    match read_cmdline_while(cmdline, name_buffer, |c| c.is_alphanumeric() || c == '_') {
        Err(_) => Err(Error::NameTooLong {
            parameter: name_buffer.as_str(),
        }),
        Ok(char_count_read) => {
            ...
        }
    )
5 Likes

*name_buffer is reborrowed twice in the and_then closure.

                // reborrow 1               vvvvvvvvvvv
                read_cmdline_while(cmdline, name_buffer, |_| true).ok();

                Err(Error::InvalidRemaining {
                    // reborrow 2 vvvvvvvvvvvvvvvvv
                    remaining: name_buffer.as_str(),
                })

Think of a closure as a struct with fields for it's captures.

struct MapErrClosure<'n> {
    name_buffer: &'n name_buffer,
}
struct AndThenClosure<'c, 'n> {
    cmdline: &'c mut Peekable<Chars>,
    name_buffer: &'n mut name_buffer,
}

The closure gets created at the declaration site -- not every time the closure is called, but before that. You also have to create closures where the 'n lifetime is 'p long due to the function signature. That's why the exclusive borrow in the and_then closure ends up conflicting.

The solution is to not create the borrow until you need to return -- by using matching and early returns instead of closures.

2 Likes

I appreciate the extremely informative responses. I often lean towards using method chaining as I think it looks better, but I see here that it creates a situation in which the compiler cannot discern the actual borrowing semantics taking place.

So, I will switch to using match statement in this case.

Thank you again for the prompt assistance!

Other advantages of match:

  • Spells out what it is doing; does not require the reader to know all the method names from Result and Option and keep track of what the type is along the way in order to fully understand the code.
  • Compatible with control flow operations like await, ?, break, and continue.
  • Less likely to need to be replaced with something completely different if the requirements change (e.g. if you decide to return something other than a Result for some reason).

Straightforward and boring code is good code.

4 Likes

:+1:

This is why, personally, I just never use https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or_else and similar. Every time there's a two-disjoint-closures API, it works a bit worse than just writing the match as soon as you do anything interesting.