Get rid of ErrorReportingCtx [5/N] #67474

mark-i-m · 2019-12-21T06:08:01Z

We can now use MirBorrowckCtxt instead :)

6 files changed, 122 insertions(+), 243 deletions(-)

This is a followup to (and thus blocked on) #67241.

r? @matthewjasper

cc @eddyb

I while try to do one more to get rid of the weird usage of RegionInferenceCtx in borrow_check::diagnostics::{region_errors, region_naming}. I think those uses can possibly also be refactored to use MirBorrowckCtxt...

mark-i-m · 2019-12-21T06:08:27Z

@rustbot modify labels: -S-waiting-on-review +S-blocked

bors · 2019-12-23T05:53:06Z

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

mark-i-m · 2019-12-28T16:37:19Z

@matthewjasper @eddyb This is ready

@rustbot modify labels: +S-waiting-on-review -S-blocked

src/librustc_mir/borrow_check/diagnostics/region_name.rs

src/librustc_mir/borrow_check/diagnostics/region_errors.rs

matthewjasper · 2019-12-29T22:21:00Z

@bors r+

bors · 2019-12-29T22:21:01Z

📌 Commit c8ebc319a4bf75a9184af86a6b61616242857788 has been approved by matthewjasper

bors · 2019-12-30T02:05:15Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout simplify-borrow_check-4 (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self simplify-borrow_check-4 --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Auto-merging src/librustc_mir/borrow_check/diagnostics/region_name.rs
CONFLICT (content): Merge conflict in src/librustc_mir/borrow_check/diagnostics/region_name.rs
Automatic merge failed; fix conflicts and then commit the result.

bors · 2019-12-30T02:06:15Z

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

mark-i-m · 2019-12-30T03:57:22Z

Rebased. Rerunning tests locally (it will take a while), but I'm not expecting changes...

mark-i-m · 2019-12-30T05:52:29Z

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

mark-i-m · 2019-12-30T06:10:01Z

Tests passing locally (stage 1, compare-mode=nll)

matthewjasper · 2019-12-30T13:55:26Z

@bors r+

bors · 2019-12-30T13:55:28Z

📌 Commit 96ce607 has been approved by matthewjasper

bors · 2019-12-30T14:47:32Z

⌛ Testing commit 96ce607 with merge 9d6f871...

@matthewjasper

Get rid of ErrorReportingCtx [5/N] We can now use `MirBorrowckCtxt` instead :) ``` 6 files changed, 122 insertions(+), 243 deletions(-) ``` This is a followup to (and thus blocked on) #67241. r? @matthewjasper cc @eddyb I while try to do one more to get rid of the weird usage of `RegionInferenceCtx` in `borrow_check::diagnostics::{region_errors, region_naming}`. I think those uses can possibly also be refactored to use `MirBorrowckCtxt`...

bors · 2019-12-30T18:06:47Z

☀️ Test successful - checks-azure
Approved by: matthewjasper
Pushing 9d6f871 to master...

@matthewjasper

Region naming refactoring [6/N] Followup to #67474 EDIT: this PR is probably best read commit-by-commit... The major changes in this PR include: - moving many functions around to modules that better suit them. In particular, a lot of methods were moved from `borrow_check::diagnostics::region_errors` to `borrow_check::region_infer`, and `report_region_errors` was moved from `borrow_check` to `borrow_check::diagnostics::region_errors`. - `borrow_check::diagnostics::{region_errors, region_name}` are now most comprised of methods on `MirBorrowckCtxt` instead of `RegionInferenceContext`, allowing us to get rid of the annoying `pub(in crate::borrow_check)` on most of the fields of the latter, along with a number of method arguments on many methods. - I renamed `MirBorrowckCtxt.nonlexical_regioncx` to just `regioncx` because their is no lexical lifetimes any more, and the old name was annoyingly verbose, causing many lines to wrap unnecessarily. - I got rid of `ErrorRegionNamingContext`. Region naming is implemented as inherent methods on `MirBorrowckCtxt`, so we just move the naming stuff into that struct. The PR is rather large, but the commits are fairly self-contained (though they don't all compile). There was one minor output change to one test with `compare-mode=nll`, which I think is acceptable. Between this PR and the last one, a net of 200 lines are removed, most of which was function parameters and context structs :tada: Some samples: ```diff - self.nonlexical_regioncx.free_region_constraint_info( - &self.body, - &self.local_names, - &self.upvars, - self.mir_def_id, - self.infcx, - borrow_region_vid, - region, - ); + self.free_region_constraint_info(borrow_region_vid, region); ``` ```diff - .or_else(|| { - self.give_name_if_anonymous_region_appears_in_yield_ty( - infcx, - body, - *mir_def_id, - fr, - renctx, - ) - }); + .or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr)) ``` r? @matthewjasper cc @eddyb

rust-highfive assigned matthewjasper Dec 21, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 21, 2019

rustbot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 21, 2019

mark-i-m mentioned this pull request Dec 21, 2019

Region naming refactoring [6/N] #67476

Merged

mark-i-m force-pushed the simplify-borrow_check-4 branch 2 times, most recently from 208e42f to 12bc5e9 Compare December 22, 2019 23:29

mark-i-m force-pushed the simplify-borrow_check-4 branch 2 times, most recently from 7d873e6 to 2dd035e Compare December 28, 2019 16:36

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Dec 28, 2019

mark-i-m changed the title ~~Get rid of ErrorReportingCtx~~ Get rid of ErrorReportingCtx [5/N] Dec 29, 2019

matthewjasper reviewed Dec 29, 2019

View reviewed changes

src/librustc_mir/borrow_check/diagnostics/region_name.rs Outdated Show resolved Hide resolved

src/librustc_mir/borrow_check/diagnostics/region_errors.rs Show resolved Hide resolved

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 29, 2019

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 30, 2019

mark-i-m added 2 commits December 29, 2019 21:55

Get rid of ErrorReportingCtx

eb218fc

fix review comment

96ce607

mark-i-m force-pushed the simplify-borrow_check-4 branch from c8ebc31 to 96ce607 Compare December 30, 2019 03:55

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 30, 2019

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 30, 2019

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 30, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 30, 2019

bors merged commit 96ce607 into rust-lang:master Dec 30, 2019

mark-i-m deleted the simplify-borrow_check-4 branch December 30, 2019 20:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get rid of ErrorReportingCtx [5/N] #67474

Get rid of ErrorReportingCtx [5/N] #67474

mark-i-m commented Dec 21, 2019 •

edited

Loading

mark-i-m commented Dec 21, 2019

bors commented Dec 23, 2019

mark-i-m commented Dec 28, 2019

matthewjasper commented Dec 29, 2019

bors commented Dec 29, 2019

bors commented Dec 30, 2019

bors commented Dec 30, 2019

mark-i-m commented Dec 30, 2019

mark-i-m commented Dec 30, 2019

mark-i-m commented Dec 30, 2019

matthewjasper commented Dec 30, 2019

bors commented Dec 30, 2019

bors commented Dec 30, 2019

bors commented Dec 30, 2019

Get rid of ErrorReportingCtx [5/N] #67474

Get rid of ErrorReportingCtx [5/N] #67474

Conversation

mark-i-m commented Dec 21, 2019 • edited Loading

mark-i-m commented Dec 21, 2019

bors commented Dec 23, 2019

mark-i-m commented Dec 28, 2019

matthewjasper commented Dec 29, 2019

bors commented Dec 29, 2019

bors commented Dec 30, 2019

bors commented Dec 30, 2019

mark-i-m commented Dec 30, 2019

mark-i-m commented Dec 30, 2019

mark-i-m commented Dec 30, 2019

matthewjasper commented Dec 30, 2019

bors commented Dec 30, 2019

bors commented Dec 30, 2019

bors commented Dec 30, 2019

mark-i-m commented Dec 21, 2019 •

edited

Loading