Borrow checker extends borrow range in code with early return

[CodeSandwich]

Borrow checker seems to extend mutable borrow of variable to the end of the function in case of an early return of value dependent on the borrow:

fn foo(x: &mut u8) -> Option<&u8> {
    if let Some(y) = bar(x) {
        return Some(y) // comment this out to calm compiler
    }
    bar(x)
}

fn bar(x: &mut u8) -> Option<&u8> { Some(x) }

Gives:

error[E0499]: cannot borrow `*x` as mutable more than once at a time
 --> src/main.rs:7:9
  |
4 |     if let Some(y) = bar(x) {
  |                          - first mutable borrow occurs here
...
7 |     bar(x)
  |         ^ second mutable borrow occurs here
8 | }
  | - first borrow ends here

Compilation fails on stable, nightly and nightly with NLL, works on nightly with polonius.
Try it on Playground

[Centril]

cc @nikomatsakis

[CodeSandwich]

Thank you for accepting! Should it be a low priority issue? The broken borrowing flow seems simple and is very hard to bypass when needed.

[i30817]

From my understanding polonius was created to fix this usecase among others since it's very common for terse code. You probably won't get it with NLL, otherwise it would already be there and a major motivation for polonius wouldn't exist.

On the other hand, it's very probable that polonius will replace NLL as the default 'NLL 2.0' eventually.

tl;dr: this issue is low priority because it's already being acted on and it's not going to be fixed without the lengthy process of making polonius ready to ship as default. My guess is it can't be lifted over as a single feature. Read the blog entry about it here.

[jonhoo]

I think #51826 suggests that this isn't solved by NLL?

[ghost]

tl;dr: this compiles return a.unwrap(); but this does not if 1 == 1 { return a.unwrap(); } EDIT: fixed by polonius (ie. compile with cargo rustc -- -Z polonius)

I've this code that seems to be hitting the same (current)issue. In this code I've illustrated 4 cases, 2 of which don't work. I've uncommented block code for case 3 below, but feel free to uncomment only the block for the relevant case you want to see act:
playground link

#![feature(nll)]
#![nll]
#![allow(dead_code)]

//#[derive(Debug)] //no need for this example
enum Opt<T> {
    //a non-Copy Option<T> XXX actually Option<T> is Copy only when T is Copy ; not so for this Opt<T> which is never Copy
    Some(T),
    None,
}

impl<T> Opt<T> {
    #[inline]
    pub fn as_mut(&mut self) -> Opt<&mut T> {
        match *self {
            Opt::Some(ref mut x) => Opt::Some(x),
            Opt::None => Opt::None,
        }
    }
    #[inline]

    pub fn unwrap(self) -> T {
        match self {
            Opt::Some(val) => val,
            Opt::None => panic!("called `Option::unwrap()` on a `None` value"),
        }
    }
}

struct WrappedI32(i32); //avoid Copy
struct Foo(Opt<WrappedI32>);

impl Foo { //example stolen&modified from jmcomets from irc
    fn get_or_set(&mut self, value: WrappedI32) -> &mut WrappedI32 {
        /*match self.0 {//this compiles, but not entirely sure it's equivalent to 'case 4' below
            Opt::Some(ref mut value) => return value,
            Opt::None => {
                self.0 = Opt::Some(value);
                self.0.as_mut().unwrap()
            }
        }*/
        {//this doesn't compile: that is, cases 3&4 don't work
            
            
            //case 1: compiles
            /*
            let a = self.0.as_mut();
            return a.unwrap();
            */
            
            //case 2: compiles
            /*
            let a = self.0.as_mut();
            */
            
            //case 3: fails to compile
            // /*
            let a = self.0.as_mut();
            if 1 == 2 {
                return a.unwrap();
            }
            // */
            
            //case 4: fails to compile
            /*
            if let Opt::Some(value) = self.0.as_mut() { //allowing this block compile fails
                return value;
            }
            */
        }
        self.0 = Opt::Some(value); //compile fails is here: error[E0506]: cannot assign to `self.0` because it is borrowed
        self.0.as_mut().unwrap()
    }
}

fn main() {
    // ...
}

error is like this:

Compiling playground v0.0.1 (/playground)
error[E0506]: cannot assign to `self.0` because it is borrowed
  --> src/main.rs:72:9
   |
35 |     fn get_or_set(&mut self, value: WrappedI32) -> &mut WrappedI32 {
   |                   - let's call the lifetime of this reference `'1`
...
59 |             let a = self.0.as_mut();
   |                     ------ borrow of `self.0` occurs here
60 |             if 1 == 2 {
61 |                 return a.unwrap();
   |                        ---------- returning this value requires that `self.0` is borrowed for `'1`
...
72 |         self.0 = Opt::Some(value); //compile fails is here: error[E0506]: cannot assign to `self.0` because it is borrowed
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to borrowed `self.0` occurs here

error: aborting due to previous error

EDIT: 1==2 can be replaced with 1==1 and it has the same effect!

Actually maybe it's:
#51526
#51826
I don't even know anymore

[euclio]

I ran into this today with https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=06cc9ab7151cd48d162b938e8898639f.

Polonius fixes it. Is there a workaround for just NLL?

[Havvy]

This looks like a duplicate of #21906. If not, it's a duplicate of #51545.

[ijackson]

This bug just bit me again. It seems this keeps coming up. #84361 shows a number of people being affected by this, and that issue seems to have evaded attention from anyone familiar with the previous reports eg this one. No doubt there are others.

Is there anything we could do to improve the UX here? Some heuristic hint in the error message maybe?

AFAICT from my reading, it is expected that polonius will fix this. At least, this was expected in 2019...

I struggled, and failed, to find out any information about polonius's state and likely future timescale. Searching for issues with NLL-polonius gave results but I found it difficult to see the wood for the trees. The NLL tracking issue #43234 does not mention polonius. Despite polonius having its own -Z option, I didn't find an unstable feature tracking issue for it and it isn't mentioned in the unstable book. Should there be a tracking issue?

[nikomatsakis]

There isn't much active work on polonius at the moment.

[asmello]

For the interested, this workaround worked for me:

if bar(x).is_some() {
    return bar(x).unwrap()
}

Caching bar(x) in a variable doesn't work, which is very unfortunate if that call is expensive (thankfully it's not in my case).