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

[NLL] "borrowed value must be valid for lifetime '_#2r..." #48428

Closed
nikomatsakis opened this issue Feb 22, 2018 · 6 comments
Closed

[NLL] "borrowed value must be valid for lifetime '_#2r..." #48428

nikomatsakis opened this issue Feb 22, 2018 · 6 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-NLL Area: Non-lexical lifetimes (NLL) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

In NLL, there is one rule of diagnostics:

Thou must-eth never print-eth a region.

Unfortunately, we break it. Twice. Here is one example:

#![feature(nll)]

fn gimme(x: &(u32,)) -> &u32 { &x.0 }

fn main() {
    let v = 22;
    let x = gimme(&(v,));
    println!("{:?}", x);
}

This prints the following:

error[E0597]: borrowed value does not live long enough
 --> src/main.rs:7:20
  |
7 |     let x = gimme(&(v,));
  |                    ^^^^ - temporary value only lives until here
  |                    |
  |                    temporary value does not live long enough
  |
  = note: borrowed value must be valid for lifetime '_#2r...

Note the final line. In comparison, the AST-based version gets the following error, which is much better:

error[E0597]: borrowed value does not live long enough
 --> src/main.rs:7:20
  |
7 |     let x = gimme(&(v,));
  |                    ^^^^ - temporary value dropped here while still borrowed
  |                    |
  |                    temporary value does not live long enough
8 |     println!("{:?}", x);
9 | }
  | - temporary value needs to live until here
  |
  = note: consider using a `let` binding to increase its lifetime

We probably want to integrate with the "dump-cause" mode and something like this:

error[E0597]: borrowed value does not live long enough
 --> src/main.rs:7:20
  |
7 |     let x = gimme(&(v,));
  |                    ^^^^ - temporary value only lives until here
  |                    |
  |                    temporary value does not live long enough
 8 |    println!("{}", x);
   |                   ^ reference later used here
@nikomatsakis
Copy link
Contributor Author

The problem steps from this call to note_and_explain_region:

self.tcx.note_and_explain_region(scope_tree, &mut err,
"borrowed value must be valid for ",
borrow.region, "...");

There is another call here:

self.tcx.note_and_explain_region(scope_tree, &mut err,
"borrowed value must be valid for ",
borrow.region, "...");

which I guess would be triggered by this test case:

#![feature(nll)]

fn gimme(x: &(u32,)) -> &u32 { &x.0 }

fn main() {
    let x = gimme({ let v = (22,); &v });
    println!("{:?}", x);
}

@nikomatsakis nikomatsakis added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-NLL Area: Non-lexical lifetimes (NLL) WG-compiler-nll labels Feb 22, 2018
@nikomatsakis nikomatsakis added this to the NLL: diagnostic parity milestone Feb 22, 2018
@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Feb 22, 2018

In NLL mode, there is already a helper for printing out these messages in NLL mode, it's called explain_why_borrow_containts_point. In fact, when we call note_and_explain_region, we are also calling this method:

self.tcx.note_and_explain_region(scope_tree, &mut err,
"borrowed value must be valid for ",
borrow.region, "...");
self.explain_why_borrow_contains_point(context, borrow, &mut err);

I would say that we almost just want to remove note_and_explain_region altogether -- at least in NLL mode. However, we may also want to expand explain_why_borrow_contains_point. For example, we want to be sure that this example prints something helpful (here, the data must be live for 'a):

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

fn gimme(x: &(u32,)) -> &u32 { &x.0 }

fn foo<'a>(x: &'a (u32,)) -> &'a u32 {
    let v = 22;
    gimme(&(v,))
}

fn main() {
}

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Feb 22, 2018

cc @rust-lang/wg-compiler-nll -- in general, I think we need someone to "take charge" of the "dump-cause" code, which is the code that tries to label the "third use" in NLL, indicating why a borrow lasts for a certain time. This code is currently only enabled when you pass -Znll-dump-cause to the compiler, because the inference is kind of expensive. So there are a few parts to this: this issue mostly about expanding and improving the output, but #46590 is about making it fast.

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Feb 22, 2018

I'd be happy to work closely with someone on this! I don't think it's particularly hard, will just take a bit of focus.

@nikomatsakis
Copy link
Contributor Author

@spastorino has indicated some interest here...

@spastorino spastorino self-assigned this Feb 22, 2018
@nikomatsakis
Copy link
Contributor Author

So I think there are a lot of moving parts. Here is an attempt at making a list:

bors added a commit that referenced this issue Mar 4, 2018
[NLL] Avoid borrowed value must be valid for lifetime '_#2r..." in errors

Closes #48428

- [x] If NLL is enabled, [do not invoke `note_and_explain_region`](#48428 (comment))
- [x] Modify `-Zdump-nll-cause` to not print [the overwhelming debug output here](https://github.com/rust-lang/rust/blob/master/src/librustc_mir/borrow_check/nll/region_infer/mod.rs#L1288-L1299). This way we should I believe at least get nice-ish output for [our original example](#48428 (comment)).
- [x] Extend `explain_why_borrow_contains_point` to also work for "universal lifetimes" like the `'a` in [the example at the end of this comment](#48428 (comment)).
- [ ] Figure out how to enable causal information all the time (but that is #46590).
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) 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

2 participants