Skip to content

Commit

Permalink
Rollup merge of rust-lang#76119 - Amjad50:stabilizing-move_ref_patter…
Browse files Browse the repository at this point in the history
…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);
}
```
  • Loading branch information
JohnTitor authored Oct 15, 2020
2 parents 95e810b + afb9eeb commit c8ca030
Show file tree
Hide file tree
Showing 50 changed files with 277 additions and 494 deletions.
8 changes: 6 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0007.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#### Note: this error code is no longer emitted by the compiler.

This error indicates that the bindings in a match arm would require a value to
be moved into more than one location, thus violating unique ownership. Code
like the following is invalid as it requires the entire `Option<String>` to be
Expand All @@ -6,11 +8,13 @@ inner `String` to be moved into a variable called `s`.

Erroneous code example:

```compile_fail,E0007
```compile_fail,E0382
#![feature(bindings_after_at)]
let x = Some("s".to_string());
match x {
op_string @ Some(s) => {}, // error: cannot bind by-move with sub-bindings
op_string @ Some(s) => {}, // error: use of moved value
None => {},
}
```
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_feature/src/accepted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,9 @@ declare_features! (
(accepted, track_caller, "1.46.0", Some(47809), None),
/// Allows `#[doc(alias = "...")]`.
(accepted, doc_alias, "1.48.0", Some(50146), None),
/// Allows patterns with concurrent by-move and by-ref bindings.
/// For example, you can write `Foo(a, ref b)` where `a` is by-move and `b` is by-ref.
(accepted, move_ref_pattern, "1.48.0", Some(68354), None),

// -------------------------------------------------------------------------
// feature-group-end: accepted features
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,6 @@ declare_features! (
/// For example, you can write `x @ Some(y)`.
(active, bindings_after_at, "1.41.0", Some(65490), None),

/// Allows patterns with concurrent by-move and by-ref bindings.
/// For example, you can write `Foo(a, ref b)` where `a` is by-move and `b` is by-ref.
(active, move_ref_pattern, "1.42.0", Some(68354), None),

/// Allows `impl const Trait for T` syntax.
(active, const_trait_impl, "1.42.0", Some(67792), None),

Expand Down
70 changes: 4 additions & 66 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> {
hir::LocalSource::AwaitDesugar => ("`await` future binding", None),
};
self.check_irrefutable(&loc.pat, msg, sp);
self.check_patterns(false, &loc.pat);
self.check_patterns(&loc.pat);
}

fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
intravisit::walk_param(self, param);
self.check_irrefutable(&param.pat, "function argument", None);
self.check_patterns(false, &param.pat);
self.check_patterns(&param.pat);
}
}

Expand Down Expand Up @@ -119,10 +119,7 @@ impl PatCtxt<'_, '_> {
}

impl<'tcx> MatchVisitor<'_, 'tcx> {
fn check_patterns(&mut self, has_guard: bool, pat: &Pat<'_>) {
if !self.tcx.features().move_ref_pattern {
check_legality_of_move_bindings(self, has_guard, pat);
}
fn check_patterns(&mut self, pat: &Pat<'_>) {
pat.walk_always(|pat| check_borrow_conflicts_in_at_patterns(self, pat));
if !self.tcx.features().bindings_after_at {
check_legality_of_bindings_in_at_patterns(self, pat);
Expand Down Expand Up @@ -165,7 +162,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
) {
for arm in arms {
// Check the arm for some things unrelated to exhaustiveness.
self.check_patterns(arm.guard.is_some(), &arm.pat);
self.check_patterns(&arm.pat);
}

let mut cx = self.new_cx(scrut.hir_id);
Expand Down Expand Up @@ -601,65 +598,6 @@ fn is_binding_by_move(cx: &MatchVisitor<'_, '_>, hir_id: HirId, span: Span) -> b
!cx.typeck_results.node_type(hir_id).is_copy_modulo_regions(cx.tcx.at(span), cx.param_env)
}

/// Check the legality of legality of by-move bindings.
fn check_legality_of_move_bindings(cx: &mut MatchVisitor<'_, '_>, has_guard: bool, pat: &Pat<'_>) {
let sess = cx.tcx.sess;
let typeck_results = cx.typeck_results;

// Find all by-ref spans.
let mut by_ref_spans = Vec::new();
pat.each_binding(|_, hir_id, span, _| {
if let Some(ty::BindByReference(_)) =
typeck_results.extract_binding_mode(sess, hir_id, span)
{
by_ref_spans.push(span);
}
});

// Find bad by-move spans:
let by_move_spans = &mut Vec::new();
let mut check_move = |p: &Pat<'_>, sub: Option<&Pat<'_>>| {
// Check legality of moving out of the enum.
//
// `x @ Foo(..)` is legal, but `x @ Foo(y)` isn't.
if sub.map_or(false, |p| p.contains_bindings()) {
struct_span_err!(sess, p.span, E0007, "cannot bind by-move with sub-bindings")
.span_label(p.span, "binds an already bound by-move value by moving it")
.emit();
} else if !has_guard && !by_ref_spans.is_empty() {
by_move_spans.push(p.span);
}
};
pat.walk_always(|p| {
if let hir::PatKind::Binding(.., sub) = &p.kind {
if let Some(ty::BindByValue(_)) =
typeck_results.extract_binding_mode(sess, p.hir_id, p.span)
{
if is_binding_by_move(cx, p.hir_id, p.span) {
check_move(p, sub.as_deref());
}
}
}
});

// Found some bad by-move spans, error!
if !by_move_spans.is_empty() {
let mut err = feature_err(
&sess.parse_sess,
sym::move_ref_pattern,
by_move_spans.clone(),
"binding by-move and by-ref in the same pattern is unstable",
);
for span in by_ref_spans.iter() {
err.span_label(*span, "by-ref pattern here");
}
for span in by_move_spans.iter() {
err.span_label(*span, "by-move pattern here");
}
err.emit();
}
}

/// Check that there are no borrow or move conflicts in `binding @ subpat` patterns.
///
/// For example, this would reject:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
#![feature(or_patterns)]
#![feature(box_patterns)]

#![feature(move_ref_pattern)]

enum Test {
Foo,
Bar,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: cannot borrow value as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:40:9
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:38:9
|
LL | ref foo @ [.., ref mut bar] => (),
| -------^^^^^^^^-----------^
Expand All @@ -8,7 +8,7 @@ LL | ref foo @ [.., ref mut bar] => (),
| immutable borrow, by `foo`, occurs here

error: cannot borrow value as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:124:9
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:122:9
|
LL | ref foo @ Some(box ref mut s) => (),
| -------^^^^^^^^^^^^---------^
Expand All @@ -17,7 +17,7 @@ LL | ref foo @ Some(box ref mut s) => (),
| immutable borrow, by `foo`, occurs here

error[E0382]: borrow of moved value: `x`
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:22:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:20:5
|
LL | fn bindings_after_at_slice_patterns_move_binding(x: [String; 4]) {
| - move occurs because `x` has type `[String; 4]`, which does not implement the `Copy` trait
Expand All @@ -29,7 +29,7 @@ LL | &x;
| ^^ value borrowed here after move

error[E0502]: cannot borrow `x` as immutable because it is also borrowed as mutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:32:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:30:5
|
LL | ref mut foo @ [.., _] => Some(foo),
| --------------------- mutable borrow occurs here
Expand All @@ -41,7 +41,7 @@ LL | drop(r);
| - mutable borrow later used here

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:54:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:52:5
|
LL | [ref foo @ .., ref bar] => Some(foo),
| ------------ immutable borrow occurs here
Expand All @@ -53,7 +53,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:66:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:64:5
|
LL | ref foo @ [.., ref bar] => Some(foo),
| ----------------------- immutable borrow occurs here
Expand All @@ -65,7 +65,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0382]: borrow of moved value: `x`
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:80:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:78:5
|
LL | fn bindings_after_at_or_patterns_move(x: Option<Test>) {
| - move occurs because `x` has type `Option<Test>`, which does not implement the `Copy` trait
Expand All @@ -80,7 +80,7 @@ LL | &x;
| ^^ value borrowed here after move

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:90:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:88:5
|
LL | ref foo @ Some(Test::Foo | Test::Bar) => Some(foo),
| ------------------------------------- immutable borrow occurs here
Expand All @@ -92,7 +92,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0502]: cannot borrow `x` as immutable because it is also borrowed as mutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:102:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:100:5
|
LL | ref mut foo @ Some(Test::Foo | Test::Bar) => Some(foo),
| ----------------------------------------- mutable borrow occurs here
Expand All @@ -104,7 +104,7 @@ LL | drop(r);
| - mutable borrow later used here

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:116:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:114:5
|
LL | ref foo @ Some(box ref s) => Some(foo),
| ------------------------- immutable borrow occurs here
Expand All @@ -116,7 +116,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0382]: borrow of moved value: `x`
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:138:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:136:5
|
LL | fn bindings_after_at_slice_patterns_or_patterns_moves(x: [Option<Test>; 4]) {
| - move occurs because `x` has type `[Option<Test>; 4]`, which does not implement the `Copy` trait
Expand All @@ -131,7 +131,7 @@ LL | &x;
| ^^ value borrowed here after move

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:148:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:146:5
|
LL | ref a @ [ref b @ .., Some(Test::Foo | Test::Bar)] => Some(a),
| ------------------------------------------------- immutable borrow occurs here
Expand All @@ -143,7 +143,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:160:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:158:5
|
LL | ref a @ [ref b @ .., Some(Test::Foo | Test::Bar)] => Some(b),
| ---------- immutable borrow occurs here
Expand All @@ -155,7 +155,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:174:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:172:5
|
LL | [_, ref a @ Some(box ref b), ..] => Some(a),
| ----------------------- immutable borrow occurs here
Expand All @@ -167,7 +167,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:190:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:188:5
|
LL | [_, ref a @ Some(box Test::Foo | box Test::Bar), ..] => Some(a),
| ------------------------------------------- immutable borrow occurs here
Expand All @@ -179,7 +179,7 @@ LL | drop(r);
| - immutable borrow later used here

error[E0502]: cannot borrow `x` as immutable because it is also borrowed as mutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:204:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:202:5
|
LL | [_, ref mut a @ Some(box Test::Foo | box Test::Bar), ..] => Some(a),
| ----------------------------------------------- mutable borrow occurs here
Expand All @@ -191,7 +191,7 @@ LL | drop(r);
| - mutable borrow later used here

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:218:5
--> $DIR/bindings-after-at-or-patterns-slice-patterns-box-patterns.rs:216:5
|
LL | ref a @ [_, ref b @ Some(box Test::Foo | box Test::Bar), ..] => Some(a),
| ------------------------------------------------------------ immutable borrow occurs here
Expand Down
2 changes: 0 additions & 2 deletions src/test/ui/drop/dynamic-drop-async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
// edition:2018
// ignore-wasm32-bare compiled with panic=abort by default

#![feature(move_ref_pattern)]

#![allow(unused)]

use std::{
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/drop/dynamic-drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// ignore-wasm32-bare compiled with panic=abort by default

#![feature(generators, generator_trait, untagged_unions)]
#![feature(move_ref_pattern)]
#![feature(bindings_after_at)]

#![allow(unused_assignments)]
Expand Down
11 changes: 0 additions & 11 deletions src/test/ui/error-codes/E0007.rs

This file was deleted.

22 changes: 0 additions & 22 deletions src/test/ui/error-codes/E0007.stderr

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// where one side is by-ref and the other is by-move.

#![feature(bindings_after_at)]
#![feature(move_ref_pattern)]

struct X {
x: (),
Expand Down
Loading

0 comments on commit c8ca030

Please sign in to comment.