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] Suggest let binding #54088

Merged
merged 2 commits into from
Sep 14, 2018

Conversation

matthewjasper
Copy link
Contributor

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.

@rust-highfive
Copy link
Collaborator

r? @oli-obk

(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 Sep 9, 2018
@matthewjasper
Copy link
Contributor Author

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned oli-obk Sep 9, 2018
@memoryruins memoryruins added the A-NLL Area: Non-lexical lifetimes (NLL) label Sep 9, 2018
@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 11, 2018

📌 Commit 54f7311 has been approved by pnkfelix

@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 Sep 11, 2018
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.

Not a blocker, but...

@@ -8,6 +8,8 @@ LL | v3.push(&id('x')); // statement 6
...
LL | (v1, v2, v3, /* v4 is above. */ v5).use_ref();
| -- borrow later used here
|
= note: consider using a `let` binding to create a longer lived value
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the temporary rules are a common source of confusion. I think we should consider using a distinct error code for them, so that --explain can have some extra text to introduce the rules. Maybe good for a follow-up to this PR though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed #54131

@bors
Copy link
Contributor

bors commented Sep 14, 2018

⌛ Testing commit 54f7311 with merge 052d24e...

bors added a commit that referenced this pull request 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.
@bors
Copy link
Contributor

bors commented Sep 14, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing 052d24e to master...

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) 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.

7 participants