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: missing "consider using a let binding" message #49821

Closed
nikomatsakis opened this issue Apr 9, 2018 · 1 comment
Closed

NLL: missing "consider using a let binding" message #49821

nikomatsakis opened this issue Apr 9, 2018 · 1 comment
Assignees
Labels
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 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 9, 2018

(NB. This is assuming my in-progress branch)

If you run issue-36082.rs in NLL mode, you get:

> rustc --stage1 issue-36082.rs  -Zborrowck=mir
error[E0597]: borrowed value does not live long enough
  --> issue-36082.rs:21:19
   |
21 |     let val: &_ = x.borrow().0;
   |                   ^^^^^^^^^^  - temporary value only lives until here
   |                   |
   |                   temporary value does not live long enough
...
30 |     println!("{}", val);
   |                    --- borrow later used here

There is no suggestion about using a let binding to increase the lifetime of the temporary, unlike AST borrowck:

> rustc --stage1 issue-36082.rs
error[E0597]: borrowed value does not live long enough
  --> issue-36082.rs:21:19
   |
21 |     let val: &_ = x.borrow().0;
   |                   ^^^^^^^^^^  - temporary value dropped here while still borrowed
   |                   |
   |                   temporary value does not live long enough
...
31 | }
   | - temporary value needs to live until here
   |
   = note: consider using a `let` binding to increase its lifetime

That said, this seems like a case where the AST borrowck message could also be improved. I doubt many people understand it very well.

@nikomatsakis nikomatsakis added 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 NLL-diagnostics Working towards the "diagnostic parity" goal labels Apr 9, 2018
@jkordish jkordish added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label May 8, 2018
@matthewjasper matthewjasper self-assigned this Jul 12, 2018
@nikomatsakis
Copy link
Contributor Author

I'm lowering this to the Release milestone, as it seems like a relatively minor nit. Still good to see it fixed though!

bors added a commit that referenced this issue Sep 14, 2018
…felix

[NLL] Suggest let binding

Closes #49821

Also adds an alternative to `explain_why_borrow_contains_point` that allows changing error messages based on the reason that will be given. This will also be useful for #51026, #51169 and maybe further changes to does not live long enough messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 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

3 participants