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.

Sign up for GitHub

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

Jump to bottom

Added better error message for shared borrow treated as unique for purposes of lifetimes #77692

Merged
merged 1 commit into from
Dec 24, 2020
Merged

Added better error message for shared borrow treated as unique for purposes of lifetimes #77692

merged 1 commit into from
Dec 24, 2020

Conversation

Polkaverse
Copy link
Contributor

Part of Issue #76630

r? @jyn514

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2020
@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 8, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 8, 2020

I only see changes to stderr, not to the compiler itself. You need to find where mutable borrow starts here is emitted in the compiler and change that (it will be somewhere in compiler/).

Anyway, I'm not the right reviewer for this.

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned jyn514 Oct 8, 2020
estebank
estebank reviewed Oct 14, 2020
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

You need to run ./x.py test src/test/ui --bless --stage 1 to update the stderr files. This will take a while the first time.

compiler/rustc_mir/src/util/borrowck_errors.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 14, 2020
@Polkaverse Polkaverse requested a review from estebank October 19, 2020 15:11
@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 20, 2020
@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 20, 2020
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 20, 2020
@Polkaverse Polkaverse requested review from jyn514 and camelid November 28, 2020 15:21
@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 28, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 28, 2020

Hmm, 'you have already borrowed' is not really the norm for error messages, normally they're third person. Other than that it looks good though :)

@Polkaverse
Copy link
Contributor Author

@jyn514 is this good?
mutable variable x is already borrowed in the previous iteration of the loop

@jyn514
Copy link
Member

jyn514 commented Nov 28, 2020

Yup, that looks great!

@Polkaverse Polkaverse requested a review from camelid November 29, 2020 17:10
@Polkaverse Polkaverse requested a review from camelid December 1, 2020 04:57
camelid
camelid approved these changes Dec 1, 2020
View reviewed changes
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@Polkaverse
Copy link
Contributor Author

@camelid can you merge it?

@camelid
Copy link
Member

camelid commented Dec 5, 2020

No, I think we'll have to wait for @estebank or someone else to merge it (estebank tends to be very busy).

@Polkaverse
Copy link
Contributor Author

@GuillaumeGomez can you please take a look at it

@camelid
Copy link
Member

camelid commented Dec 22, 2020

I don't think Guillaume is the right person to review this.

r? @davidtwco (feel free to re-assign)

@rust-highfive rust-highfive assigned davidtwco and unassigned estebank Dec 22, 2020
@GuillaumeGomez
Copy link
Member

It's indeed been a long time since I last worked on error messages so I'll let someone else r+ it (even though I approved it).

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 22, 2020

📌 Commit 50d9b30 has been approved by davidtwco

@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 Dec 22, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 23, 2020
@Dylan-DPC
Rollup merge of rust-lang#77692 - PankajChaudhary5:issue-76630, r=dav…
78b91af 
…idtwco

Added better error message for shared borrow treated as unique for purposes of lifetimes

Part of Issue rust-lang#76630

r? `@jyn514`
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Dec 23, 2020
Closed
@bors
Copy link
Contributor

bors commented Dec 24, 2020

⌛ Testing commit 50d9b30 with merge c34c015...

@bors
Copy link
Contributor

bors commented Dec 24, 2020

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing c34c015 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 24, 2020
@bors bors merged commit c34c015 into rust-lang:master Dec 24, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camelid camelid camelid approved these changes

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

@davidtwco davidtwco davidtwco approved these changes

@estebank estebank Awaiting requested review from estebank

@jyn514 jyn514 Awaiting requested review from jyn514

Assignees

@davidtwco davidtwco

Labels
A-diagnostics Area: Messages for errors, warnings, and lints merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.
9 participants
@Polkaverse @jyn514 @camelid @GuillaumeGomez @davidtwco @bors @estebank @rust-highfive @rustbot