Skip to content

Commit e902878

Browse files
committed
Clarify the binding dance
1 parent 96ff1a4 commit e902878

File tree

1 file changed

+41
-22
lines changed

1 file changed

+41
-22
lines changed

compiler/rustc_mir_build/src/build/matches/simplify.rs

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
3838
&mut self,
3939
candidate: &mut Candidate<'pat, 'tcx>,
4040
) -> bool {
41-
// repeatedly simplify match pairs until fixed point is reached
4241
debug!("{candidate:#?}");
43-
44-
// existing_bindings and new_bindings exists to keep the semantics in order.
42+
// `original_bindings` and `new_bindings` exist to keep the semantics in order.
4543
// Reversing the binding order for bindings after `@` changes the binding order in places
46-
// it shouldn't be changed, for example `let (Some(a), Some(b)) = (x, y)`
44+
// where it shouldn't be changed, for example `let (Some(a), Some(b)) = (x, y)`.
4745
//
4846
// To avoid this, the binding occurs in the following manner:
49-
// * the bindings for one iteration of the following loop occurs in order (i.e. left to
50-
// right)
47+
// * the bindings for one iteration of the loop occurs in order (i.e. left to right)
5148
// * the bindings from the previous iteration of the loop is prepended to the bindings from
5249
// the current iteration (in the implementation this is done by mem::swap and extend)
5350
// * after all iterations, these new bindings are then appended to the bindings that were
@@ -59,8 +56,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
5956
// binding in iter 2: [6, 7]
6057
//
6158
// final binding: [1, 2, 3, 6, 7, 4, 5]
62-
let mut existing_bindings = mem::take(&mut candidate.bindings);
59+
//
60+
// This is because we treat refutable and irrefutable bindings differently. The binding
61+
// order should be right-to-left if there are more _irrefutable_ bindings after `@` to
62+
// please the borrow checker (#69971)
63+
// Ex
64+
// struct NonCopyStruct {
65+
// copy_field: u32,
66+
// }
67+
//
68+
// fn foo1(x: NonCopyStruct) {
69+
// let y @ NonCopyStruct { copy_field: z } = x;
70+
// // the above should turn into
71+
// let z = x.copy_field;
72+
// let y = x;
73+
// }
74+
//
75+
// If however the bindings are refutable, i.e. under a test, then we keep the bindings
76+
// left-to-right.
77+
// Ex
78+
// enum NonCopyEnum {
79+
// Variant { copy_field: u32 },
80+
// None,
81+
// }
82+
//
83+
// fn foo2(x: NonCopyEnum) {
84+
// let y @ NonCopyEnum::Variant { copy_field: z } = x else { return };
85+
// // turns into
86+
// let y = x;
87+
// let z = (x as Variant).copy_field;
88+
// // and raises an error
89+
// }
90+
let mut original_bindings = mem::take(&mut candidate.bindings);
6391
let mut new_bindings = Vec::new();
92+
// Repeatedly simplify match pairs until fixed point is reached
6493
loop {
6594
let mut changed = false;
6695
for match_pair in mem::take(&mut candidate.match_pairs) {
@@ -74,19 +103,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
74103
}
75104
}
76105

77-
// Avoid issue #69971: the binding order should be right to left if there are more
78-
// bindings after `@` to please the borrow checker
79-
// Ex
80-
// struct NonCopyStruct {
81-
// copy_field: u32,
82-
// }
83-
//
84-
// fn foo1(x: NonCopyStruct) {
85-
// let y @ NonCopyStruct { copy_field: z } = x;
86-
// // the above should turn into
87-
// let z = x.copy_field;
88-
// let y = x;
89-
// }
106+
// This does: new_bindings = candidate.bindings.take() ++ new_bindings
90107
candidate.bindings.extend_from_slice(&new_bindings);
91108
mem::swap(&mut candidate.bindings, &mut new_bindings);
92109
candidate.bindings.clear();
@@ -97,8 +114,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
97114
}
98115
}
99116

100-
existing_bindings.extend_from_slice(&new_bindings);
101-
mem::swap(&mut candidate.bindings, &mut existing_bindings);
117+
// Restore original bindings and append the new ones.
118+
// This does: candidate.bindings = original_bindings ++ new_bindings
119+
mem::swap(&mut candidate.bindings, &mut original_bindings);
120+
candidate.bindings.extend_from_slice(&new_bindings);
102121

103122
let did_expand_or =
104123
if let [MatchPair { pattern: Pat { kind: PatKind::Or { pats }, .. }, place }] =

0 commit comments

Comments
 (0)