From 5d31e29b56c9e7c8fad184eae8f5eae351f267d8 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 12 Feb 2024 21:59:17 +0100 Subject: [PATCH 1/3] Appease rust-analyzer For some reason it doesn't figure out the slice coercion. --- compiler/rustc_mir_build/src/build/matches/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 641a278c1d3..f37e8606cac 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -701,7 +701,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.bind_pattern( self.source_info(irrefutable_pat.span), candidate, - &fake_borrow_temps, + fake_borrow_temps.as_slice(), irrefutable_pat.span, None, false, @@ -1981,7 +1981,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let post_guard_block = self.bind_pattern( self.source_info(pat.span), guard_candidate, - &fake_borrow_temps, + fake_borrow_temps.as_slice(), expr_span, None, false, @@ -2468,7 +2468,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let matching = this.bind_pattern( this.source_info(pattern.span), candidate, - &fake_borrow_temps, + fake_borrow_temps.as_slice(), initializer_span, None, true, @@ -2477,7 +2477,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let failure = this.bind_pattern( this.source_info(else_block_span), wildcard, - &fake_borrow_temps, + fake_borrow_temps.as_slice(), initializer_span, None, true, From 97bfa106c7a5f916bed7b0a329276cabb20f58ea Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 28 Feb 2024 00:55:01 +0100 Subject: [PATCH 2/3] Add test for the known case that doesn't work --- .../bindings-after-at/bind-by-copy-or-pat.rs | 17 ++++++++++ .../bind-by-copy-or-pat.stderr | 31 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.rs create mode 100644 tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.stderr diff --git a/tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.rs b/tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.rs new file mode 100644 index 00000000000..1555da2fd1f --- /dev/null +++ b/tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.rs @@ -0,0 +1,17 @@ +//@ known-bug: unknown +#![allow(unused)] + +struct A(u32); + +pub fn main() { + // The or-pattern bindings are lowered after `x`, which triggers the error. + let x @ (A(a) | A(a)) = A(10); + // ERROR: use of moved value + assert!(x.0 == 10); + assert!(a == 10); + + // This works. + let (x @ A(a) | x @ A(a)) = A(10); + assert!(x.0 == 10); + assert!(a == 10); +} diff --git a/tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.stderr b/tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.stderr new file mode 100644 index 00000000000..79808186358 --- /dev/null +++ b/tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.stderr @@ -0,0 +1,31 @@ +error[E0382]: use of moved value + --> $DIR/bind-by-copy-or-pat.rs:8:16 + | +LL | let x @ (A(a) | A(a)) = A(10); + | - ^ ----- move occurs because value has type `A`, which does not implement the `Copy` trait + | | | + | | value used here after move + | value moved here + | +help: borrow this binding in the pattern to avoid moving the value + | +LL | let ref x @ (A(a) | A(a)) = A(10); + | +++ + +error[E0382]: use of moved value + --> $DIR/bind-by-copy-or-pat.rs:8:23 + | +LL | let x @ (A(a) | A(a)) = A(10); + | - ^ ----- move occurs because value has type `A`, which does not implement the `Copy` trait + | | | + | | value used here after move + | value moved here + | +help: borrow this binding in the pattern to avoid moving the value + | +LL | let ref x @ (A(a) | A(a)) = A(10); + | +++ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0382`. From 00497ad24bcef346b2324ae4ad7b7c68315272c0 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 28 Feb 2024 00:34:33 +0100 Subject: [PATCH 3/3] Lower bindings in a predictable order --- .../src/build/matches/simplify.rs | 70 ++++++------------- ...wise_branch.opt1.EarlyOtherwiseBranch.diff | 6 +- ...wise_branch.opt2.EarlyOtherwiseBranch.diff | 6 +- ...wise_branch.opt3.EarlyOtherwiseBranch.diff | 6 +- ...ement_tuple.opt1.EarlyOtherwiseBranch.diff | 12 ++-- ...ch_68867.try_sum.EarlyOtherwiseBranch.diff | 48 ++++++------- ...nch_noopt.noopt1.EarlyOtherwiseBranch.diff | 6 +- ....match_tuple.SimplifyCfg-initial.after.mir | 8 +-- 8 files changed, 66 insertions(+), 96 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index 53a5056cc3f..b5f0a97b28a 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -43,60 +43,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // let y = x; // } // - // We can't just reverse the binding order, because we must preserve pattern-order - // otherwise, e.g. in `let (Some(a), Some(b)) = (x, y)`. Our rule then is: deepest-first, - // and bindings at the same depth stay in source order. - // - // To do this, every time around the loop we prepend the newly found bindings to the - // bindings we already had. - // - // example: - // candidate.bindings = [1, 2, 3] - // bindings in iter 1: [4, 5] - // bindings in iter 2: [6, 7] - // - // final bindings: [6, 7, 4, 5, 1, 2, 3] - let mut accumulated_bindings = mem::take(candidate_bindings); - let mut simplified_match_pairs = Vec::new(); - // Repeatedly simplify match pairs until we're left with only unsimplifiable ones. - loop { - for mut match_pair in mem::take(match_pairs) { - if let TestCase::Irrefutable { binding, ascription } = match_pair.test_case { - if let Some(binding) = binding { - candidate_bindings.push(binding); - } - if let Some(ascription) = ascription { - candidate_ascriptions.push(ascription); - } - // Simplifiable pattern; we replace it with its subpairs and simplify further. - match_pairs.append(&mut match_pair.subpairs); - } else { - // Unsimplifiable pattern; we recursively simplify its subpairs and don't - // process it further. - self.simplify_match_pairs( - &mut match_pair.subpairs, - candidate_bindings, - candidate_ascriptions, - ); - simplified_match_pairs.push(match_pair); + // We therefore lower bindings from left-to-right, except we lower the `x` in `x @ pat` + // after any bindings in `pat`. This doesn't work for or-patterns: the current structure of + // match lowering forces us to lower bindings inside or-patterns last. + for mut match_pair in mem::take(match_pairs) { + self.simplify_match_pairs( + &mut match_pair.subpairs, + candidate_bindings, + candidate_ascriptions, + ); + if let TestCase::Irrefutable { binding, ascription } = match_pair.test_case { + if let Some(binding) = binding { + candidate_bindings.push(binding); } - } - - // This does: accumulated_bindings = candidate.bindings.take() ++ accumulated_bindings - candidate_bindings.extend_from_slice(&accumulated_bindings); - mem::swap(candidate_bindings, &mut accumulated_bindings); - candidate_bindings.clear(); - - if match_pairs.is_empty() { - break; + if let Some(ascription) = ascription { + candidate_ascriptions.push(ascription); + } + // Simplifiable pattern; we replace it with its already simplified subpairs. + match_pairs.append(&mut match_pair.subpairs); + } else { + // Unsimplifiable pattern; we keep it. + match_pairs.push(match_pair); } } - // Store computed bindings back in `candidate_bindings`. - mem::swap(candidate_bindings, &mut accumulated_bindings); - // Store simplified match pairs back in `match_pairs`. - mem::swap(match_pairs, &mut simplified_match_pairs); - // 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. diff --git a/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff index 8b427cff677..7a374c5675a 100644 --- a/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff @@ -51,13 +51,13 @@ - } - - bb3: { - StorageLive(_9); - _9 = (((_3.1: std::option::Option) as Some).0: u32); StorageLive(_8); _8 = (((_3.0: std::option::Option) as Some).0: u32); + StorageLive(_9); + _9 = (((_3.1: std::option::Option) as Some).0: u32); _0 = const 0_u32; - StorageDead(_8); StorageDead(_9); + StorageDead(_8); - goto -> bb4; + goto -> bb3; } diff --git a/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff index 32a8dd8b8b4..95bcfe71792 100644 --- a/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff @@ -57,13 +57,13 @@ - } - - bb4: { - StorageLive(_10); - _10 = (((_3.1: std::option::Option) as Some).0: u32); StorageLive(_9); _9 = (((_3.0: std::option::Option) as Some).0: u32); + StorageLive(_10); + _10 = (((_3.1: std::option::Option) as Some).0: u32); _0 = const 0_u32; - StorageDead(_9); StorageDead(_10); + StorageDead(_9); - goto -> bb6; + goto -> bb4; } diff --git a/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff index cc16af721ca..e058c409cb5 100644 --- a/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff @@ -51,13 +51,13 @@ - } - - bb3: { - StorageLive(_9); - _9 = (((_3.1: std::option::Option) as Some).0: bool); StorageLive(_8); _8 = (((_3.0: std::option::Option) as Some).0: u32); + StorageLive(_9); + _9 = (((_3.1: std::option::Option) as Some).0: bool); _0 = const 0_u32; - StorageDead(_8); StorageDead(_9); + StorageDead(_8); - goto -> bb4; + goto -> bb3; } diff --git a/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff index eb8926d27ee..f98d68e6ffc 100644 --- a/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff @@ -69,16 +69,16 @@ - bb4: { + bb3: { - StorageLive(_13); - _13 = (((_4.2: std::option::Option) as Some).0: u32); - StorageLive(_12); - _12 = (((_4.1: std::option::Option) as Some).0: u32); StorageLive(_11); _11 = (((_4.0: std::option::Option) as Some).0: u32); + StorageLive(_12); + _12 = (((_4.1: std::option::Option) as Some).0: u32); + StorageLive(_13); + _13 = (((_4.2: std::option::Option) as Some).0: u32); _0 = const 0_u32; - StorageDead(_11); - StorageDead(_12); StorageDead(_13); + StorageDead(_12); + StorageDead(_11); - goto -> bb5; + goto -> bb4; } diff --git a/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff index 6179bab11fe..a5b5659a31a 100644 --- a/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff @@ -116,12 +116,12 @@ } bb6: { - StorageLive(_13); - _39 = deref_copy (_4.1: &ViewportPercentageLength); - _13 = (((*_39) as Vw).0: f32); StorageLive(_12); - _40 = deref_copy (_4.0: &ViewportPercentageLength); - _12 = (((*_40) as Vw).0: f32); + _39 = deref_copy (_4.0: &ViewportPercentageLength); + _12 = (((*_39) as Vw).0: f32); + StorageLive(_13); + _40 = deref_copy (_4.1: &ViewportPercentageLength); + _13 = (((*_40) as Vw).0: f32); StorageLive(_14); StorageLive(_15); _15 = _12; @@ -132,18 +132,18 @@ StorageDead(_15); _3 = ViewportPercentageLength::Vw(move _14); StorageDead(_14); - StorageDead(_12); StorageDead(_13); + StorageDead(_12); goto -> bb10; } bb7: { - StorageLive(_18); - _41 = deref_copy (_4.1: &ViewportPercentageLength); - _18 = (((*_41) as Vh).0: f32); StorageLive(_17); - _42 = deref_copy (_4.0: &ViewportPercentageLength); - _17 = (((*_42) as Vh).0: f32); + _41 = deref_copy (_4.0: &ViewportPercentageLength); + _17 = (((*_41) as Vh).0: f32); + StorageLive(_18); + _42 = deref_copy (_4.1: &ViewportPercentageLength); + _18 = (((*_42) as Vh).0: f32); StorageLive(_19); StorageLive(_20); _20 = _17; @@ -154,18 +154,18 @@ StorageDead(_20); _3 = ViewportPercentageLength::Vh(move _19); StorageDead(_19); - StorageDead(_17); StorageDead(_18); + StorageDead(_17); goto -> bb10; } bb8: { - StorageLive(_23); - _43 = deref_copy (_4.1: &ViewportPercentageLength); - _23 = (((*_43) as Vmin).0: f32); StorageLive(_22); - _44 = deref_copy (_4.0: &ViewportPercentageLength); - _22 = (((*_44) as Vmin).0: f32); + _43 = deref_copy (_4.0: &ViewportPercentageLength); + _22 = (((*_43) as Vmin).0: f32); + StorageLive(_23); + _44 = deref_copy (_4.1: &ViewportPercentageLength); + _23 = (((*_44) as Vmin).0: f32); StorageLive(_24); StorageLive(_25); _25 = _22; @@ -176,18 +176,18 @@ StorageDead(_25); _3 = ViewportPercentageLength::Vmin(move _24); StorageDead(_24); - StorageDead(_22); StorageDead(_23); + StorageDead(_22); goto -> bb10; } bb9: { - StorageLive(_28); - _45 = deref_copy (_4.1: &ViewportPercentageLength); - _28 = (((*_45) as Vmax).0: f32); StorageLive(_27); - _46 = deref_copy (_4.0: &ViewportPercentageLength); - _27 = (((*_46) as Vmax).0: f32); + _45 = deref_copy (_4.0: &ViewportPercentageLength); + _27 = (((*_45) as Vmax).0: f32); + StorageLive(_28); + _46 = deref_copy (_4.1: &ViewportPercentageLength); + _28 = (((*_46) as Vmax).0: f32); StorageLive(_29); StorageLive(_30); _30 = _27; @@ -198,8 +198,8 @@ StorageDead(_30); _3 = ViewportPercentageLength::Vmax(move _29); StorageDead(_29); - StorageDead(_27); StorageDead(_28); + StorageDead(_27); goto -> bb10; } diff --git a/tests/mir-opt/early_otherwise_branch_noopt.noopt1.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_noopt.noopt1.EarlyOtherwiseBranch.diff index d7908ab3cd2..7fdd8554e38 100644 --- a/tests/mir-opt/early_otherwise_branch_noopt.noopt1.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_noopt.noopt1.EarlyOtherwiseBranch.diff @@ -59,13 +59,13 @@ } bb5: { - StorageLive(_10); - _10 = (((_3.1: std::option::Option) as Some).0: u32); StorageLive(_9); _9 = (((_3.0: std::option::Option) as Some).0: u32); + StorageLive(_10); + _10 = (((_3.1: std::option::Option) as Some).0: u32); _0 = const 0_u32; - StorageDead(_9); StorageDead(_10); + StorageDead(_9); goto -> bb8; } diff --git a/tests/mir-opt/exponential_or.match_tuple.SimplifyCfg-initial.after.mir b/tests/mir-opt/exponential_or.match_tuple.SimplifyCfg-initial.after.mir index ea5cd55b560..596dcef85fd 100644 --- a/tests/mir-opt/exponential_or.match_tuple.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/exponential_or.match_tuple.SimplifyCfg-initial.after.mir @@ -19,8 +19,7 @@ fn match_tuple(_1: (u32, bool, Option, u32)) -> u32 { bb0: { PlaceMention(_1); - _2 = discriminant((_1.2: std::option::Option)); - switchInt(move _2) -> [0: bb3, 1: bb2, otherwise: bb1]; + switchInt((_1.0: u32)) -> [1: bb2, 4: bb2, otherwise: bb1]; } bb1: { @@ -29,11 +28,12 @@ fn match_tuple(_1: (u32, bool, Option, u32)) -> u32 { } bb2: { - switchInt((((_1.2: std::option::Option) as Some).0: i32)) -> [1: bb3, 8: bb3, otherwise: bb1]; + _2 = discriminant((_1.2: std::option::Option)); + switchInt(move _2) -> [0: bb4, 1: bb3, otherwise: bb1]; } bb3: { - switchInt((_1.0: u32)) -> [1: bb4, 4: bb4, otherwise: bb1]; + switchInt((((_1.2: std::option::Option) as Some).0: i32)) -> [1: bb4, 8: bb4, otherwise: bb1]; } bb4: {