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

Make #![feature(bind_by_move_pattern_guards)] sound without #[feature(nll)] #63059

Merged
merged 9 commits into from
Aug 3, 2019
Merged

Make #![feature(bind_by_move_pattern_guards)] sound without #[feature(nll)] #63059

merged 9 commits into from
Aug 3, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jul 28, 2019 •
edited
Loading

Implements #15287 (comment) making #![feature(bind_by_move_pattern_guards)]] sound without also having #![feature(nll)]. The logic here is that if we see a match guard, we will refuse to downgrade NLL errors to warnings. This is in preparation for hopefully stabilizing the former feature in #63118.

As fall out from the implementation we also:
Fixes #31287
Fixes #27282

r? @matthewjasper

@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 28, 2019
matthewjasper
matthewjasper reviewed Jul 28, 2019
View reviewed changes
Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

This mostly looks good, but I wasn't expecting all matches with guards to result in hard borrowck errors. I'm not sure if @rust-lang/lang has an opinion here.

src/librustc_mir/hair/pattern/check_match.rs Show resolved Hide resolved
@Centril
Copy link
Contributor Author

Centril commented Jul 28, 2019

[...] but I wasn't expecting all matches with guards to result in hard borrowck errors. [...]

Fwiw, I tried some hacks to avoid it but that went nowhere... =)

@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2019

📌 Commit cd79609 has been approved by matthewjasper

@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 Jul 29, 2019
@Centril Centril mentioned this pull request Jul 29, 2019
Merged
Centril added a commit to Centril/rust that referenced this pull request Jul 29, 2019
@Centril
Rollup merge of rust-lang#63059 - Centril:sound-bind-by-move, r=matth…
81510c9 
…ewjasper

Make `#![feature(bind_by_move_pattern_guards)]` sound without `#[feature(nll)]`

Implements rust-lang#15287 (comment).

Fixes rust-lang#31287
Fixes rust-lang#27282

r? @matthewjasper
@Centril Centril mentioned this pull request Jul 29, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 30, 2019
@Centril
Rollup merge of rust-lang#63059 - Centril:sound-bind-by-move, r=matth…
11a913c 
…ewjasper

Make `#![feature(bind_by_move_pattern_guards)]` sound without `#[feature(nll)]`

Implements rust-lang#15287 (comment).

Fixes rust-lang#31287
Fixes rust-lang#27282

r? @matthewjasper
@Centril Centril mentioned this pull request Jul 30, 2019
Closed
@Centril

This comment has been minimized.

Sign in to view
@bors 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 Jul 30, 2019
@Centril

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 3, 2019
@Centril
Copy link
Contributor Author

Centril commented Aug 3, 2019

Same error again, but this time tested standalone and not in a rollup:

   Compiling proc-macro2 v0.4.26
   Compiling autocfg v0.1.4
error: linker `lld-link.exe` not found
  |
  = note: The system cannot find the file specified. (os error 2)

note: the msvc targets depend on the msvc linker but `link.exe` was not found

note: please ensure that VS 2013, VS 2015, VS 2017 or VS 2019 was installed with the Visual C++ option

error: aborting due to previous error

error: Could not compile `proc-macro2`.
warning: build failed, waiting for other jobs to finish...
error: build failed
thread 'main' panicked at 'tests failed for https://github.com/servo/servo', src\tools\cargotest\main.rs:86:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

cc @alexcrichton @pietroalbini

@pietroalbini
Copy link
Member

pietroalbini commented Aug 3, 2019 •
edited
Loading

The Servo failure is probably caused by servo/servo#23256. The last Servo commit that didn't use lld is servo/servo@caac107, so bumping to that should work. cc @jdm

@Centril
Copy link
Contributor Author

Centril commented Aug 3, 2019

Updated the servo ref to caac107ae8145ef2fd20365e2b8fadaf09c2eb3b per ^---.

@bors r=matthewjasper

@bors
Copy link
Contributor

bors commented Aug 3, 2019

📌 Commit b289f6f has been approved by matthewjasper

@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 Aug 3, 2019
@bors

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 3, 2019
@pietroalbini

This comment has been minimized.

Sign in to view
@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 Aug 3, 2019
@pietroalbini

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Aug 3, 2019

⌛ Testing commit b289f6f with merge 6e0d27d...

bors added a commit that referenced this pull request Aug 3, 2019
@bors
Auto merge of #63059 - Centril:sound-bind-by-move, r=matthewjasper
6e0d27d 
Make `#![feature(bind_by_move_pattern_guards)]` sound without `#[feature(nll)]`

Implements #15287 (comment) making `#![feature(bind_by_move_pattern_guards)]]` sound without also having `#![feature(nll)]`. The logic here is that if we see a `match` guard, we will refuse to downgrade NLL errors to warnings. This is in preparation for hopefully stabilizing the former feature in #63118.

As fall out from the implementation we also:
Fixes #31287
Fixes #27282

r? @matthewjasper
@bors
Copy link
Contributor

bors commented Aug 3, 2019

☀️ Test successful - checks-azure
Approved by: matthewjasper
Pushing 6e0d27d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 3, 2019
@bors bors merged commit b289f6f into rust-lang:master Aug 3, 2019
@Centril Centril deleted the sound-bind-by-move branch August 3, 2019 23:58
@Centril Centril mentioned this pull request Aug 4, 2019
Closed
3 tasks
@chrissimpkins chrissimpkins mentioned this pull request Feb 10, 2020
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewjasper matthewjasper matthewjasper left review comments

Assignees

@matthewjasper matthewjasper

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.

Pattern guard can consume value that is being matched Can mutate in match-arm using a closure
7 participants
@Centril @rust-highfive @matthewjasper @bors @Manishearth @SimonSapin @pietroalbini