Improve borrow error when assigning borrowed local value to "argument by mutable reference"

[estebank]

Given

fn f(mut p: &mut i32) {

    let mut number = 111;
    p = &mut number;

    *p = 2;
    println!("{}", *p);
}

we currently emit

error[E0597]: `number` does not live long enough
 --> f44.rs:4:9
  |
1 | fn f(mut p: &mut i32) {
  |             - let's call the lifetime of this reference `'1`
...
4 |     p = &mut number;
  |     ----^^^^^^^^^^^
  |     |   |
  |     |   borrowed value does not live long enough
  |     assignment requires that `number` is borrowed for `'1`
...
8 | }
  | - `number` dropped here while still borrowed

but it could be more informative:

error[E0597]: `number` does not live long enough
 --> f44.rs:4:9
  |
1 | fn f(mut p: &mut i32) {
  |             - let's call the lifetime of this reference `'1`
...
3 |     let mut number = 111;
  |             -----   --- this value is assigned to `number` and lives as long as `f`
  |             |
  |             this binding only lives for the duration of function `f` and gets dropped at the function
4 |     p = &mut number;
  |     ----^^^^^^^^^^^
  |     |   |
  |     |   borrowed value does not live long enough
  |     this assignment to `p` will outlive `f` because it is a `&mut i32` owned by the caller's scope
...
8 | }
  | - `number` dropped here while still borrowed

[BoxyUwU]

I dislike the suggested error message- p is not owned by the callers scope. The lifetime on the p reference will outlive the borrow of number because it is chosen by the caller who can only name lifetimes that outlive borrows of locals inside of f. If this error is to be improved (imo) it should suggest adding a let mut p = p at the start of the function to allow shortening the caller chosen lifetime to that of the borrow of number. (I suppose to be clear- i think the missing information here is that the lifetime on p is chosen by the caller who can only name lifetimes that outlive borrows of the locals, this explains both why the let mut p = p would work and why the original code doesnt work)

[ryanavella]

This is an odd one. I used to think that fn f(mut t: T) { ... } was just sugar for fn f(t: T) {let mut t = t; ... }, but clearly that isn't the case. The error can be suppressed by rewriting it in my hypothetical desugared form, like this:

fn f(p: &mut i32) {
    let mut p = p; // <-- suppresses error!

    let mut number = 111;
    p = &mut number;

    *p = 2;
    println!("{}", *p);
}

I'm not familiar with rustc internals so I don't know what information is known by this pass, but is it possible to distinguish this specific case from the more general case and suggest introducing a let-binding?

i.e. something like this:

error[E0597]: `number` does not live long enough

....

help: use `let mut` if you meant to introduce a local binding
  |
4 |     let mut p = &mut number;
  |     ~~~~~~~ 

... or this

error[E0597]: `number` does not live long enough

....

help: use `let mut` if you meant to introduce a local binding
  |
2 |     let mut p = p;
  |     ~~~~~~~~~~~~~~
3 |     let mut number = 111;

[jam1garner]

this assignment to `p` will outlive `f` because it is a `&mut i32` owned by the caller's scope

I heavily dislike this phrasing, and would go as far as to say it's incorrect.

Now to help demonstrate, here's an example that I think this *would* be applicable for:

fn f(p: &mut &mut i32) {

    let mut number = 111;
    *p = &mut number;

    **p = 2;
    println!("{}", **p);
}

(Note the change from mut p: &mut i32 to p: &mut &mut i32, as it's the only change aside from updating deref counts to match)

Going back to the phrasing:

a &mut i32 owned by the caller's scope

I would argue this implies the reference (not the underlying data) is owned by the caller. Which, isn't really the case, as it's actually pass-by-move-ing the reference, so the caller doesn't actually own the reference itself anymore.

Brief intermission: the distinction I'm making is both pedantic and dependent on (a) my mental model and (b) my interpretation of words like "own". So really, whether or not I'm correct comes down to semantics and wholly respect disagreement, but everything will be phrased as if it is fact for the sake of not bogging this down with uncertain phrasing.

So to indicate what I feel is actually happening, I think it's important to desugar what's going on, especially as I think Rust could very well allow the code you posted (more on this in a minute). This means that the explanation being in terms of "this is wrong, it doesn't live long enough" is potentially suboptimal, as the reason the code is rejected is more arbitrary than that.

So desugared:

fn f<'a>(mut p: &'a mut i32) {

    let mut number = 111;
    p = &mut number;

    *p = 2;
    println!("{}", *p);
}

Now the issue here is not "the reference outlives number", the issue is that the caller can choose arbitrary lifetimes that &mut number cannot necessarily conform to. From a functionality standpoint, the implementation just sees 'a as "must live as long as this function, maybe longer".

So maybe the explanation should include the desugaring? I am not sure how to explain it otherwise.

[jam1garner]

I think @ryanavella's suggestion is not a bad one (honestly not sure I like that's it's necessary though. I'm curious if lifetimes should be coercible to the minimum when not observable by the caller?), although I would add some context explaining why this fixes it:

help: use `let mut` if you meant to introduce a local binding to shorten the lifetime to this function's scope

(phrasing subject to overhaul, my only nit was "this suggestion is too magical and unintuitive without the 'why'")

[cuviper]

Note that you can compile code like the OP wrote when there's a deeper pattern involved -- see #86989.

fn f((_i, mut p): (usize, &mut i32)) {

    let mut number = 111;
    p = &mut number;

    *p = 2;
    println!("{}", *p);
}

But still, if the intention was to change the caller's reference to the local, that will never work.

[ryanavella]

@cuviper That is definitely interesting, even a 1-tuple is enough of a pattern to suppress the error:

fn f((mut p,): (&mut i32,)) {

    let mut number = 111;
    p = &mut number;

    *p = 2;
    println!("{}", *p);
}

It isn't clear to me which should be the "correct behavior" here. I can see an argument that the error is spurious because there is nothing inherently unsound here. Plus the error message is confusing to newcomers.

But is it possible that some users could be relying on this error to enforce a type-level invariant? If so, I wouldn't know how to construct an example. All of the examples I can think of can be easily bypassed by shadow-binding, as @BoxyUwU first suggested.

[jam1garner]

wow! great link @cuviper, very interesting behavior. hopefully the non-nested bindings are loosened to act like the nested ones?

[mdaffin]

Plus the error message is confusing to newcomers.

It is very confusing to new comers. But so is the whole situation. It seems quite a lot of people seem to assume the problem is that p itself somehow outlives the function and so must anything that gets assigned to it otherwise it is unsound when this is not the case. Nothing is technically unsound here but IMO it smells badly of a bug in the code. Why would you ever want to reassign p when it points to a reference?

That is the only thing you can do by declaring it mut. It makes me think that the caller either mean to rebind with let or they actually wanted p: &mut &mut u32 to change where the original reference points to.

[QuineDot]

That is definitely interesting, even a 1-tuple is enough of a pattern to suppress the error

@ryanavella Subpatterns are enough.

fn f(mut p @ _: &mut i32) {
    let mut number = 111;
    p = &mut number;
    *p = 2;
    println!("{}", *p);
}