Initial implementation of `#![feature(move_ref_pattern)]`#68376

Centril · 2020-01-19T23:24:42Z

Following up on #45600, under the gate #![feature(move_ref_pattern)], (ref x, mut y) is allowed subject to restrictions necessary for soundness. The match checking implementation and tests for #![feature(bindings_after_at)] is also adjusted as necessary.

Closes #45600.
Tracking issue: #68354.

r? @matthewjasper

src/librustc_error_codes/error_codes/E0009.md

src/librustc_mir_build/hair/pattern/check_match.rs

estebank · 2020-01-25T04:34:13Z

CC #30418

Centril · 2020-01-26T15:50:46Z

CC #30418

This unrelated error is comes from rustc_resolve and is specific to or-patterns:

src/test/ui/pattern/move-ref-patterns/move-ref-patterns-closure-captures-pass.rs

src/librustc_mir_build/hair/pattern/check_match.rs

bindings_after_at: harden tests

Centril · 2020-02-02T20:20:28Z

Addressed the comments. :)

Centril · 2020-02-02T20:24:39Z

Also, do you think the test suite is sufficient for eventual stabilization purposes or do you think more would be necessary?

matthewjasper · 2020-02-08T21:55:20Z

I can't think of any other tests that I would want.
@bors r+

bors · 2020-02-08T21:55:21Z

📌 Commit d2b88b7 has been approved by matthewjasper

bors · 2020-02-09T04:01:34Z

⌛ Testing commit d2b88b7 with merge f8d830b...

@matthewjasper

Initial implementation of `#![feature(move_ref_pattern)]` Following up on #45600, under the gate `#![feature(move_ref_pattern)]`, `(ref x, mut y)` is allowed subject to restrictions necessary for soundness. The match checking implementation and tests for `#![feature(bindings_after_at)]` is also adjusted as necessary. Closes #45600. Tracking issue: #68354. r? @matthewjasper

bors · 2020-02-09T07:11:49Z

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

@Centril

…n, r=nikomatsakis Stabilize move_ref_pattern # Implementation - Initially the rule was added in the run-up to 1.0. The AST-based borrow checker was having difficulty correctly enforcing match expressions that combined ref and move bindings, and so it was decided to simplify forbid the combination out right. - The move to MIR-based borrow checking made it possible to enforce the rules in a finer-grained level, but we kept the rule in place in an effort to be conservative in our changes. - In rust-lang#68376, @Centril lifted the restriction but required a feature-gate. - This PR removes the feature-gate. Tracking issue: rust-lang#68354. # Description This PR is to stabilize the feature `move_ref_pattern`, which allows patterns containing both `by-ref` and `by-move` bindings at the same time. For example: `Foo(ref x, y)`, where `x` is `by-ref`, and `y` is `by-move`. The rules of moving a variable also apply here when moving *part* of a variable, such as it can't be referenced or moved before. If this pattern is used, it would result in *partial move*, which means that part of the variable is moved. The variable that was partially moved from cannot be used as a whole in this case, only the parts that are still not moved can be used. ## Documentation - The reference (rust-lang/reference#881) - Rust by example (rust-lang/rust-by-example#1377) ## Tests There are many tests, but I think one of the comperhensive ones: - [borrowck-move-ref-pattern-pass.rs](https://github.com/Centril/rust/blob/85fbf49ce0e2274d0acf798f6e703747674feec3/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern-pass.rs) - [borrowck-move-ref-pattern.rs](https://github.com/Centril/rust/blob/85fbf49ce0e2274d0acf798f6e703747674feec3/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.rs) # Examples ```rust #[derive(PartialEq, Eq)] struct Finished {} #[derive(PartialEq, Eq)] struct Processing { status: ProcessStatus, } #[derive(PartialEq, Eq)] enum ProcessStatus { One, Two, Three, } #[derive(PartialEq, Eq)] enum Status { Finished(Finished), Processing(Processing), } fn check_result(_url: &str) -> Status { // fetch status from some server Status::Processing(Processing { status: ProcessStatus::One, }) } fn wait_for_result(url: &str) -> Finished { let mut previous_status = None; loop { match check_result(url) { Status::Finished(f) => return f, Status::Processing(p) => { match (&mut previous_status, p.status) { (None, status) => previous_status = Some(status), // first status (Some(previous), status) if *previous == status => {} // no change, ignore (Some(previous), status) => { // Now it can be used // new status *previous = status; } } } } } } ``` Before, we would have used: ```rust match (&previous_status, p.status) { (Some(previous), status) if *previous == status => {} // no change, ignore (_, status) => { // new status previous_status = Some(status); } } ``` Demonstrating *partial move* ```rust fn main() { #[derive(Debug)] struct Person { name: String, age: u8, } let person = Person { name: String::from("Alice"), age: 20, }; // `name` is moved out of person, but `age` is referenced let Person { name, ref age } = person; println!("The person's age is {}", age); println!("The person's name is {}", name); // Error! borrow of partially moved value: `person` partial move occurs //println!("The person struct is {:?}", person); // `person` cannot be used but `person.age` can be used as it is not moved println!("The person's age from person struct is {}", person.age); } ```

@Centril

…n, r=nikomatsakis Stabilize move_ref_pattern # Implementation - Initially the rule was added in the run-up to 1.0. The AST-based borrow checker was having difficulty correctly enforcing match expressions that combined ref and move bindings, and so it was decided to simplify forbid the combination out right. - The move to MIR-based borrow checking made it possible to enforce the rules in a finer-grained level, but we kept the rule in place in an effort to be conservative in our changes. - In rust-lang#68376, @Centril lifted the restriction but required a feature-gate. - This PR removes the feature-gate. Tracking issue: rust-lang#68354. # Description This PR is to stabilize the feature `move_ref_pattern`, which allows patterns containing both `by-ref` and `by-move` bindings at the same time. For example: `Foo(ref x, y)`, where `x` is `by-ref`, and `y` is `by-move`. The rules of moving a variable also apply here when moving *part* of a variable, such as it can't be referenced or moved before. If this pattern is used, it would result in *partial move*, which means that part of the variable is moved. The variable that was partially moved from cannot be used as a whole in this case, only the parts that are still not moved can be used. ## Documentation - The reference (rust-lang/reference#881) - Rust by example (rust-lang/rust-by-example#1377) ## Tests There are many tests, but I think one of the comperhensive ones: - [borrowck-move-ref-pattern-pass.rs](https://github.com/Centril/rust/blob/85fbf49ce0e2274d0acf798f6e703747674feec3/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern-pass.rs) - [borrowck-move-ref-pattern.rs](https://github.com/Centril/rust/blob/85fbf49ce0e2274d0acf798f6e703747674feec3/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.rs) # Examples ```rust #[derive(PartialEq, Eq)] struct Finished {} #[derive(PartialEq, Eq)] struct Processing { status: ProcessStatus, } #[derive(PartialEq, Eq)] enum ProcessStatus { One, Two, Three, } #[derive(PartialEq, Eq)] enum Status { Finished(Finished), Processing(Processing), } fn check_result(_url: &str) -> Status { // fetch status from some server Status::Processing(Processing { status: ProcessStatus::One, }) } fn wait_for_result(url: &str) -> Finished { let mut previous_status = None; loop { match check_result(url) { Status::Finished(f) => return f, Status::Processing(p) => { match (&mut previous_status, p.status) { (None, status) => previous_status = Some(status), // first status (Some(previous), status) if *previous == status => {} // no change, ignore (Some(previous), status) => { // Now it can be used // new status *previous = status; } } } } } } ``` Before, we would have used: ```rust match (&previous_status, p.status) { (Some(previous), status) if *previous == status => {} // no change, ignore (_, status) => { // new status previous_status = Some(status); } } ``` Demonstrating *partial move* ```rust fn main() { #[derive(Debug)] struct Person { name: String, age: u8, } let person = Person { name: String::from("Alice"), age: 20, }; // `name` is moved out of person, but `age` is referenced let Person { name, ref age } = person; println!("The person's age is {}", age); println!("The person's name is {}", name); // Error! borrow of partially moved value: `person` partial move occurs //println!("The person struct is {:?}", person); // `person` cannot be used but `person.age` can be used as it is not moved println!("The person's age from person struct is {}", person.age); } ```

@Centril

…n, r=nikomatsakis Stabilize move_ref_pattern # Implementation - Initially the rule was added in the run-up to 1.0. The AST-based borrow checker was having difficulty correctly enforcing match expressions that combined ref and move bindings, and so it was decided to simplify forbid the combination out right. - The move to MIR-based borrow checking made it possible to enforce the rules in a finer-grained level, but we kept the rule in place in an effort to be conservative in our changes. - In rust-lang#68376, @Centril lifted the restriction but required a feature-gate. - This PR removes the feature-gate. Tracking issue: rust-lang#68354. # Description This PR is to stabilize the feature `move_ref_pattern`, which allows patterns containing both `by-ref` and `by-move` bindings at the same time. For example: `Foo(ref x, y)`, where `x` is `by-ref`, and `y` is `by-move`. The rules of moving a variable also apply here when moving *part* of a variable, such as it can't be referenced or moved before. If this pattern is used, it would result in *partial move*, which means that part of the variable is moved. The variable that was partially moved from cannot be used as a whole in this case, only the parts that are still not moved can be used. ## Documentation - The reference (rust-lang/reference#881) - Rust by example (rust-lang/rust-by-example#1377) ## Tests There are many tests, but I think one of the comperhensive ones: - [borrowck-move-ref-pattern-pass.rs](https://github.com/Centril/rust/blob/85fbf49ce0e2274d0acf798f6e703747674feec3/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern-pass.rs) - [borrowck-move-ref-pattern.rs](https://github.com/Centril/rust/blob/85fbf49ce0e2274d0acf798f6e703747674feec3/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.rs) # Examples ```rust #[derive(PartialEq, Eq)] struct Finished {} #[derive(PartialEq, Eq)] struct Processing { status: ProcessStatus, } #[derive(PartialEq, Eq)] enum ProcessStatus { One, Two, Three, } #[derive(PartialEq, Eq)] enum Status { Finished(Finished), Processing(Processing), } fn check_result(_url: &str) -> Status { // fetch status from some server Status::Processing(Processing { status: ProcessStatus::One, }) } fn wait_for_result(url: &str) -> Finished { let mut previous_status = None; loop { match check_result(url) { Status::Finished(f) => return f, Status::Processing(p) => { match (&mut previous_status, p.status) { (None, status) => previous_status = Some(status), // first status (Some(previous), status) if *previous == status => {} // no change, ignore (Some(previous), status) => { // Now it can be used // new status *previous = status; } } } } } } ``` Before, we would have used: ```rust match (&previous_status, p.status) { (Some(previous), status) if *previous == status => {} // no change, ignore (_, status) => { // new status previous_status = Some(status); } } ``` Demonstrating *partial move* ```rust fn main() { #[derive(Debug)] struct Person { name: String, age: u8, } let person = Person { name: String::from("Alice"), age: 20, }; // `name` is moved out of person, but `age` is referenced let Person { name, ref age } = person; println!("The person's age is {}", age); println!("The person's name is {}", name); // Error! borrow of partially moved value: `person` partial move occurs //println!("The person struct is {:?}", person); // `person` cannot be used but `person.age` can be used as it is not moved println!("The person's age from person struct is {}", person.age); } ```

Centril added F-bindings_after_at `#![feature(bindings_after_at)]` F-move_ref_pattern `#![feature(move_ref_pattern)]` labels Jan 19, 2020

rust-highfive assigned matthewjasper Jan 19, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 19, 2020

Centril force-pushed the move-ref-patterns branch from 7c4b0a1 to f6ecb0a Compare January 19, 2020 23:32

This comment has been minimized.

Sign in to view

Centril force-pushed the move-ref-patterns branch from f6ecb0a to b871dda Compare January 20, 2020 10:03

This comment has been minimized.

Sign in to view

Centril force-pushed the move-ref-patterns branch from b871dda to ab261d4 Compare January 21, 2020 22:30

estebank reviewed Jan 25, 2020

View reviewed changes

src/librustc_error_codes/error_codes/E0009.md Outdated Show resolved Hide resolved

src/librustc_mir_build/hair/pattern/check_match.rs Outdated Show resolved Hide resolved

matthewjasper reviewed Jan 30, 2020

View reviewed changes

src/test/ui/pattern/move-ref-patterns/move-ref-patterns-closure-captures-pass.rs Outdated Show resolved Hide resolved

src/librustc_mir_build/hair/pattern/check_match.rs Outdated Show resolved Hide resolved

src/librustc_mir_build/hair/pattern/check_match.rs Outdated Show resolved Hide resolved

Centril commented Jan 30, 2020

View reviewed changes

src/librustc_mir_build/hair/pattern/check_match.rs Outdated Show resolved Hide resolved

Centril added 3 commits February 2, 2020 14:13

introduce #![feature(move_ref_pattern)]

7af9ff3

move_ref_patterns: introduce tests

d984f12

bindings_after_at: harden tests

move_ref_pattern: adjust error index

0253f86

Centril force-pushed the move-ref-patterns branch from ab261d4 to 0253f86 Compare February 2, 2020 13:13

Centril 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 Feb 2, 2020

Centril added 3 commits February 2, 2020 16:07

move_ref_pattern: don't ICE on unreachable 2xby-move conflicts

8d4973f

move_ref_pattern: change pov in diagnostics & add binding names

bd318be

move_ref_pattern: test captures inside closure

d2b88b7

Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 2, 2020

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2020

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 8, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 9, 2020

bors merged commit d2b88b7 into rust-lang:master Feb 9, 2020

Centril deleted the move-ref-patterns branch February 9, 2020 07:31

Centril mentioned this pull request Feb 10, 2020

Tracking issue for pattern with by-move & by-ref bindings #68354

Closed

Centril mentioned this pull request Mar 8, 2020

test(patterns): add patterns feature tests to borrowck test suite #69817

Merged

Centril added A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-patterns Relating to patterns and pattern matching labels Apr 10, 2020

Amjad50 mentioned this pull request Aug 30, 2020

Stabilize move_ref_pattern #76119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Initial implementation of `#![feature(move_ref_pattern)]`#68376