Skip to content

Commit

Permalink
Auto merge of #78638 - vn-ki:bindigs-after-at-issue-69971, r=oli-obk
Browse files Browse the repository at this point in the history 
reverse binding order in matches to allow the subbinding of copyable fields in bindings after @

Fixes #69971

### TODO

- [x] Regression tests

r? `@oli-obk`
  • Loading branch information
@bors
bors committed Nov 5, 2020
2 parents f7801d6 + 5827fba commit b1d9f31
Show file tree
Hide file tree
Showing 22 changed files with 568 additions and 595 deletions.
44 changes: 44 additions & 0 deletions compiler/rustc_mir_build/src/build/matches/simplify.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,36 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
candidate: &mut Candidate<'pat, 'tcx>,
) -> bool {
// repeatedly simplify match pairs until fixed point is reached
debug!(?candidate, "simplify_candidate");

// existing_bindings and new_bindings exists to keep the semantics in order.
// Reversing the binding order for bindings after `@` changes the binding order in places
// it shouldn't be changed, for example `let (Some(a), Some(b)) = (x, y)`
//
// To avoid this, the binding occurs in the following manner:
// * the bindings for one iteration of the following loop occurs in order (i.e. left to
// right)
// * the bindings from the previous iteration of the loop is prepended to the bindings from
// the current iteration (in the implementation this is done by mem::swap and extend)
// * after all iterations, these new bindings are then appended to the bindings that were
// prexisting (i.e. `candidate.binding` when the function was called).
//
// example:
// candidate.bindings = [1, 2, 3]
// binding in iter 1: [4, 5]
// binding in iter 2: [6, 7]
//
// final binding: [1, 2, 3, 6, 7, 4, 5]
let mut existing_bindings = mem::take(&mut candidate.bindings);
let mut new_bindings = Vec::new();
loop {
let match_pairs = mem::take(&mut candidate.match_pairs);

if let [MatchPair { pattern: Pat { kind: box PatKind::Or { pats }, .. }, place }] =
*match_pairs
{
existing_bindings.extend_from_slice(&new_bindings);
mem::swap(&mut candidate.bindings, &mut existing_bindings);
candidate.subcandidates = self.create_or_subcandidates(candidate, place, pats);
return true;
}
Expand All @@ -64,13 +88,33 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}
}
// Avoid issue #69971: the binding order should be right to left if there are more
// bindings after `@` to please the borrow checker
// Ex
// struct NonCopyStruct {
// copy_field: u32,
// }
//
// fn foo1(x: NonCopyStruct) {
// let y @ NonCopyStruct { copy_field: z } = x;
// // the above should turn into
// let z = x.copy_field;
// let y = x;
// }
candidate.bindings.extend_from_slice(&new_bindings);
mem::swap(&mut candidate.bindings, &mut new_bindings);
candidate.bindings.clear();

if !changed {
existing_bindings.extend_from_slice(&new_bindings);
mem::swap(&mut candidate.bindings, &mut existing_bindings);
// Move or-patterns to the end, because they can result in us
// creating additional candidates, so we want to test them as
// late as possible.
candidate
.match_pairs
.sort_by_key(|pair| matches!(*pair.pattern.kind, PatKind::Or { .. }));
debug!(simplified = ?candidate, "simplify_candidate");
return false; // if we were not able to simplify any, done.
}
}
Expand Down
49 changes: 49 additions & 0 deletions src/test/ui/pattern/bindings-after-at/bind-by-copy.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// run-pass

// Test copy

#![feature(bindings_after_at)]

struct A { a: i32, b: i32 }
struct B { a: i32, b: C }
struct D { a: i32, d: C }
#[derive(Copy,Clone)]
struct C { c: i32 }

pub fn main() {
match (A {a: 10, b: 20}) {
x@A {a, b: 20} => { assert!(x.a == 10); assert!(a == 10); }
A {b: _b, ..} => { panic!(); }
}

let mut x@B {b, ..} = B {a: 10, b: C {c: 20}};
assert_eq!(x.a, 10);
x.b.c = 30;
assert_eq!(b.c, 20);
let mut y@D {d, ..} = D {a: 10, d: C {c: 20}};
assert_eq!(y.a, 10);
y.d.c = 30;
assert_eq!(d.c, 20);

let some_b = Some(B { a: 10, b: C { c: 20 } });

// in irrefutable pattern
if let Some(x @ B { b, .. }) = some_b {
assert_eq!(x.b.c, 20);
assert_eq!(b.c, 20);
} else {
unreachable!();
}

let some_b = Some(B { a: 10, b: C { c: 20 } });

if let Some(x @ B { b: mut b @ C { c }, .. }) = some_b {
assert_eq!(x.b.c, 20);
assert_eq!(b.c, 20);
b.c = 30;
assert_eq!(b.c, 30);
assert_eq!(c, 20);
} else {
unreachable!();
}
}
4 changes: 2 additions & 2 deletions .../ui/pattern/bindings-after-at/bind-by-move-neither-can-live-while-the-other-survives-1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,28 @@ fn main() {
let x = Some(X { x: () });
match x {
Some(ref _y @ _z) => {} //~ ERROR cannot move out of value because it is borrowed
//~| ERROR borrow of moved value
None => panic!(),
}

let x = Some(X { x: () });
match x {
Some(_z @ ref _y) => {}
//~^ ERROR borrow of moved value
//~| ERROR borrow of moved value
None => panic!(),
}

let mut x = Some(X { x: () });
match x {
Some(ref mut _y @ _z) => {} //~ ERROR cannot move out of value because it is borrowed
//~| ERROR borrow of moved value
None => panic!(),
}

let mut x = Some(X { x: () });
match x {
Some(_z @ ref mut _y) => {}
//~^ ERROR borrow of moved value
//~| ERROR borrow of moved value
None => panic!(),
}
}
36 changes: 18 additions & 18 deletions ...pattern/bindings-after-at/bind-by-move-neither-can-live-while-the-other-survives-1.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | Some(ref _y @ _z) => {}
| value borrowed, by `_y`, here

error: borrow of moved value
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:20:14
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:21:14
|
LL | Some(_z @ ref _y) => {}
| --^^^------
Expand All @@ -27,7 +27,7 @@ LL | Some(ref mut _y @ _z) => {}
| value borrowed, by `_y`, here

error: borrow of moved value
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:34:14
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:35:14
|
LL | Some(_z @ ref mut _y) => {}
| --^^^----------
Expand All @@ -37,34 +37,34 @@ LL | Some(_z @ ref mut _y) => {}
| move occurs because `_z` has type `X` which does not implement the `Copy` trait

error[E0382]: borrow of moved value
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:20:19
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:14:14
|
LL | Some(_z @ ref _y) => {}
| -----^^^^^^
| | |
| | value borrowed here after move
| value moved here
LL | Some(ref _y @ _z) => {}
| ^^^^^^^^^--
| | |
| | value moved here
| value borrowed here after move
|
= note: move occurs because value has type `X`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `x.0`
|
LL | Some(ref _z @ ref _y) => {}
| ^^^
LL | Some(ref _y @ ref _z) => {}
| ^^^

error[E0382]: borrow of moved value
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:34:19
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:28:14
|
LL | Some(_z @ ref mut _y) => {}
| -----^^^^^^^^^^
| | |
| | value borrowed here after move
| value moved here
LL | Some(ref mut _y @ _z) => {}
| ^^^^^^^^^^^^^--
| | |
| | value moved here
| value borrowed here after move
|
= note: move occurs because value has type `X`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `x.0`
|
LL | Some(ref _z @ ref mut _y) => {}
| ^^^
LL | Some(ref mut _y @ ref _z) => {}
| ^^^

error: aborting due to 6 previous errors

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/pattern/bindings-after-at/bind-by-move-no-subbindings-fun-param.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn main() {}
struct A(Box<u8>);

