Skip to content

NLL: missing suggestion to use move keyword on closures #51169

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
pnkfelix opened this issue May 29, 2018 · 1 comment
Closed

NLL: missing suggestion to use move keyword on closures #51169

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

Comments

@pnkfelix
Copy link
Member

pnkfelix commented May 29, 2018

In a number of tests, NLL regresses the diagnostics in that they stop including the suggestion: “to force the closure to take ownership, use the move keyword.”

This is relevant to the following ui/ stderr test files:

Update: various diagnostics have improved to the point where I can now identify them as belonging to this issue, when before they were too impoverished for me to tell. I'm adding to the list above as I discover such cases.

@pnkfelix pnkfelix added A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal WG-compiler-nll labels May 29, 2018
@pnkfelix pnkfelix changed the title NLL: missing suggestion that to use move keyword on closures NLL: missing suggestion to use move keyword on closures Jun 4, 2018
@nikomatsakis nikomatsakis added this to the Rust 2018 Preview 2 milestone Jun 29, 2018
@nikomatsakis
Copy link
Contributor

I'll mark this as Edition Preview 2 because it's a .. fairly useful hint? Hard to judge these things.

@matthewjasper matthewjasper self-assigned this Jul 15, 2018
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
# for free to join this conversation on GitHub. Already have an account? # 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