Skip to content

Don't stop evaluating due to errors before borrow checking#60125

Merged
bors merged 3 commits intorust-lang:masterfrom
estebank:continue-evaluating
Apr 23, 2019
Merged

Don't stop evaluating due to errors before borrow checking#60125
bors merged 3 commits intorust-lang:masterfrom
estebank:continue-evaluating

Conversation

@estebank
Copy link
Contributor

@estebank estebank commented Apr 19, 2019

r? @oli-obk

Fix #60005. Follow up to #59903. Blocked on #53708, fixing the ICE in src/test/ui/consts/match_ice.rs.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 19, 2019
@estebank estebank changed the title Never stop due to errors before borrow checking Don't stop evaluating due to errors before borrow checking Apr 19, 2019
@rust-highfive

This comment has been minimized.

Sign in to view
@estebank estebank force-pushed the continue-evaluating branch from c3725fe to 595a151 Compare April 20, 2019 01:27
oli-obk
oli-obk reviewed Apr 20, 2019
View reviewed changes
src/test/ui/error-codes/E0007.stderr Outdated
Copy link
Contributor

@oli-obk oli-obk Apr 20, 2019

Choose a reason for hiding this comment

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

this span isn't quite right. I think it should point to op_string

src/test/ui/rfc-2005-default-binding-mode/for.stderr Outdated
Copy link
Contributor

@oli-obk oli-obk Apr 20, 2019

Choose a reason for hiding this comment

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

this diagnostic is actually better than the first one.

@bors

This comment has been minimized.

Sign in to view
@estebank estebank force-pushed the continue-evaluating branch from 595a151 to 87ef96d Compare April 22, 2019 20:14
@estebank
Copy link
Contributor Author

estebank commented Apr 22, 2019

@oli-obk It seems to me that with the change made in this PR, we can safely disable everything in check_match and no currently compiling code will stop compiling, and a subset of currently rejected code would now be accepted (as tested locally), which seems like it could be problematic because people could write code that compiles in latest stable but wouldn't compile in older rustc.

@estebank estebank mentioned this pull request Apr 22, 2019
Closed
oli-obk
oli-obk reviewed Apr 23, 2019
View reviewed changes
src/test/ui/borrowck/borrowck-mutate-in-guard.rs
Enum::A(_) if { x = Enum::B(false); false } => 1,
//~^ ERROR cannot assign in a pattern guard
//~| WARN cannot assign `x` in match guard
//~| WARN this error has been downgraded to a warning for backwards compatibility
Copy link
Contributor

@oli-obk oli-obk Apr 23, 2019

Choose a reason for hiding this comment

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

as a next step we should make this a deny-by-default lint.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 23, 2019

a subset of currently rejected code would now be accepted (as tested locally)
which seems like it could be problematic because people could write code that compiles in latest stable but wouldn't compile in older rustc.

That's fine. We do this all the time. I mean, every new syntactical feature is basically that.

we can safely disable everything in check_match

There's also code for the exhaustiveness checks in match checking, we can't remove that.

Let's merge this PR and then have a look at what effect killing the match borrow check duplication has (in a new PR)

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 23, 2019

📌 Commit 87ef96d has been approved by oli-obk

@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 Apr 23, 2019
@bors
Copy link
Collaborator

bors commented Apr 23, 2019

⌛ Testing commit 87ef96d with merge 31f5d69...

bors added a commit that referenced this pull request Apr 23, 2019
@bors
Auto merge of #60125 - estebank:continue-evaluating, r=oli-obk
31f5d69 
Don't stop evaluating due to errors before borrow checking

r? @oli-obk

Fix #60005. Follow up to #59903. Blocked on #53708, fixing the ICE in `src/test/ui/consts/match_ice.rs`.
@bors
Copy link
Collaborator

bors commented Apr 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing 31f5d69 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 23, 2019
@bors bors merged commit 87ef96d into rust-lang:master Apr 23, 2019
@tesuji tesuji mentioned this pull request Oct 14, 2020
Closed
@estebank estebank deleted the continue-evaluating branch November 9, 2023 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

Labels

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove abort_if_errors before borrow checking

4 participants

@estebank @rust-highfive @bors @oli-obk