fn f(a @ A(u): A) -> Box<u8> {
//~^ ERROR use of moved value
//~^ ERROR use of partially moved value
drop(a);
u
}
13 changes: 7 additions & 6 deletions src/test/ui/pattern/bindings-after-at/bind-by-move-no-subbindings-fun-param.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
error[E0382]: use of moved value
--> $DIR/bind-by-move-no-subbindings-fun-param.rs:9:12
error[E0382]: use of partially moved value
--> $DIR/bind-by-move-no-subbindings-fun-param.rs:9:6
|
LL | fn f(a @ A(u): A) -> Box<u8> {
| ------^-
| ^^^^^^-^
| | |
| | value used here after move
| value moved here
| move occurs because value has type `A`, which does not implement the `Copy` trait
| | value partially moved here
| value used here after partial move
|
= note: partial move occurs because value has type `Box<u8>`, which does not implement the `Copy` trait

error: aborting due to previous error

Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/pattern/bindings-after-at/borrowck-move-and-move.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ fn main() {

let a @ b = U; //~ ERROR use of moved value

let a @ (b, c) = (U, U); //~ ERROR use of moved value
let a @ (b, c) = (U, U); //~ ERROR use of partially moved value

let a @ (b, c) = (u(), u()); //~ ERROR use of moved value
let a @ (b, c) = (u(), u()); //~ ERROR use of partially moved value

match Ok(U) {
a @ Ok(b) | a @ Err(b) => {} //~ ERROR use of moved value
Expand All @@ -24,10 +24,10 @@ fn main() {
fn fun(a @ b: U) {} //~ ERROR use of moved value

match [u(), u(), u(), u()] {
xs @ [a, .., b] => {} //~ ERROR use of moved value
xs @ [a, .., b] => {} //~ ERROR use of partially moved value
}

match [u(), u(), u(), u()] {
xs @ [_, ys @ .., _] => {} //~ ERROR use of moved value
xs @ [_, ys @ .., _] => {} //~ ERROR use of partially moved value
}
}
68 changes: 36 additions & 32 deletions src/test/ui/pattern/bindings-after-at/borrowck-move-and-move.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,33 @@
error[E0382]: use of moved value
--> $DIR/borrowck-move-and-move.rs:13:13
--> $DIR/borrowck-move-and-move.rs:13:9
|
LL | let a @ b = U;
| ----^ - move occurs because value has type `U`, which does not implement the `Copy` trait
| ^^^^- - move occurs because value has type `U`, which does not implement the `Copy` trait
| | |
| | value used here after move
| value moved here
| | value moved here
| value used here after move

error[E0382]: use of moved value
--> $DIR/borrowck-move-and-move.rs:15:17
error[E0382]: use of partially moved value
--> $DIR/borrowck-move-and-move.rs:15:9
|
LL | let a @ (b, c) = (U, U);
| --------^- ------ move occurs because value has type `(U, U)`, which does not implement the `Copy` trait
| ^^^^^^^^-^
| | |
| | value used here after move
| value moved here
| | value partially moved here
| value used here after partial move
|
= note: partial move occurs because value has type `U`, which does not implement the `Copy` trait

error[E0382]: use of moved value
--> $DIR/borrowck-move-and-move.rs:17:17
error[E0382]: use of partially moved value
--> $DIR/borrowck-move-and-move.rs:17:9
|
LL | let a @ (b, c) = (u(), u());
| --------^- ---------- move occurs because value has type `(U, U)`, which does not implement the `Copy` trait
| ^^^^^^^^-^
| | |
| | value used here after move
| value moved here
| | value partially moved here
| value used here after partial move
|
= note: partial move occurs because value has type `U`, which does not implement the `Copy` trait

error[E0382]: use of moved value
--> $DIR/borrowck-move-and-move.rs:20:16
Expand All @@ -47,36 +51,36 @@ LL | a @ Ok(b) | a @ Err(b) => {}
| | value used here after move
| value moved here

error[E0382]: use of moved value
--> $DIR/borrowck-move-and-move.rs:27:22
error[E0382]: use of partially moved value
--> $DIR/borrowck-move-and-move.rs:27:9
|
LL | match [u(), u(), u(), u()] {
| -------------------- move occurs because value has type `[U; 4]`, which does not implement the `Copy` trait
LL | xs @ [a, .., b] => {}
| -------------^-
| ^^^^^^^^^^^^^-^
| | |
| | value used here after move
| value moved here
| | value partially moved here
| value used here after partial move
|
= note: partial move occurs because value has type `U`, which does not implement the `Copy` trait

error[E0382]: use of moved value
--> $DIR/borrowck-move-and-move.rs:31:18
error[E0382]: use of partially moved value
--> $DIR/borrowck-move-and-move.rs:31:9
|
LL | match [u(), u(), u(), u()] {
| -------------------- move occurs because value has type `[U; 4]`, which does not implement the `Copy` trait
LL | xs @ [_, ys @ .., _] => {}
| ---------^^^^^^^----
| ^^^^^^^^^-------^^^^
| | |
| | value used here after move
| value moved here
| | value partially moved here
| value used here after partial move
|
= note: partial move occurs because value has type `U`, which does not implement the `Copy` trait

error[E0382]: use of moved value
--> $DIR/borrowck-move-and-move.rs:24:16
--> $DIR/borrowck-move-and-move.rs:24:12
|
LL | fn fun(a @ b: U) {}
| ----^
| ^^^^-
| | |
| | value used here after move
| value moved here
| | value moved here
| value used here after move
| move occurs because value has type `U`, which does not implement the `Copy` trait

error: aborting due to 8 previous errors
Expand Down
Loading

0 comments on commit b1d9f31

Please sign in to comment.