Skip to content

[NLL] Improve closure region bound errors #54798

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

Merged
merged 2 commits into from
Oct 9, 2018

Conversation

matthewjasper
Copy link
Contributor

@matthewjasper matthewjasper commented Oct 3, 2018

Previously, we would report free region errors that originate from closure with the span of the closure and a "closure body requires ..." message. This is now updated to use a reason and span from inside the closure.

@rust-highfive
Copy link
Contributor

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 3, 2018
@matthewjasper
Copy link
Contributor Author

r? @nikomatsakis

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome!

(category, span)
}

fn retrieve_closure_constraint_info(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we assign the correct category to begin with?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it doesn't really save us very much, since the Constraint doesn't have anywhere to store the Span (I guess one could be added) and there is a PR that I'll be making soon that needs to know when bounds come from closure constraints.

@bors
Copy link
Collaborator

bors commented Oct 4, 2018

☔ The latest upstream changes (presumably #54649) made this pull request unmergeable. Please resolve the merge conflicts.

Allows us to use the category of outlive requirements inside a closure
when reporting free region errors caused by its closure bounds.
Now use the category and span that are associated to the most
interesting bound that led to the closure bound.
@matthewjasper matthewjasper force-pushed the free-region-closure-errors branch from a9a448e to 8258107 Compare October 6, 2018 09:11
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 8, 2018

📌 Commit 8258107 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 8, 2018
@bors
Copy link
Collaborator

bors commented Oct 9, 2018

⌛ Testing commit 8258107 with merge 607243b...

bors added a commit that referenced this pull request Oct 9, 2018
…komatsakis

[NLL]  Improve closure region bound errors

Previously, we would report free region errors that originate from closure with the span of the closure and a "closure body requires ..." message. This is now updated to use a reason and span from inside the closure.
@bors
Copy link
Collaborator

bors commented Oct 9, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 607243b to master...

@bors bors merged commit 8258107 into rust-lang:master Oct 9, 2018
@matthewjasper matthewjasper deleted the free-region-closure-errors branch October 9, 2018 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants