Skip to content

Add feature to enable bind by move pattern guards - #54034

Merged
bors merged 3 commits into
rust-lang:masterfrom
pnkfelix:issue-15287-bind-by-move-pattern-guards
Sep 18, 2018
Merged

Add feature to enable bind by move pattern guards#54034
bors merged 3 commits into
rust-lang:masterfrom
pnkfelix:issue-15287-bind-by-move-pattern-guards

Conversation

@pnkfelix

@pnkfelixpnkfelix commented Sep 7, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor

Implement #15287 as described on #15287 (comment)

@rust-highfive

rust-highfive commented Sep 7, 2018

Copy link
Copy Markdown
Contributor

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-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
Comment threadsrc/test/ui/rfc-0107-bind-by-move-pattern-guards/feature-gate.no_gate.stderr Outdated

@pnkfelixpnkfelixSep 7, 2018

Copy link
Copy Markdown
ContributorAuthor

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.

@pnkfelixpnkfelixSep 10, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

(done, see commit c329897)

@varkorvarkorSep 11, 2018

Copy link
Copy Markdown
Contributor

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.

@pnkfelixpnkfelixSep 11, 2018

Copy link
Copy Markdown
ContributorAuthor

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

bors commented Sep 10, 2018

Copy link
Copy Markdown
Collaborator

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

varkor
varkor reviewed Sep 11, 2018
View reviewed changes
Comment threadsrc/test/ui/rfc-0107-bind-by-move-pattern-guards/bind-by-move-no-guards.rs Outdated
varkor
varkor reviewed Sep 11, 2018
View reviewed changes
Comment threadsrc/test/ui/rfc-0107-bind-by-move-pattern-guards/feature-gate.rs Outdated
varkor
varkor reviewed Sep 11, 2018
View reviewed changes
Comment threadsrc/test/ui/rfc-0107-bind-by-move-pattern-guards/rfc-basic-examples.rs Outdated
varkor
varkor reviewed Sep 11, 2018
View reviewed changes
Comment threadsrc/test/ui/rfc-0107-bind-by-move-pattern-guards/rfc-reject-double-move-across-arms.rs Outdated
varkor
varkor reviewed Sep 11, 2018
View reviewed changes
Comment threadsrc/test/ui/rfc-0107-bind-by-move-pattern-guards/rfc-reject-double-move-in-first-arm.rs Outdated
@varkor

varkor commented Sep 11, 2018

Copy link
Copy Markdown
Contributor

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

r? @nikomatsakis

@pnkfelix
pnkfelixforce-pushed the issue-15287-bind-by-move-pattern-guards branch from c329897 to 1bdc926CompareSeptember 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

@nikomatsakisnikomatsakis left a comment

Copy link
Copy Markdown
Contributor

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
@nikomatsakisnikomatsakis 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
pnkfelixforce-pushed the issue-15287-bind-by-move-pattern-guards branch from 1bdc926 to e801defCompareSeptember 15, 2018 06:51
@rust-highfive

This comment has been minimized.

Sign in to view
@pnkfelix
pnkfelixforce-pushed the issue-15287-bind-by-move-pattern-guards branch from e801def to 40d8f54CompareSeptember 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
pnkfelixforce-pushed the issue-15287-bind-by-move-pattern-guards branch from 40d8f54 to 3a07d3dCompareSeptember 17, 2018 11:47
@pnkfelix

pnkfelix commented Sep 17, 2018

Copy link
Copy Markdown
ContributorAuthor

@bors r=nikomatsakis

@bors

bors commented Sep 17, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 3a07d3d has been approved by nikomatsakis

@borsbors 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
@borsbors 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

bors commented Sep 18, 2018

Copy link
Copy Markdown
Collaborator

⌛ 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

bors commented Sep 18, 2018

Copy link
Copy Markdown
Collaborator

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

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

Reviewers

@nikomatsakisnikomatsakisnikomatsakis approved these changes
+1 more reviewer
@varkorvarkorvarkor left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@nikomatsakisnikomatsakis

Labels

S-waiting-on-borsStatus: 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