Skip to content

NLL: missing note that "borrowed value needs to live until here" (sometimes?) #51168

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
pnkfelix opened this issue May 29, 2018 · 5 comments
Closed
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-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

Consider: https://github.com/rust-lang/rust/blob/master/src/test/ui/span/issue-11925.nll.stderr

Compare it to: https://github.com/rust-lang/rust/blob/master/src/test/ui/span/issue-11925.stderr

The former is missing a note that "borrowed value needs to live until here"


Oftentimes, NLL diagnostics do include a note of the form "needs to live until use here." But apparently for this case (due to closures? Or destructors? Or both?) we do not have such a use to point at.

Anyway, this represents a regression in the informativeness of our diagnostics under NLL.

@pnkfelix pnkfelix added NLL-diagnostics Working towards the "diagnostic parity" goal WG-compiler-nll A-NLL Area: Non-lexical lifetimes (NLL) labels May 29, 2018
@nikomatsakis
Copy link
Contributor

The reason for this is indeed the closure, I think. We detect the error when analyzing the closure body, so it is hard for us to report a span that is outside the closure. I think this will be pretty tricky to fix and is maybe not that important. What we could probably do is take a different tactic: we could explain that x is owned by the closure and yet we are returning a reference to it (just as we would a top-level function, perhaps).

I'm imagining:

error[E0597]: `x` is owned by the closure, so you cannot return a reference to it
  --> $DIR/issue-11925.rs:18:35
   |
LL |         let f = to_fn_once(move|| &x); //~ ERROR does not live long enough
   |                                   ^
   |                                   |
   |                                   |
| cannot return reference to data owned by the closure

for bonus points, we can highlight the move keyword and/or the use of x that caused it to a move in the first place (we have that data available somewhere).

@nikomatsakis
Copy link
Contributor

I think that fixing this would also fix the examples here:

#51027

@nikomatsakis
Copy link
Contributor

Actually I'm marking this as a sub-issue of #51027

@XAMPPRocky XAMPPRocky added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Oct 2, 2018
@matthewjasper
Copy link
Contributor

nominating for discussion on whether this should be kept open

@nikomatsakis
Copy link
Contributor

Decided to close. The current output is arguably clearer, and it would be quite challenging to add the "needs to live until here" note, since we don't have that information at the point where we report the error anymore. It's not clear that it adds a lot of information anyway.

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-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants