"lifetime of borrowed pointer outlives lifetime of captured variable" error message very unclear

[jonas-schievink]

For this code:

fn doit(data: &'static mut ()) {
    || doit(data);
}

(works fine if you drop the mut, still happens with a move closure).

The compiler (stable and nightly) spits out this error:

error[E0313]: lifetime of borrowed pointer outlives lifetime of captured variable `data`...
 --> <anon>:3:13
  |
3 |     || doit(data);
  |             ^^^^
  |
  = note: ...the borrowed pointer is valid for the static lifetime...
note: ...but `data` is only valid for the lifetime  as defined on the body at 3:7
 --> <anon>:3:8
  |
3 |     || doit(data);
  |        ^^^^^^^^^^

error: aborting due to previous error

• The last note is broken: for the lifetime as defined on the actual lifetime is invisible. There is also no lifetime defined in the entire program, so it's not clear which lifetime the note is talking about.
• The main error message is very unclear. Is it saying that data itself must live as long as the data behind the reference? Why?

According to the rustc source code, this error is related to an implicit reborrow done by the closure. The error mentions nothing of that sort.

[jonas-schievink]

Workaround (why would this work, but not the case above?):

fn doit(data: &'static mut ()) {
    || {
        let d = data;
        doit(d);
    };
}

[Mark-Simulacrum]

cc @nikomatsakis -- you've been doing some work on collecting these issues recently

[nikomatsakis]

Hmm. This error seems to arise due to a couple of interacting things. In particular, the closure is a temporary value, and as the final expression in the function its lifetime winds up being too big in some cases (https://github.com/rust-lang/rust/issues/21114), and that is (in part) what is causing this error -- or at least I think. In any case, I'll add it to the tracking issue and try to take a look at it later on! Thanks @Mark-Simulacrum for calling it out.

[nikomatsakis]

Closing in favor of #42516.

Well, actually, I'll keep this open.

[pnkfelix]

Under the NLL migration mode, we get the existing (bad) error messaging.

But under #![feature(nll)] proper, we get this (play):

error: unsatisfied lifetime constraints
 --> src/main.rs:5:8
  |
5 |     || doit(data);
  |     -- ^^^^^^^^^^ argument requires that `'1` must outlive `'static`
  |     |
  |     lifetime `'1` represents this closure's body
  |
  = note: closure implements `FnMut`, so references to captured variables can't escape the closure

error[E0597]: `data` does not live long enough
 --> src/main.rs:5:13
  |
5 |     || doit(data);
  |     -- -----^^^^-
  |     |  |    |
  |     |  |    borrowed value does not live long enough
  |     |  argument requires that `data` is borrowed for `'static`
  |     value captured here
6 | }
  |  - `data` dropped here while still borrowed

[pnkfelix]

My personal take is that the error message under #![feature(nll)] is sufficiently clear.

I have tagged this as NLL-fixed-by-NLL, but it is worth pointing out that this is not fixed by the NLL migration mode. Do we need a separate tag for issues that require migrating from migration to #![feature(nll)] itself?

[pnkfelix]

Nominating for discussion at NLL meeting, just in terms of determining how this should be tagged to ensure that we keep it open until #![feature(nll)] is out of migration and totally stabilized.

Also, it needs tests that encode the current semantics under all three revisions: borrowck=ast, borrowck=migrate, and borrowck=mir.

[pnkfelix]

triage: hmm clearly the WG-nll meetings need to do a better job of looking at the nominated issues. My bad.

[pnkfelix]

well, since I know that we are going to make a work queue from the things tagged as NLL-deferred, I'm going to add that tag to this issue. (But I'll also leave the I-nominated on it, since the question and tasks I posed above remain unresolved.)

[pnkfelix]

we discussed this at the NLL meeting. the rough consensus was that the NLL-fixed-by-NLL tag applies here, even though you need the full #![feature(nll)] and not just the migration mode to observe the fix. Removing the nomination label.

[pnkfelix]

Re-triaging for #56754. NLL-fixed-by-NLL, so leaving open. Assigning to self for the main remaining task of adding the tests. But also tagging as P-low since I don't think that is a high priority task.

[pnkfelix]

Workaround (why would this work, but not the case above?):

fn doit(data: &'static mut ()) {
    || {
        let d = data;
        doit(d);
    };
}

reading this comment more carefully, now I too am wondering why we accept the above but not the original example.

Stranger still, (depending on your POV), we still reject variants like this:

#![feature(nll)]

fn doit(data: &'static mut ()) {
    || {
        let d: &mut _ = data;
        doit(d)
    };
}

What type are we inferring for d that lets it pass up above, but not below...?

[estebank]

A case that might change its behavior once this is fixed:

#![feature(nll)]

use std::cell::RefCell;

fn main() {
    let s = RefCell::new(Some(10));
    if let Some(n) = *(s.borrow_mut()) {
        println!("num: {}", n);
    }
}

where it compile-passes if the if let block is followed by a semicolon. Taken from https://github.com/rust-lang/rust/issues/49616, which is closed but still reproduces.

[pnkfelix]

@estebank I don't think #49616 is same as whats occuring here; I posted argument as to why over here on #21114.

[rylev]

Triage: this produces an error message that seems fairly clear to me.

Going to remove E-needs-test since #62520 added a test for this.

@pnkfelix you mentioned in #62520 that you're "confused about some aspects of the behavior we are observing that bug" - is that captured here? Now that nll is fully a thing, can this be closed?

[Aaron1011]

What's the status of this issue? Can it be closed?

[estebank]

@Aaron1011 I think this can be closed once we enable NLL by default, although I'm sure there are some tweaks we could make to the output to make it clearer:

error: lifetime may not live long enough
 --> src/lib.rs:3:8
  |
3 |     || doit(data);
  |     -- ^^^^^^^^^^ argument requires that `'1` must outlive `'static`
  |     |
  |     lifetime `'1` represents this closure's body
  |
  = note: closure implements `FnMut`, so references to captured variables can't escape the closure

error[E0597]: `data` does not live long enough
 --> src/lib.rs:3:13
  |
3 |     || doit(data);
  |     -- -----^^^^-
  |     |  |    |
  |     |  |    borrowed value does not live long enough
  |     |  argument requires that `data` is borrowed for `'static`
  |     value captured here
4 | }
  |  - `data` dropped here while still borrowed

[estebank]

Closing this as per the prior comments, we've been using the NLL borrow checker for a while now.