Skip to content

[nll] borrows that must be valid for a free lifetime should explain why #52534

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

Closed
nikomatsakis opened this issue Jul 19, 2018 · 7 comments
Closed
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal

Comments

@nikomatsakis
Copy link
Contributor

Consider this example:

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

fn foo(_: impl FnOnce(&u32) -> &u32) {
}

fn bar() {
    let x = 22;
    foo(|a| &x)
}

fn main() { }

This fails to compile -- and rightly so. The closure is supposed to return something with the same lifetime as its argument, but it returns &x. However, the error we get is quite confusing:

error[E0597]: `x` does not live long enough
  --> src/main.rs:9:9
   |
9  |     foo(|a| &x)
   |         ^^^^^^ borrowed value does not live long enough
10 | }
   | - borrowed value only lives until here
   |
   = note: borrowed value must be valid for the static lifetime...

this error arises because we infer (again, correctly) that if the &x expression returned a &'static u32, it would be suitable -- and hence it infers that the borrow must last for 'static (but it cannot).

@nikomatsakis nikomatsakis added A-diagnostics Area: Messages for errors, warnings, and lints A-NLL Area: Non-lexical lifetimes (NLL) WG-compiler-nll NLL-diagnostics Working towards the "diagnostic parity" goal labels Jul 19, 2018
@nikomatsakis nikomatsakis changed the title [nll] promotion to static can be quite confusing [nll] borrows that must be valid for a free lifetime should explain why Aug 21, 2018
@nikomatsakis
Copy link
Contributor Author

I'm repurposing this issue slightly. I think the general problem here is this note:

   = note: borrowed value must be valid for the static lifetime...

in particular, it would be great if we could explain why this is the case. We have some of the machinery that we need now, which we use as part of region errors. To start though it'd be good to brainstorm just what this would look like.

cc @davidtwco @estebank

@nikomatsakis nikomatsakis added this to the Rust 2018 RC milestone Aug 21, 2018
@estebank
Copy link
Contributor

estebank commented Aug 21, 2018

Nightly emits

error[E0597]: `x` does not live long enough
  --> src/main.rs:9:14
   |
9  |     foo(|a| &x)
   |         ---  ^ borrowed value does not live long enough
   |         |
   |         value captured here
10 | }
   | - `x` dropped here while still borrowed
   |
   = note: borrowed value must be valid for the static lifetime...

It could be (the text would need more effort):

error[E0597]: `x` does not live long enough
  --> src/main.rs:9:14
   |
8  |     let x = 22;
   |        - value defined here
9  |     foo(|a| &x)
   |         ---  ^ borrowed value does not live long enough
   |         |
   |         value captured here
10 | }
   | - `x` dropped here while still borrowed
   |
   = note: borrowed value must be valid for the `'static` lifetime...
   = note: ...because the closure cannot find any lifetime in its arguments that would be applicable, so it requires `'static`

Maybe the following?

error[E0597]: `x` does not live long enough
  --> src/main.rs:9:14
   |
8  |     let x = 22;
   |        - value defined here which is valid until the end of the scope
9  |     foo(|a| &x)
   |         ---  ^ borrowed value does not live long enough
   |         |
   |         value captured here
10 | }
   | - `x` dropped here while still borrowed
   |
   = note: borrowed value must be valid for the `'static` lifetime...
note: ...because `x` is captured by the closure...
  |
9 |     foo(|a| &x)
  |             ^^
note: ...but the closure outlives the current scope when passed into `foo`
  |
9 |     foo(|a| &x)
  |     ^^^

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Sep 7, 2018

I think in my ideal world we would recognize, in this case, that the 'static lifetime is kind of a bad path, and give a quite different error:

error[E0597]: lifetime error
  --> src/main.rs:9:14
   |
9  |     foo(|a| &x)
   |          -  ^^ return value has to have the lifetime `'1`
   |          |
   |          let's call the type of this `&'1 u32`
   |

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Sep 7, 2018

To start, we can take the closure out of the equation and think instead of this example:

fn foo(x: &u32) -> &u32 {
    let x = 22;
    &x
}

This currently gives (playground):

error[E0597]: `y` does not live long enough
 --> src/main.rs:3:6
  |
3 |     &y
  |      ^ borrowed value does not live long enough
4 | }
  | - borrowed value only lives until here
  |
note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 1:1...
 --> src/main.rs:1:1
  |
1 | / fn foo(x: &u32) -> &u32 {
2 | |     let y = 22;
3 | |     &y
4 | | }
  | |_^

@nikomatsakis
Copy link
Contributor Author

I think I would like potentially a similar error:

error[E0597]: lifetime error
  --> src/main.rs:9:14
   |
8 | fn foo(x: &u32) -> &u32 {
  |            -
  |            | let's call this `'1`
9 |     &y
   |          ^ `y` would have to be valid for `'1`
10| }
  | - but `y` goes out of scope here

maybe something like that?

@nikomatsakis
Copy link
Contributor Author

This suggests our original could've been:

error[E0597]: lifetime error
  --> src/main.rs:9:14
   |
9  |     foo(|a| &x)
   |          -  ^^ `x` would have to be valid for the lifetime `'1`
   |          |
   |          let's call the type of this `&'1 u32`
   |  }
   |  - but `x` goes out of scope here

but somehow this seems kind of "meh". In both cases, there is maybe something even clearer, where we talk about "data local to the closure" or "data local to the function", but I can't quite see how to say it right now.

@nikomatsakis
Copy link
Contributor Author

Talking about where x goes out of scope feels a bit like a non-sequitor to me.

bors added a commit that referenced this issue Sep 23, 2018
[nll] borrows that must be valid for a free lifetime should explain why

Fixes #52534.

r? @nikomatsakis
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-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal
Projects
None yet
Development

No branches or pull requests

3 participants