Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jonas-schievink opened this issue Jun 9, 2017 · 18 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions A-NLL Area: Non-lexical lifetimes (NLL) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-diagnostics Working towards the "diagnostic parity" goal NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jonas-schievink
Copy link
Contributor

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
Copy link
Contributor Author

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

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

@Mark-Simulacrum Mark-Simulacrum added the A-lifetimes Area: Lifetimes / regions label Jun 23, 2017
@Mark-Simulacrum
Copy link
Member

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

@nikomatsakis
Copy link
Contributor

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 (#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
Copy link
Contributor

nikomatsakis commented Jul 17, 2017

Closing in favor of #42516.

Well, actually, I'll keep this open.

@Mark-Simulacrum Mark-Simulacrum added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jul 27, 2017
@pnkfelix pnkfelix added the A-NLL Area: Non-lexical lifetimes (NLL) label Nov 9, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Nov 9, 2018

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 pnkfelix added the NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. label Nov 9, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Nov 9, 2018

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
Copy link
Member

pnkfelix commented Nov 9, 2018

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 pnkfelix added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Nov 9, 2018
@pnkfelix
Copy link
Member

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

@pnkfelix
Copy link
Member

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
Copy link
Member

pnkfelix commented Dec 6, 2018

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
Copy link
Member

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 pnkfelix self-assigned this Dec 19, 2018
@pnkfelix pnkfelix added P-low Low priority NLL-diagnostics Working towards the "diagnostic parity" goal and removed NLL-deferred NLL-diagnostics Working towards the "diagnostic parity" goal labels Dec 19, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Jul 9, 2019

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...?

Centril added a commit to Centril/rust that referenced this issue Jul 9, 2019
…crichton

Regression test for issue 42574.

Cc rust-lang#42574.

I'm not going to say this *closes* that issue yet, for two reasons:

 1. I am still confused about some aspects of the behavior we are observing that bug

 2. The "fix" to the diagnostic relies on full NLL (`#![feature(nll)]`); migration mode still has a subpar diagnostic.
Centril added a commit to Centril/rust that referenced this issue Jul 9, 2019
…crichton

Regression test for issue 42574.

Cc rust-lang#42574.

I'm not going to say this *closes* that issue yet, for two reasons:

 1. I am still confused about some aspects of the behavior we are observing that bug

 2. The "fix" to the diagnostic relies on full NLL (`#![feature(nll)]`); migration mode still has a subpar diagnostic.
@estebank
Copy link
Contributor

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 #49616, which is closed but still reproduces.

@pnkfelix
Copy link
Member

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

@crlf0710 crlf0710 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 11, 2020
@rylev
Copy link
Member

rylev commented Jun 17, 2021

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?

@rylev rylev removed the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Jun 17, 2021
@Aaron1011
Copy link
Member

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

@estebank
Copy link
Contributor

estebank commented Oct 4, 2021

@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
Copy link
Contributor

estebank commented Nov 7, 2023

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

@estebank estebank closed this as completed Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions A-NLL Area: Non-lexical lifetimes (NLL) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-diagnostics Working towards the "diagnostic parity" goal NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants