bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #76119 - Amjad50:stabilizing-move_ref_pattern, r=nikomatsakis

Browse source 
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 #68376, @Centril lifted the restriction but required a feature-gate.
- This PR removes the feature-gate.

Tracking issue: #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](85fbf49ce0/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern-pass.rs)
- [borrowck-move-ref-pattern.rs](85fbf49ce0/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);
}
```
This commit is contained in:
Dylan DPC 2020-10-16 02:10:07 +02:00 • committed by GitHub
commit 85dbb03490
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
50 changed files with 277 additions and 494 deletions
Download patch file Download diff file Expand all files Collapse all files

70
compiler/rustc_mir_build/src/thir/pattern/check_match.rs
View file

@ -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);
}
}
@ -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);
@ -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);
@ -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: