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 diagnostics has lost "closure may outlive the current function" notes #51026

Closed
6 of 7 tasks
pnkfelix opened this issue May 24, 2018 · 3 comments
Closed
6 of 7 tasks
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 torwads 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

pnkfelix commented May 24, 2018

The following ui/ tests have regressed in the quality of their diagnostic output (compared to AST borrowck, at least) due to the loss of notes of the form "closure may outlive the current function"

  • borrowck/borrowck-escaping-closure-error-1.rs
  • borrowck/borrowck-escaping-closure-error-2.rs
  • issue-4335.stderr
  • region-borrow-params-issue-29793-small.rs
  • regions-nested-fns-2.rs "closure may outlive the current function" isn't correct here
  • src/test/ui/issue-40510-3.nll.stderr
  • src/test/ui/issue-49824.nll.stderr

(This list of tests is drawn from an informal paper document that I have been using to keep notes for myself as I work on this...)

@pnkfelix pnkfelix added A-NLL Area: Non Lexical Lifetimes (NLL) WG-compiler-nll NLL-diagnostics Working torwads the "diagnostic parity" goal labels May 24, 2018
@nikomatsakis
Copy link
Contributor

I'm tagging this as RC though it seems related to the other region error issues described in #51027

@nikomatsakis
Copy link
Contributor

Some comments on this starting about here on Zulip -- around here I highlighted how we might solve this

@matthewjasper
Copy link
Contributor

Added the two extra cases that that discussion was about

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.
@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
bors added a commit that referenced this issue Oct 21, 2018
…nikomatsakis

[NLL] Use new region infer errors when explaining borrows

Use the new free region infer errors for explaining borrows

This gives at least some explanation for why a borrow is expected to
last for a certain free region. Also:

* Reports E0373: "closure may outlive the current function" with NLL.
* Special cases the case of returning a reference to (or value referencing) a local variable or temporary (E0515).
* Special case assigning a reference to a local variable in a closure to a captured variable. (E0521)

Closes #51026 - `regions-nested-fns-2.rs` isn't changed to that diagnostic, since that would not be the correct error here.
Closes #51169
cc #53882 - The error is (IMO) better now, but it could be better when we trace lifetimes in these error messages.

r? @nikomatsakis cc @pnkfelix
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 torwads 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