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

Add feature to enable bind by move pattern guards #54034

Merged
merged 3 commits into from
Sep 18, 2018
Merged

Add feature to enable bind by move pattern guards #54034

merged 3 commits into from
Sep 18, 2018

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Sep 7, 2018 •
edited
Loading

Implement #15287 as described on #15287 (comment)

@rust-highfive
Copy link
Collaborator

r? @varkor

(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 7, 2018
pnkfelix
pnkfelix commented Sep 7, 2018
View reviewed changes
src/test/ui/rfc-0107-bind-by-move-pattern-guards/feature-gate.no_gate.stderr
@@ -0,0 +1,9 @@
error[E0008]: cannot bind by-move into a pattern guard
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I guess it would be nice (when on nightly and with MIR-borrowck enabled) if this issued a diagnostic suggesting the feature gate.

Copy link
Member Author

Choose a reason for hiding this comment

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

(done, see commit c329897)

Copy link
Member

Choose a reason for hiding this comment

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

Could also be useful to suggest enabling NLL and the feature if not? Perhaps that's a little convoluted though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's necessary; the 2018 edition has NLL enabled anyway (under migrate mode), so I think that will be enough.

@bors
Copy link
Contributor

bors commented Sep 10, 2018

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

@varkor
Copy link
Member

varkor commented Sep 11, 2018

The implementation looks good to me, but I'd feel more comfortable if someone more familiar with borrowck double-checked.

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned varkor Sep 11, 2018
@pnkfelix pnkfelix force-pushed the issue-15287-bind-by-move-pattern-guards branch from c329897 to 1bdc926 Compare September 12, 2018 08:27
@rust-highfive

This comment has been minimized.

Sign in to view
nikomatsakis
nikomatsakis approved these changes Sep 12, 2018
View reviewed changes
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.

r=me

@nikomatsakis

This comment has been minimized.

Sign in to view
@nikomatsakis nikomatsakis 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 Sep 12, 2018
@bors

This comment has been minimized.

Sign in to view
@pnkfelix pnkfelix force-pushed the issue-15287-bind-by-move-pattern-guards branch from 1bdc926 to e801def Compare September 15, 2018 06:51
@rust-highfive

This comment has been minimized.

Sign in to view
@pnkfelix pnkfelix force-pushed the issue-15287-bind-by-move-pattern-guards branch from e801def to 40d8f54 Compare September 17, 2018 11:46
@pnkfelix
Add feature(bind_by_move_pattern_guards).
7d844e8 
Note it requires MIR-borrowck to be enabled to actually do anything.

Note also that it implicitly turns off our AST-based check for
mutation in guards.
@pnkfelix
Tests for feature(bind_by_move_pattern_guards).
c50884c 
Apparently copyright notices are no longer necessary apparently.
(See rust-lang#43498 and rust-lang#53654.)
@pnkfelix
On nightly with NLL, suggest `#![feature(bind_by_move_pattern_guards)…
3a07d3d 
…]` when it might fix the code.
@pnkfelix pnkfelix force-pushed the issue-15287-bind-by-move-pattern-guards branch from 40d8f54 to 3a07d3d Compare September 17, 2018 11:47
@pnkfelix
Copy link
Member Author

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Sep 17, 2018

📌 Commit 3a07d3d has been approved by nikomatsakis

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 17, 2018
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 17, 2018
@bors
Copy link
Contributor

bors commented Sep 18, 2018

⌛ Testing commit 3a07d3d with merge 79fcc58...

bors added a commit that referenced this pull request Sep 18, 2018
@bors
Auto merge of #54034 - pnkfelix:issue-15287-bind-by-move-pattern-guar…
79fcc58 
…ds, r=nikomatsakis

Add feature to enable bind by move pattern guards

Implement #15287 as described on #15287 (comment)
@bors
Copy link
Contributor

bors commented Sep 18, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 79fcc58 to master...

@bors bors merged commit 3a07d3d into rust-lang:master Sep 18, 2018
This was referenced Sep 18, 2018
Merged
Merged
@pnkfelix pnkfelix mentioned this pull request Sep 19, 2018
Closed
3 tasks
@pnkfelix pnkfelix mentioned this pull request Dec 19, 2018
Closed
@Centril Centril mentioned this pull request Aug 20, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varkor varkor varkor left review comments

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
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.
5 participants
@pnkfelix @rust-highfive @bors @varkor @nikomatsakis