From c15437c0c6beb2b7be4b86569e124c8b2ccd03bb Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 10:45:20 +0000 Subject: [PATCH 01/10] Improve error message and add tests for borrowck match handling --- .../borrow_check/error_reporting.rs | 30 ++++++---- src/test/ui/nll/match-cfg-fake-edges.rs | 58 +++++++++++++++++++ src/test/ui/nll/match-cfg-fake-edges.stderr | 34 +++++++++++ .../ui/nll/match-guards-partially-borrow.rs | 14 ++++- .../nll/match-guards-partially-borrow.stderr | 51 +++++++--------- 5 files changed, 144 insertions(+), 43 deletions(-) create mode 100644 src/test/ui/nll/match-cfg-fake-edges.rs create mode 100644 src/test/ui/nll/match-cfg-fake-edges.stderr diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 4d65862375a96..744dd61f4f593 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -1330,22 +1330,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let loan_span = loan_spans.args_or_use(); let tcx = self.infcx.tcx; - let mut err = if loan.kind == BorrowKind::Shallow { - tcx.cannot_mutate_in_match_guard( + if loan.kind == BorrowKind::Shallow { + let mut err = tcx.cannot_mutate_in_match_guard( span, loan_span, &self.describe_place(place).unwrap_or_else(|| "_".to_owned()), "assign", Origin::Mir, - ) - } else { - tcx.cannot_assign_to_borrowed( - span, - loan_span, - &self.describe_place(place).unwrap_or_else(|| "_".to_owned()), - Origin::Mir, - ) - }; + ); + loan_spans.var_span_label( + &mut err, + format!("borrow occurs due to use{}", loan_spans.describe()), + ); + + err.buffer(&mut self.errors_buffer); + + return; + } + + let mut err = tcx.cannot_assign_to_borrowed( + span, + loan_span, + &self.describe_place(place).unwrap_or_else(|| "_".to_owned()), + Origin::Mir, + ); loan_spans.var_span_label( &mut err, diff --git a/src/test/ui/nll/match-cfg-fake-edges.rs b/src/test/ui/nll/match-cfg-fake-edges.rs new file mode 100644 index 0000000000000..a3add8856dfa1 --- /dev/null +++ b/src/test/ui/nll/match-cfg-fake-edges.rs @@ -0,0 +1,58 @@ +// Test that we have enough false edges to avoid exposing the exact matching +// algorithm in borrow checking. + +#![feature(nll, bind_by_move_pattern_guards)] + +fn guard_always_precedes_arm(y: i32) { + let mut x; + // x should always be initialized, as the only way to reach the arm is + // through the guard. + match y { + 0 | 2 if { x = 2; true } => x, + _ => 2, + }; +} + +fn guard_may_be_skipped(y: i32) { + let x; + // Even though x *is* always initialized, we don't want to have borrowck + // results be based on whether patterns are exhaustive. + match y { + _ if { x = 2; true } => 1, + _ if { + x; //~ ERROR use of possibly uninitialized variable: `x` + false + } => 2, + _ => 3, + }; +} + +fn guard_may_be_taken(y: bool) { + let x = String::new(); + // Even though x *is* never moved before the use, we don't want to have + // borrowck results be based on whether patterns are disjoint. + match y { + false if { drop(x); true } => 1, + true => { + x; //~ ERROR use of moved value: `x` + 2 + } + false => 3, + }; +} + +fn all_previous_tests_may_be_done(y: &mut (bool, bool)) { + let r = &mut y.1; + // We don't actually test y.1 to select the second arm, but we don't want + // borrowck results to be based on the order we match patterns. + match y { + (false, true) => 1, //~ ERROR cannot use `y.1` because it was mutably borrowed + (true, _) => { + r; + 2 + } + (false, _) => 3, + }; +} + +fn main() {} diff --git a/src/test/ui/nll/match-cfg-fake-edges.stderr b/src/test/ui/nll/match-cfg-fake-edges.stderr new file mode 100644 index 0000000000000..a855b28a978dd --- /dev/null +++ b/src/test/ui/nll/match-cfg-fake-edges.stderr @@ -0,0 +1,34 @@ +error[E0381]: use of possibly uninitialized variable: `x` + --> $DIR/match-cfg-fake-edges.rs:23:13 + | +LL | x; //~ ERROR use of possibly uninitialized variable: `x` + | ^ use of possibly uninitialized `x` + +error[E0382]: use of moved value: `x` + --> $DIR/match-cfg-fake-edges.rs:37:13 + | +LL | let x = String::new(); + | - move occurs because `x` has type `std::string::String`, which does not implement the `Copy` trait +... +LL | false if { drop(x); true } => 1, + | - value moved here +LL | true => { +LL | x; //~ ERROR use of moved value: `x` + | ^ value used here after move + +error[E0503]: cannot use `y.1` because it was mutably borrowed + --> $DIR/match-cfg-fake-edges.rs:49:17 + | +LL | let r = &mut y.1; + | -------- borrow of `y.1` occurs here +... +LL | (false, true) => 1, //~ ERROR cannot use `y.1` because it was mutably borrowed + | ^^^^ use of borrowed `y.1` +LL | (true, _) => { +LL | r; + | - borrow later used here + +error: aborting due to 3 previous errors + +Some errors occurred: E0381, E0382, E0503. +For more information about an error, try `rustc --explain E0381`. diff --git a/src/test/ui/nll/match-guards-partially-borrow.rs b/src/test/ui/nll/match-guards-partially-borrow.rs index f359800812c87..cb8ea2c3d26bd 100644 --- a/src/test/ui/nll/match-guards-partially-borrow.rs +++ b/src/test/ui/nll/match-guards-partially-borrow.rs @@ -30,7 +30,7 @@ fn ok_indirect_mutation_in_guard(mut p: &bool) { fn mutation_invalidates_pattern_in_guard(mut q: bool) { match q { - // s doesn't match the pattern with the guard by the end of the guard. + // q doesn't match the pattern with the guard by the end of the guard. false if { q = true; //~ ERROR true @@ -41,7 +41,7 @@ fn mutation_invalidates_pattern_in_guard(mut q: bool) { fn mutation_invalidates_previous_pattern_in_guard(mut r: bool) { match r { - // s matches a previous pattern by the end of the guard. + // r matches a previous pattern by the end of the guard. true => (), _ if { r = true; //~ ERROR @@ -116,6 +116,16 @@ fn bad_mutation_in_guard4(mut w: (&mut bool,)) { } } +fn bad_mutation_in_guard5(mut t: bool) { + match t { + s if { + t = !t; //~ ERROR + false + } => (), // What value should `s` have in the arm? + _ => (), + } +} + fn bad_indirect_mutation_in_guard(mut y: &bool) { match *y { true => (), diff --git a/src/test/ui/nll/match-guards-partially-borrow.stderr b/src/test/ui/nll/match-guards-partially-borrow.stderr index 2cbfeb886b572..b6302b3a65d3c 100644 --- a/src/test/ui/nll/match-guards-partially-borrow.stderr +++ b/src/test/ui/nll/match-guards-partially-borrow.stderr @@ -6,9 +6,6 @@ LL | match q { ... LL | q = true; //~ ERROR | ^^^^^^^^ cannot assign -... -LL | _ => (), - | - borrow later used here error[E0510]: cannot assign `r` in match guard --> $DIR/match-guards-partially-borrow.rs:47:13 @@ -18,9 +15,6 @@ LL | match r { ... LL | r = true; //~ ERROR | ^^^^^^^^ cannot assign -... -LL | _ => (), - | - borrow later used here error[E0503]: cannot use `s` because it was mutably borrowed --> $DIR/match-guards-partially-borrow.rs:56:11 @@ -41,9 +35,6 @@ LL | match t { ... LL | t = true; //~ ERROR | ^^^^^^^^ cannot assign -... -LL | false => (), - | ----- borrow later used here error[E0506]: cannot assign to `u` because it is borrowed --> $DIR/match-guards-partially-borrow.rs:83:13 @@ -53,8 +44,8 @@ LL | match u { ... LL | u = true; //~ ERROR | ^^^^^^^^ assignment to borrowed `u` occurs here -... -LL | x => (), +LL | false +LL | } => (), | - borrow later used here error[E0510]: cannot mutably borrow `x.0` in match guard @@ -74,59 +65,59 @@ LL | match w { ... LL | *w.0 = true; //~ ERROR | ^^^^^^^^^^^ assignment to borrowed `*w.0` occurs here -... -LL | x => (), +LL | false +LL | } => (), + | - borrow later used here + +error[E0506]: cannot assign to `t` because it is borrowed + --> $DIR/match-guards-partially-borrow.rs:122:13 + | +LL | match t { + | - borrow of `t` occurs here +LL | s if { +LL | t = !t; //~ ERROR + | ^^^^^^ assignment to borrowed `t` occurs here +LL | false +LL | } => (), // What value should `s` have in the arm? | - borrow later used here error[E0510]: cannot assign `y` in match guard - --> $DIR/match-guards-partially-borrow.rs:123:13 + --> $DIR/match-guards-partially-borrow.rs:133:13 | LL | match *y { | -- value is immutable in match guard ... LL | y = &true; //~ ERROR | ^^^^^^^^^ cannot assign -... -LL | false => (), - | ----- borrow later used here error[E0510]: cannot assign `z` in match guard - --> $DIR/match-guards-partially-borrow.rs:134:13 + --> $DIR/match-guards-partially-borrow.rs:144:13 | LL | match z { | - value is immutable in match guard ... LL | z = &true; //~ ERROR | ^^^^^^^^^ cannot assign -... -LL | &false => (), - | ------ borrow later used here error[E0510]: cannot assign `a` in match guard - --> $DIR/match-guards-partially-borrow.rs:146:13 + --> $DIR/match-guards-partially-borrow.rs:156:13 | LL | match a { | - value is immutable in match guard ... LL | a = &true; //~ ERROR | ^^^^^^^^^ cannot assign -... -LL | false => (), - | ----- borrow later used here error[E0510]: cannot assign `b` in match guard - --> $DIR/match-guards-partially-borrow.rs:157:13 + --> $DIR/match-guards-partially-borrow.rs:167:13 | LL | match b { | - value is immutable in match guard ... LL | b = &true; //~ ERROR | ^^^^^^^^^ cannot assign -... -LL | &b => (), - | -- borrow later used here -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors Some errors occurred: E0503, E0506, E0510. For more information about an error, try `rustc --explain E0503`. From d51b5cdd82ec0e36212fc75b83652372980d56ca Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 10:56:55 +0000 Subject: [PATCH 02/10] Clean up MIR match lowering * Adjust fake borrows to only be live over guards. * Remove unused `slice_len_checked` field. * Split the methods on builder into those for matches and those for all kinds of pattern bindings. --- src/librustc_mir/build/matches/mod.rs | 846 ++++++++++++++---------- src/librustc_mir/build/matches/test.rs | 266 +++----- src/librustc_mir/build/matches/util.rs | 1 - src/test/mir-opt/issue-49232.rs | 36 +- src/test/mir-opt/match_false_edges.rs | 336 +++++----- src/test/mir-opt/match_test.rs | 59 +- src/test/mir-opt/remove_fake_borrows.rs | 98 ++- 7 files changed, 838 insertions(+), 804 deletions(-) diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 96d2c90345933..a287659ec8e60 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -1,7 +1,9 @@ -//! Code related to match expressions. These are sufficiently complex -//! to warrant their own module and submodules. :) This main module -//! includes the high-level algorithm, the submodules contain the -//! details. +//! Code related to match expressions. These are sufficiently complex to +//! warrant their own module and submodules. :) This main module includes the +//! high-level algorithm, the submodules contain the details. +//! +//! This also includes code for pattern bindings in `let` statements and +//! function parameters. use crate::build::scope::{CachedBlock, DropKind}; use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard, ValWithinGuard}; @@ -23,12 +25,78 @@ mod util; use std::convert::TryFrom; -/// ArmHasGuard is isomorphic to a boolean flag. It indicates whether -/// a match arm has a guard expression attached to it. -#[derive(Copy, Clone, Debug)] -pub(crate) struct ArmHasGuard(pub bool); - impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { + /// Generates MIR for a `match` expression. + /// + /// The MIR that we generate for a match looks like this. + /// + /// ```text + /// [ 0. Pre-match ] + /// | + /// [ 1. Evaluate Scrutinee] + /// [ (fake read of scrutinee) ] + /// | + /// [ 2. Decision tree -- check discriminants ] <--------+ + /// | | + /// | (once a specific arm is chosen) | + /// | | + /// [pre_binding_block] [otherwise_block] + /// | | + /// [ 3. Create "guard bindings" for arm ] | + /// [ (create fake borrows) ] | + /// | | + /// [ 4. Execute guard code ] | + /// [ (read fake borrows) ] --(guard is false)-----------+ + /// | + /// | (guard results in true) + /// | + /// [ 5. Create real bindings and execute arm ] + /// | + /// [ Exit match ] + /// ``` + /// + /// All of the different arms have been stacked on top of each other to + /// simplify the diagram. For an arm with no guard the blocks marked 3 and + /// 4 and the fake borrows are omitted. + /// + /// We generate MIR in the following steps: + /// + /// 1. Evaluate the scrutinee and add the fake read of it. + /// 2. Create the prebinding and otherwise blocks. + /// 3. Create the decision tree and record the places that we bind or test. + /// 4. Determine the fake borrows that are needed from the above places. + /// Create the required temporaries for them. + /// 5. Create everything else: Create everything else: the guards and the + /// arms. + /// + /// ## Fake Reads and borrows + /// + /// Match exhaustiveness checking is no able to handle the case where the + /// place being matched on is mutated in the guards. There is an AST check + /// that tries to stop this but it is buggy and overly restrictive. Instead + /// we add "fake borrows" to the guards that prevent any mutation of the + /// place being matched. There are a some subtleties: + /// + /// 1. Borrowing `*x` doesn't prevent assigning to `x`. If `x` is a shared + /// refence, the borrow isn't even tracked. As such we have to add fake + /// borrows of any prefixes of a place + /// 2. We don't want `match x { _ => (), }` to conflict with mutable + /// borrows of `x`, so we only add fake borrows for places which are + /// bound or tested by the match. + /// 3. We don't want the fake borrows to conflict with `ref mut` bindings, + /// so we lower `ref mut` bindings as two-phase borrows for the guard. + /// 4. The fake borrows may be of places in inactive variants, so it would + /// be UB to generate code for them. They therefore have to be removed + /// by a MIR pass run after borrow checking. + /// + /// ## False edges + /// + /// We don't want to have the exact structure of the decision tree be + /// visible through borrow checking. False edges ensure that the CFG as + /// seen by borrow checking doesn't encode this. False edges are added: + /// + /// * From each prebinding block to the next prebinding block. + /// * From each otherwise block to the next prebinding block. pub fn match_expr( &mut self, destination: &Place<'tcx>, @@ -38,6 +106,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { arms: Vec>, ) -> BlockAnd<()> { let tcx = self.hir.tcx(); + + // Step 1. Evaluate the scrutinee and add the fake read of it. + let discriminant_span = discriminant.span(); let discriminant_place = unpack!(block = self.as_place(block, discriminant)); @@ -66,26 +137,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ), }); - let mut arm_blocks = ArmBlocks { - blocks: arms.iter().map(|_| self.cfg.start_new_block()).collect(), - }; - - // Get the arm bodies and their scopes, while declaring bindings. - let arm_bodies: Vec<_> = arms.iter() - .map(|arm| { - // BUG: use arm lint level - let body = self.hir.mirror(arm.body.clone()); - let scope = self.declare_bindings( - None, - body.span, - LintLevel::Inherited, - &arm.patterns[..], - ArmHasGuard(arm.guard.is_some()), - Some((Some(&discriminant_place), discriminant_span)), - ); - (body, scope.unwrap_or(self.source_scope)) - }) - .collect(); + // Step 2. Create the otherwise and prebinding blocks. // create binding start block for link them by false edges let candidate_count = arms.iter().map(|c| c.patterns.len()).sum::(); @@ -93,70 +145,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { .map(|_| self.cfg.start_new_block()) .collect(); - let mut has_guard = false; - - // assemble a list of candidates: there is one candidate per - // pattern, which means there may be more than one candidate - // *per arm*. These candidates are kept sorted such that the - // highest priority candidate comes first in the list. - // (i.e., same order as in source) - - let candidates: Vec<_> = arms.iter() - .enumerate() - .flat_map(|(arm_index, arm)| { - arm.patterns - .iter() - .enumerate() - .map(move |(pat_index, pat)| (arm_index, pat_index, pat, arm.guard.clone())) - }) - .zip( - pre_binding_blocks - .iter() - .zip(pre_binding_blocks.iter().skip(1)), - ) - .map( - |( - (arm_index, pat_index, pattern, guard), - (pre_binding_block, next_candidate_pre_binding_block) - )| { - has_guard |= guard.is_some(); - - // One might ask: why not build up the match pair such that it - // matches via `borrowed_input_temp.deref()` instead of - // using the `discriminant_place` directly, as it is doing here? - // - // The basic answer is that if you do that, then you end up with - // accceses to a shared borrow of the input and that conflicts with - // any arms that look like e.g. - // - // match Some(&4) { - // ref mut foo => { - // ... /* mutate `foo` in arm body */ ... - // } - // } - // - // (Perhaps we could further revise the MIR - // construction here so that it only does a - // shared borrow at the outset and delays doing - // the mutable borrow until after the pattern is - // matched *and* the guard (if any) for the arm - // has been run.) - - Candidate { - span: pattern.span, - match_pairs: vec![MatchPair::new(discriminant_place.clone(), pattern)], - bindings: vec![], - ascriptions: vec![], - guard, - arm_index, - pat_index, - pre_binding_block: *pre_binding_block, - next_candidate_pre_binding_block: *next_candidate_pre_binding_block, - } - }, - ) - .collect(); - + // There's one move pre_binding block that there are candidates so that + // every candidate has a next prebinding_block. let outer_source_info = self.source_info(span); self.cfg.terminate( *pre_binding_blocks.last().unwrap(), @@ -164,27 +154,72 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { TerminatorKind::Unreachable, ); + let mut match_has_guard = false; + + let mut candidate_pre_binding_blocks = pre_binding_blocks.iter(); + let mut next_candidate_pre_binding_blocks = pre_binding_blocks.iter().skip(1); + + // Assemble a list of candidates: there is one candidate per pattern, + // which means there may be more than one candidate *per arm*. + let mut arm_candidates: Vec<_> = arms + .iter() + .map(|arm| { + let arm_has_guard = arm.guard.is_some(); + match_has_guard |= arm_has_guard; + let arm_candidates: Vec<_> = arm.patterns + .iter() + .zip(candidate_pre_binding_blocks.by_ref()) + .zip(next_candidate_pre_binding_blocks.by_ref()) + .map( + |((pattern, pre_binding_block), next_candidate_pre_binding_block)| { + Candidate { + span: pattern.span, + match_pairs: vec![ + MatchPair::new(discriminant_place.clone(), pattern), + ], + bindings: vec![], + ascriptions: vec![], + otherwise_block: if arm_has_guard { + Some(self.cfg.start_new_block()) + } else { + None + }, + pre_binding_block: *pre_binding_block, + next_candidate_pre_binding_block: + *next_candidate_pre_binding_block, + } + }, + ) + .collect(); + (arm, arm_candidates) + }) + .collect(); + + // Step 3. Create the decision tree and record the places that we bind or test. + // Maps a place to the kind of Fake borrow that we want to perform on // it: either Shallow or Shared, depending on whether the place is // bound in the match, or just switched on. // If there are no match guards then we don't need any fake borrows, // so don't track them. - let mut fake_borrows = if has_guard && tcx.generate_borrow_of_any_match_input() { + let mut fake_borrows = if match_has_guard && tcx.generate_borrow_of_any_match_input() { Some(FxHashMap::default()) } else { None }; - let pre_binding_blocks: Vec<_> = candidates - .iter() - .map(|cand| (cand.pre_binding_block, cand.span)) - .collect(); + // These candidates are kept sorted such that the highest priority + // candidate comes first in the list. (i.e., same order as in source) + // As we gnerate the decision tree, + let candidates = &mut arm_candidates + .iter_mut() + .flat_map(|(_, candidates)| candidates) + .collect::>(); // this will generate code to test discriminant_place and // branch to the appropriate arm block let otherwise = self.match_candidates( discriminant_span, - &mut arm_blocks, candidates, block, &mut fake_borrows, @@ -197,29 +232,59 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // // In that case, the inexhaustive tips of the decision tree // can't be reached - terminate them with an `unreachable`. - let source_info = self.source_info(span); - let mut otherwise = otherwise; otherwise.sort(); otherwise.dedup(); // variant switches can introduce duplicate target blocks for block in otherwise { self.cfg - .terminate(block, source_info, TerminatorKind::Unreachable); + .terminate(block, outer_source_info, TerminatorKind::Unreachable); } } - if let Some(fake_borrows) = fake_borrows { - self.add_fake_borrows(&pre_binding_blocks, fake_borrows, source_info, block); - } + // Step 4. Determine the fake borrows that are needed from the above + // places. Create the required temporaries for them. + + let fake_borrow_temps = if let Some(ref borrows) = fake_borrows { + self.calculate_fake_borrows(borrows, discriminant_span) + } else { + Vec::new() + }; + + // Step 5. Create everything else: the guards and the arms. // all the arm blocks will rejoin here let end_block = self.cfg.start_new_block(); let outer_source_info = self.source_info(span); - for (arm_index, (body, source_scope)) in arm_bodies.into_iter().enumerate() { - let mut arm_block = arm_blocks.blocks[arm_index]; - // Re-enter the source scope we created the bindings in. - self.source_scope = source_scope; + + for (arm, candidates) in arm_candidates { + let mut arm_block = self.cfg.start_new_block(); + + let body = self.hir.mirror(arm.body.clone()); + let scope = self.declare_bindings( + None, + body.span, + LintLevel::Inherited, + &arm.patterns[..], + ArmHasGuard(arm.guard.is_some()), + Some((Some(&discriminant_place), discriminant_span)), + ); + + for (pat_index, candidate) in candidates.into_iter().enumerate() { + self.bind_and_guard_matched_candidate( + candidate, + arm.guard.clone(), + arm_block, + &fake_borrow_temps, + discriminant_span, + pat_index, + ); + } + + if let Some(source_scope) = scope { + self.source_scope = source_scope; + } + unpack!(arm_block = self.into(destination, arm_block, body)); self.cfg.terminate( arm_block, @@ -227,6 +292,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { TerminatorKind::Goto { target: end_block }, ); } + self.source_scope = outer_source_info.scope; end_block.unit() @@ -359,11 +425,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { match_pairs: vec![MatchPair::new(initializer.clone(), &irrefutable_pat)], bindings: vec![], ascriptions: vec![], - guard: None, - // since we don't call `match_candidates`, next fields is unused - arm_index: 0, - pat_index: 0, + // since we don't call `match_candidates`, next fields are unused + otherwise_block: None, pre_binding_block: block, next_candidate_pre_binding_block: block, }; @@ -613,13 +677,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } -/// List of blocks for each arm (and potentially other metadata in the -/// future). -struct ArmBlocks { - blocks: Vec, -} - -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct Candidate<'pat, 'tcx: 'pat> { // span of the original pattern that gave rise to this candidate span: Span, @@ -630,21 +688,15 @@ pub struct Candidate<'pat, 'tcx: 'pat> { // ...these bindings established... bindings: Vec>, - // ...these types asserted... + // ...and these types asserted... ascriptions: Vec>, - // ...and the guard must be evaluated... - guard: Option>, - - // ...and then we branch to arm with this index. - arm_index: usize, + // ...and the guard must be evaluated, if false branch to Block... + otherwise_block: Option, // ...and the blocks for add false edges between candidates pre_binding_block: BasicBlock, next_candidate_pre_binding_block: BasicBlock, - - // This uniquely identifies this candidate *within* the arm. - pat_index: usize, } #[derive(Clone, Debug)] @@ -676,13 +728,6 @@ pub struct MatchPair<'pat, 'tcx: 'pat> { // ... must match this pattern. pattern: &'pat Pattern<'tcx>, - - // HACK(eddyb) This is used to toggle whether a Slice pattern - // has had its length checked. This is only necessary because - // the "rest" part of the pattern right now has type &[T] and - // as such, it requires an Rvalue::Slice to be generated. - // See RFC 495 / issue #23121 for the eventual (proper) solution. - slice_len_checked: bool, } #[derive(Clone, Debug, PartialEq)] @@ -722,6 +767,11 @@ pub struct Test<'tcx> { kind: TestKind<'tcx>, } +/// ArmHasGuard is isomorphic to a boolean flag. It indicates whether +/// a match arm has a guard expression attached to it. +#[derive(Copy, Clone, Debug)] +pub(crate) struct ArmHasGuard(pub bool); + /////////////////////////////////////////////////////////////////////////// // Main matching algorithm @@ -732,7 +782,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// candidates are sorted such that the first item in the list /// has the highest priority. When a candidate is found to match /// the value, we will generate a branch to the appropriate - /// block found in `arm_blocks`. + /// prebinding block. /// /// The return value is a list of "otherwise" blocks. These are /// points in execution where we found that *NONE* of the @@ -747,13 +797,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// list. This is important to keep the size of the generated code /// under control. See `test_candidates` for more details. /// - /// If `add_fake_borrows` is true, then places which need fake borrows + /// If `fake_borrows` is Some, then places which need fake borrows /// will be added to it. fn match_candidates<'pat>( &mut self, span: Span, - arm_blocks: &mut ArmBlocks, - mut candidates: Vec>, + candidates: &mut [&mut Candidate<'pat, 'tcx>], mut block: BasicBlock, fake_borrows: &mut Option, BorrowKind>>, ) -> Vec { @@ -762,17 +811,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { span, block, candidates ); - // Start by simplifying candidates. Once this process is - // complete, all the match pairs which remain require some - // form of test, whether it be a switch or pattern comparison. - for candidate in &mut candidates { + // Start by simplifying candidates. Once this process is complete, all + // the match pairs which remain require some form of test, whether it + // be a switch or pattern comparison. + for candidate in &mut *candidates { self.simplify_candidate(candidate); } - // The candidates are sorted by priority. Check to see - // whether the higher priority candidates (and hence at - // the front of the vec) have satisfied all their match - // pairs. + // The candidates are sorted by priority. Check to see whether the + // higher priority candidates (and hence at the front of the slice) + // have satisfied all their match pairs. let fully_matched = candidates .iter() .take_while(|c| c.match_pairs.is_empty()) @@ -781,87 +829,172 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { "match_candidates: {:?} candidates fully matched", fully_matched ); - let mut unmatched_candidates = candidates.split_off(fully_matched); + let (matched_candidates, unmatched_candidates) = candidates.split_at_mut(fully_matched); + + if !matched_candidates.is_empty() { + block = if let Some(last_otherwise_block) = self.select_matched_candidates( + matched_candidates, + block, + fake_borrows, + ) { + last_otherwise_block + } else { + // Any remaining candidates are unreachable. + if unmatched_candidates.is_empty() { + return Vec::new(); + } else { + self.cfg.start_new_block() + } + }; + } + + // If there are no candidates that still need testing, we're + // done. Since all matches are exhaustive, execution should + // never reach this point. + if unmatched_candidates.is_empty() { + return vec![block]; + } + + // Test candidates where possible. + let (otherwise, untested_candidates) = self.test_candidates( + span, + unmatched_candidates, + block, + fake_borrows, + ); + + // If the target candidates were exhaustive, then we are done. + // But for borrowck continue build decision tree. + if untested_candidates.is_empty() { + return otherwise; + } + + // Otherwise, let's process those remaining candidates. + let join_block = self.join_otherwise_blocks(span, otherwise); + self.match_candidates( + span, + untested_candidates, + join_block, + &mut None, + ) + } + + /// Link up matched candidates. For example, if we have something like + /// this: + /// + /// ... + /// Some(x) if cond => ... + /// Some(x) => ... + /// Some(x) if cond => ... + /// ... + /// + /// We generate real edges from: + /// * `block` to the prebinding_block of the first pattern, + /// * the otherwise block of the first pattern to the second pattern, + /// * the otherwise block of the third pattern to the a block with an + /// Unreachable terminator. + /// + /// As well as that we add fake edges from the otherwise blocks to the + /// prebinding block of the next candidate in the original set of + /// candidates. + fn select_matched_candidates( + &mut self, + matched_candidates: &mut [&mut Candidate<'_, 'tcx>], + block: BasicBlock, + fake_borrows: &mut Option, BorrowKind>>, + ) -> Option { + debug_assert!( + !matched_candidates.is_empty(), + "select_matched_candidates called with no candidates", + ); // Insert a *Shared* borrow of any places that are bound. if let Some(fake_borrows) = fake_borrows { for Binding { source, .. } - in candidates.iter().flat_map(|candidate| &candidate.bindings) + in matched_candidates.iter().flat_map(|candidate| &candidate.bindings) { fake_borrows.insert(source.clone(), BorrowKind::Shared); } } - let fully_matched_with_guard = candidates.iter().take_while(|c| c.guard.is_some()).count(); + let fully_matched_with_guard = matched_candidates + .iter() + .position(|c| c.otherwise_block.is_none()) + .unwrap_or(matched_candidates.len() - 1); - let unreachable_candidates = if fully_matched_with_guard + 1 < candidates.len() { - candidates.split_off(fully_matched_with_guard + 1) - } else { - vec![] - }; + let (reachable_candidates, unreachable_candidates) + = matched_candidates.split_at_mut(fully_matched_with_guard + 1); - for candidate in candidates { - // If so, apply any bindings, test the guard (if any), and - // branch to the arm. - if let Some(b) = self.bind_and_guard_matched_candidate(block, arm_blocks, candidate) { - block = b; - } else { - // if None is returned, then any remaining candidates - // are unreachable (at least not through this path). - // Link them with false edges. - debug!( - "match_candidates: add false edges for unreachable {:?} and unmatched {:?}", - unreachable_candidates, unmatched_candidates - ); - for candidate in unreachable_candidates { - let source_info = self.source_info(candidate.span); - let target = self.cfg.start_new_block(); - if let Some(otherwise) = - self.bind_and_guard_matched_candidate(target, arm_blocks, candidate) - { - self.cfg - .terminate(otherwise, source_info, TerminatorKind::Unreachable); - } - } + let first_candidate = &reachable_candidates[0]; - if unmatched_candidates.is_empty() { - return vec![]; + let candidate_source_info = self.source_info(first_candidate.span); + + self.cfg.terminate( + block, + candidate_source_info, + TerminatorKind::Goto { + target: first_candidate.pre_binding_block, + }, + ); + + for window in reachable_candidates.windows(2) { + if let [first_candidate, second_candidate] = window { + let source_info = self.source_info(first_candidate.span); + if let Some(otherwise_block) = first_candidate.otherwise_block { + self.cfg.terminate( + otherwise_block, + source_info, + TerminatorKind::FalseEdges { + real_target: second_candidate.pre_binding_block, + imaginary_targets: vec![ + first_candidate.next_candidate_pre_binding_block + ], + } + ) } else { - let target = self.cfg.start_new_block(); - return self.match_candidates( - span, - arm_blocks, - unmatched_candidates, - target, - &mut None, - ); + bug!("candidate other than the last has no guard"); } + } else { + bug!("<[_]>::windows returned incorrectly sized window"); } } - // If there are no candidates that still need testing, we're done. - // Since all matches are exhaustive, execution should never reach this point. - if unmatched_candidates.is_empty() { - return vec![block]; - } - // Test candidates where possible. - let (otherwise, tested_candidates) = - self.test_candidates(span, arm_blocks, &unmatched_candidates, block, fake_borrows); + // if None is returned, then + debug!("match_candidates: add false edges for unreachable {:?}", unreachable_candidates); + for candidate in unreachable_candidates { + if let Some(otherwise) = candidate.otherwise_block { + let source_info = self.source_info(candidate.span); + let unreachable = self.cfg.start_new_block(); + self.cfg.terminate( + otherwise, + source_info, + TerminatorKind::FalseEdges { + real_target: unreachable, + imaginary_targets: vec![candidate.next_candidate_pre_binding_block], + } + ); + self.cfg.terminate(unreachable, source_info, TerminatorKind::Unreachable); + } + } - // If the target candidates were exhaustive, then we are done. - // But for borrowck continue build decision tree. + let last_candidate = reachable_candidates.last().unwrap(); - // If all candidates were sorted into `target_candidates` somewhere, then - // the initial set was inexhaustive. - let untested_candidates = unmatched_candidates.split_off(tested_candidates); - if untested_candidates.len() == 0 { - return otherwise; + if let Some(otherwise) = last_candidate.otherwise_block { + let source_info = self.source_info(last_candidate.span); + let block = self.cfg.start_new_block(); + self.cfg.terminate( + otherwise, + source_info, + TerminatorKind::FalseEdges { + real_target: block, + imaginary_targets: vec![last_candidate.next_candidate_pre_binding_block] + } + ); + Some(block) + } else { + None } - - // Otherwise, let's process those remaining candidates. - let join_block = self.join_otherwise_blocks(span, otherwise); - self.match_candidates(span, arm_blocks, untested_candidates, join_block, &mut None) } fn join_otherwise_blocks(&mut self, span: Span, mut otherwise: Vec) -> BasicBlock { @@ -995,17 +1128,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// In addition to avoiding exponential-time blowups, this algorithm /// also has nice property that each guard and arm is only generated /// once. - fn test_candidates<'pat>( + fn test_candidates<'pat, 'b, 'c>( &mut self, span: Span, - arm_blocks: &mut ArmBlocks, - candidates: &[Candidate<'pat, 'tcx>], + mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>], block: BasicBlock, fake_borrows: &mut Option, BorrowKind>>, - ) -> (Vec, usize) { + ) -> (Vec, &'b mut [&'c mut Candidate<'pat, 'tcx>]) { // extract the match-pair from the highest priority candidate let match_pair = &candidates.first().unwrap().match_pairs[0]; let mut test = self.test(match_pair); + let match_place = match_pair.place.clone(); // most of the time, the test to perform is simply a function // of the main candidate; but for a test like SwitchInt, we @@ -1019,7 +1152,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } => { for candidate in candidates.iter() { if !self.add_cases_to_switch( - &match_pair.place, + &match_place, candidate, switch_ty, options, @@ -1034,7 +1167,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ref mut variants, } => { for candidate in candidates.iter() { - if !self.add_variants_to_switch(&match_pair.place, candidate, variants) { + if !self.add_variants_to_switch(&match_place, candidate, variants) { break; } } @@ -1044,7 +1177,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Insert a Shallow borrow of any places that is switched on. fake_borrows.as_mut().map(|fb| { - fb.entry(match_pair.place.clone()).or_insert(BorrowKind::Shallow) + fb.entry(match_place.clone()).or_insert(BorrowKind::Shallow) }); // perform the test, branching to one of N blocks. For each of @@ -1055,25 +1188,29 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { "match_candidates: test={:?} match_pair={:?}", test, match_pair ); - let target_blocks = self.perform_test(block, &match_pair.place, &test); - let mut target_candidates = vec![vec![]; target_blocks.len()]; + let target_blocks = self.perform_test(block, &match_place, &test); + let mut target_candidates: Vec>> = vec![]; + target_candidates.resize_with(target_blocks.len(), Default::default); + + let total_candidate_count = candidates.len(); // Sort the candidates into the appropriate vector in // `target_candidates`. Note that at some point we may // encounter a candidate where the test is not relevant; at // that point, we stop sorting. - let tested_candidates = candidates - .iter() - .take_while(|c| { - self.sort_candidate(&match_pair.place, &test, c, &mut target_candidates) - }) - .count(); - assert!(tested_candidates > 0); // at least the last candidate ought to be tested - debug!("tested_candidates: {}", tested_candidates); - debug!( - "untested_candidates: {}", - candidates.len() - tested_candidates - ); + while let Some(candidate) = candidates.first_mut() { + if let Some(idx) = self.sort_candidate(&match_place, &test, candidate) { + let (candidate, rest) = candidates.split_first_mut().unwrap(); + target_candidates[idx].push(candidate); + candidates = rest; + } else { + break; + } + } + // at least the first candidate ought to be tested + assert!(total_candidate_count > candidates.len()); + debug!("tested_candidates: {}", total_candidate_count - candidates.len()); + debug!("untested_candidates: {}", candidates.len()); // For each outcome of test, process the candidates that still // apply. Collect a list of blocks where control flow will @@ -1082,59 +1219,99 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let otherwise: Vec<_> = target_blocks .into_iter() .zip(target_candidates) - .flat_map(|(target_block, target_candidates)| { + .flat_map(|(target_block, mut target_candidates)| { self.match_candidates( span, - arm_blocks, - target_candidates, + &mut *target_candidates, target_block, fake_borrows, ) }) .collect(); - (otherwise, tested_candidates) + (otherwise, candidates) + } + + // Determine the fake borrows that are needed to ensure that the place + // will evaluate to the same thing until an arm has been chosen. + fn calculate_fake_borrows<'b>( + &mut self, + fake_borrows: &'b FxHashMap, BorrowKind>, + temp_span: Span, + ) -> Vec<(&'b Place<'tcx>, BorrowKind, Local)> { + let tcx = self.hir.tcx(); + + debug!("add_fake_borrows fake_borrows = {:?}", fake_borrows); + + let mut all_fake_borrows = Vec::with_capacity(fake_borrows.len()); + + // Insert a Shallow borrow of the prefixes of any fake borrows. + for (place, borrow_kind) in fake_borrows + { + let mut prefix_cursor = place; + while let Place::Projection(box Projection { base, elem }) = prefix_cursor { + if let ProjectionElem::Deref = elem { + // Insert a shallow borrow after a deref. For other + // projections the borrow of prefix_cursor will + // conflict with any mutation of base. + all_fake_borrows.push((base, BorrowKind::Shallow)); + } + prefix_cursor = base; + } + + all_fake_borrows.push((place, *borrow_kind)); + } + + // Deduplicate and ensure a deterministic order. + all_fake_borrows.sort(); + all_fake_borrows.dedup(); + + debug!("add_fake_borrows all_fake_borrows = {:?}", all_fake_borrows); + + all_fake_borrows.into_iter().map(|(matched_place, borrow_kind)| { + let fake_borrow_deref_ty = matched_place.ty(&self.local_decls, tcx).to_ty(tcx); + let fake_borrow_ty = tcx.mk_imm_ref(tcx.types.re_erased, fake_borrow_deref_ty); + let fake_borrow_temp = self.local_decls.push( + LocalDecl::new_temp(fake_borrow_ty, temp_span) + ); + + (matched_place, borrow_kind, fake_borrow_temp) + }).collect() } +} + +/////////////////////////////////////////////////////////////////////////// +// Pattern binding - used for `let` and function parameters as well. +impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// Initializes each of the bindings from the candidate by - /// moving/copying/ref'ing the source as appropriate. Tests the - /// guard, if any, and then branches to the arm. Returns the block - /// for the case where the guard fails. + /// moving/copying/ref'ing the source as appropriate. Tests the guard, if + /// any, and then branches to the arm. Returns the block for the case where + /// the guard fails. /// - /// Note: we check earlier that if there is a guard, there cannot - /// be move bindings. This isn't really important for the - /// self-consistency of this fn, but the reason for it should be - /// clear: after we've done the assignments, if there were move - /// bindings, further tests would be a use-after-move (which would - /// in turn be detected by the borrowck code that runs on the - /// MIR). + /// Note: we check earlier that if there is a guard, there cannot be move + /// bindings (unless feature(bind_by_move_pattern_guards) is used). This + /// isn't really important for the self-consistency of this fn, but the + /// reason for it should be clear: after we've done the assignments, if + /// there were move bindings, further tests would be a use-after-move. + /// bind_by_move_pattern_guards avoids this by only moving the binding once + /// the guard has evaluated to true (see below). fn bind_and_guard_matched_candidate<'pat>( &mut self, - mut block: BasicBlock, - arm_blocks: &mut ArmBlocks, candidate: Candidate<'pat, 'tcx>, - ) -> Option { - debug!( - "bind_and_guard_matched_candidate(block={:?}, candidate={:?})", - block, candidate - ); + guard: Option>, + arm_block: BasicBlock, + fake_borrows: &Vec<(&Place<'tcx>, BorrowKind, Local)>, + discriminant_span: Span, + pat_index: usize, + ) { + debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate); debug_assert!(candidate.match_pairs.is_empty()); - self.ascribe_types(block, &candidate.ascriptions); - - let arm_block = arm_blocks.blocks[candidate.arm_index]; let candidate_source_info = self.source_info(candidate.span); - self.cfg.terminate( - block, - candidate_source_info, - TerminatorKind::Goto { - target: candidate.pre_binding_block, - }, - ); - - block = self.cfg.start_new_block(); + let mut block = self.cfg.start_new_block(); self.cfg.terminate( candidate.pre_binding_block, candidate_source_info, @@ -1143,6 +1320,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { imaginary_targets: vec![candidate.next_candidate_pre_binding_block], }, ); + self.ascribe_types(block, &candidate.ascriptions); // rust-lang/rust#27282: The `autoref` business deserves some // explanation here. @@ -1226,14 +1404,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // match input itself; it is up to us to create a place // holding a `&` or `&mut` that we can then borrow). - let autoref = self.hir - .tcx() - .all_pat_vars_are_implicit_refs_within_guards(); - if let Some(guard) = candidate.guard { + let tcx = self.hir.tcx(); + let autoref = tcx.all_pat_vars_are_implicit_refs_within_guards(); + if let Some(guard) = guard { if autoref { self.bind_matched_candidate_for_guard( block, - candidate.pat_index, + pat_index, &candidate.bindings, ); let guard_frame = GuardFrame { @@ -1249,12 +1426,29 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.bind_matched_candidate_for_arm_body(block, &candidate.bindings); } + let re_erased = tcx.types.re_erased; + let discriminant_source_info = self.source_info(discriminant_span); + for &(place, borrow_kind, temp) in fake_borrows { + let borrow = Rvalue::Ref( + re_erased, + borrow_kind, + place.clone(), + ); + self.cfg.push_assign( + block, + discriminant_source_info, + &Place::Local(temp), + borrow, + ); + } + // the block to branch to if the guard fails; if there is no // guard, this block is simply unreachable let guard = match guard { Guard::If(e) => self.hir.mirror(e), }; let source_info = self.source_info(guard.span); + let guard_end = self.source_info(tcx.sess.source_map().end_point(guard.span)); let cond = unpack!(block = self.as_local_operand(block, guard)); if autoref { let guard_frame = self.guard_context.pop().unwrap(); @@ -1264,7 +1458,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ); } - let false_edge_block = self.cfg.start_new_block(); + for &(_, _, temp) in fake_borrows { + self.cfg.push(block, Statement { + source_info: guard_end, + kind: StatementKind::FakeRead( + FakeReadCause::ForMatchGuard, + Place::Local(temp), + ), + }); + } // We want to ensure that the matched candidates are bound // after we have confirmed this candidate *and* any @@ -1296,7 +1498,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.cfg.terminate( block, source_info, - TerminatorKind::if_(self.hir.tcx(), cond, post_guard_block, false_edge_block), + TerminatorKind::if_( + self.hir.tcx(), + cond, + post_guard_block, + candidate.otherwise_block.unwrap() + ), ); if autoref { @@ -1308,19 +1515,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { source_info, TerminatorKind::Goto { target: arm_block }, ); - - let otherwise = self.cfg.start_new_block(); - - self.cfg.terminate( - false_edge_block, - source_info, - TerminatorKind::FalseEdges { - real_target: otherwise, - imaginary_targets: vec![candidate.next_candidate_pre_binding_block], - }, - ); - Some(otherwise) } else { + assert!(candidate.otherwise_block.is_none()); // (Here, it is not too early to bind the matched // candidate on `block`, because there is no guard result // that we have to inspect before we bind them.) @@ -1330,7 +1526,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { candidate_source_info, TerminatorKind::Goto { target: arm_block }, ); - None } } @@ -1397,8 +1592,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let ref_for_guard = self.storage_live_binding(block, binding.var_id, binding.span, RefWithinGuard); // Question: Why schedule drops if bindings are all - // shared-&'s? Answer: Because schedule_drop_for_binding - // also emits StorageDead's for those locals. + // shared-&'s? + // Answer: Because schedule_drop_for_binding also emits + // StorageDead's for those locals. self.schedule_drop_for_binding(binding.var_id, binding.span, RefWithinGuard); match binding.binding_mode { BindingMode::ByValue => { @@ -1585,86 +1781,4 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { debug!("declare_binding: vars={:?}", locals); self.var_indices.insert(var_id, locals); } - - // Determine the fake borrows that are needed to ensure that the place - // will evaluate to the same thing until an arm has been chosen. - fn add_fake_borrows<'pat>( - &mut self, - pre_binding_blocks: &[(BasicBlock, Span)], - fake_borrows: FxHashMap, BorrowKind>, - source_info: SourceInfo, - start_block: BasicBlock, - ) { - let tcx = self.hir.tcx(); - - debug!("add_fake_borrows pre_binding_blocks = {:?}, fake_borrows = {:?}", - pre_binding_blocks, fake_borrows); - - let mut all_fake_borrows = Vec::with_capacity(fake_borrows.len()); - - // Insert a Shallow borrow of the prefixes of any fake borrows. - for (place, borrow_kind) in fake_borrows - { - { - let mut prefix_cursor = &place; - while let Place::Projection(box Projection { base, elem }) = prefix_cursor { - if let ProjectionElem::Deref = elem { - // Insert a shallow borrow after a deref. For other - // projections the borrow of prefix_cursor will - // conflict with any mutation of base. - all_fake_borrows.push((base.clone(), BorrowKind::Shallow)); - } - prefix_cursor = base; - } - } - - all_fake_borrows.push((place, borrow_kind)); - } - - // Deduplicate and ensure a deterministic order. - all_fake_borrows.sort(); - all_fake_borrows.dedup(); - - debug!("add_fake_borrows all_fake_borrows = {:?}", all_fake_borrows); - - // Add fake borrows to the start of the match and reads of them before - // the start of each arm. - let mut borrowed_input_temps = Vec::with_capacity(all_fake_borrows.len()); - - for (matched_place, borrow_kind) in all_fake_borrows { - let borrowed_input = - Rvalue::Ref(tcx.types.re_erased, borrow_kind, matched_place.clone()); - let borrowed_input_ty = borrowed_input.ty(&self.local_decls, tcx); - let borrowed_input_temp = self.temp(borrowed_input_ty, source_info.span); - self.cfg.push_assign( - start_block, - source_info, - &borrowed_input_temp, - borrowed_input - ); - borrowed_input_temps.push(borrowed_input_temp); - } - - // FIXME: This could be a lot of reads (#fake borrows * #patterns). - // The false edges that we currently generate would allow us to only do - // this on the last Candidate, but it's possible that there might not be - // so many false edges in the future, so we read for all Candidates for - // now. - // Another option would be to make our own block and add our own false - // edges to it. - if tcx.emit_read_for_match() { - for &(pre_binding_block, span) in pre_binding_blocks { - let pattern_source_info = self.source_info(span); - for temp in &borrowed_input_temps { - self.cfg.push(pre_binding_block, Statement { - source_info: pattern_source_info, - kind: StatementKind::FakeRead( - FakeReadCause::ForMatchGuard, - temp.clone(), - ), - }); - } - } - } - } } diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index a41d3895d6d3c..72b92444dece9 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -69,8 +69,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } - PatternKind::Slice { ref prefix, ref slice, ref suffix } - if !match_pair.slice_len_checked => { + PatternKind::Slice { ref prefix, ref slice, ref suffix } => { let len = prefix.len() + suffix.len(); let op = if slice.is_some() { BinOp::Ge @@ -85,7 +84,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { PatternKind::AscribeUserType { .. } | PatternKind::Array { .. } | - PatternKind::Slice { .. } | PatternKind::Wild | PatternKind::Binding { .. } | PatternKind::Leaf { .. } | @@ -433,59 +431,49 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { target_block } - /// Given that we are performing `test` against `test_place`, - /// this job sorts out what the status of `candidate` will be - /// after the test. The `resulting_candidates` vector stores, for - /// each possible outcome of `test`, a vector of the candidates - /// that will result. This fn should add a (possibly modified) - /// clone of candidate into `resulting_candidates` wherever - /// appropriate. + /// Given that we are performing `test` against `test_place`, this job + /// sorts out what the status of `candidate` will be after the test. See + /// `test_candidates` for the usage of this function. The returned index is + /// the index that this candiate should be placed in the + /// `target_candidates` vec. The candidate may be modified to update its + /// `match_pairs`. /// - /// So, for example, if this candidate is `x @ Some(P0)` and the - /// Tests is a variant test, then we would add `(x as Option).0 @ - /// P0` to the `resulting_candidates` entry corresponding to the - /// variant `Some`. + /// So, for example, if this candidate is `x @ Some(P0)` and the `Test` is + /// a variant test, then we would modify the candidate to be `(x as + /// Option).0 @ P0` and return the index corresponding to the variant + /// `Some`. /// - /// However, in some cases, the test may just not be relevant to - /// candidate. For example, suppose we are testing whether `foo.x == 22`, - /// but in one match arm we have `Foo { x: _, ... }`... in that case, - /// the test for what value `x` has has no particular relevance - /// to this candidate. In such cases, this function just returns false - /// without doing anything. This is used by the overall `match_candidates` - /// algorithm to structure the match as a whole. See `match_candidates` for - /// more details. + /// However, in some cases, the test may just not be relevant to candidate. + /// For example, suppose we are testing whether `foo.x == 22`, but in one + /// match arm we have `Foo { x: _, ... }`... in that case, the test for + /// what value `x` has has no particular relevance to this candidate. In + /// such cases, this function just returns None without doing anything. + /// This is used by the overall `match_candidates` algorithm to structure + /// the match as a whole. See `match_candidates` for more details. /// - /// FIXME(#29623). In some cases, we have some tricky choices to - /// make. for example, if we are testing that `x == 22`, but the - /// candidate is `x @ 13..55`, what should we do? In the event - /// that the test is true, we know that the candidate applies, but - /// in the event of false, we don't know that it *doesn't* - /// apply. For now, we return false, indicate that the test does - /// not apply to this candidate, but it might be we can get + /// FIXME(#29623). In some cases, we have some tricky choices to make. for + /// example, if we are testing that `x == 22`, but the candidate is `x @ + /// 13..55`, what should we do? In the event that the test is true, we know + /// that the candidate applies, but in the event of false, we don't know + /// that it *doesn't* apply. For now, we return false, indicate that the + /// test does not apply to this candidate, but it might be we can get /// tighter match code if we do something a bit different. - pub fn sort_candidate<'pat>(&mut self, - test_place: &Place<'tcx>, - test: &Test<'tcx>, - candidate: &Candidate<'pat, 'tcx>, - resulting_candidates: &mut [Vec>]) - -> bool { + pub fn sort_candidate<'pat, 'cand>( + &mut self, + test_place: &Place<'tcx>, + test: &Test<'tcx>, + candidate: &mut Candidate<'pat, 'tcx>, + ) -> Option { // Find the match_pair for this place (if any). At present, // afaik, there can be at most one. (In the future, if we // adopted a more general `@` operator, there might be more // than one, but it'd be very unusual to have two sides that // both require tests; you'd expect one side to be simplified // away.) - let tested_match_pair = candidate.match_pairs.iter() - .enumerate() - .find(|&(_, mp)| mp.place == *test_place); - let (match_pair_index, match_pair) = match tested_match_pair { - Some(pair) => pair, - None => { - // We are not testing this place. Therefore, this - // candidate applies to ALL outcomes. - return false; - } - }; + let (match_pair_index, match_pair) = candidate.match_pairs + .iter() + .enumerate() + .find(|&(_, mp)| mp.place == *test_place)?; match (&test.kind, &*match_pair.pattern.kind) { // If we are performing a variant switch, then this @@ -493,17 +481,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { (&TestKind::Switch { adt_def: tested_adt_def, .. }, &PatternKind::Variant { adt_def, variant_index, ref subpatterns, .. }) => { assert_eq!(adt_def, tested_adt_def); - let new_candidate = - self.candidate_after_variant_switch(match_pair_index, - adt_def, - variant_index, - subpatterns, - candidate); - resulting_candidates[variant_index.as_usize()].push(new_candidate); - true + self.candidate_after_variant_switch(match_pair_index, + adt_def, + variant_index, + subpatterns, + candidate); + Some(variant_index.as_usize()) } - (&TestKind::Switch { .. }, _) => false, + (&TestKind::Switch { .. }, _) => None, // If we are performing a switch over integers, then this informs integer // equality, but nothing else. @@ -514,10 +500,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { &PatternKind::Constant { ref value }) if is_switch_ty(match_pair.pattern.ty) => { let index = indices[value]; - let new_candidate = self.candidate_without_match_pair(match_pair_index, - candidate); - resulting_candidates[index].push(new_candidate); - true + self.candidate_without_match_pair(match_pair_index, candidate); + Some(index) } (&TestKind::SwitchInt { switch_ty: _, ref options, ref indices }, @@ -530,14 +514,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // No switch values are contained in the pattern range, // so the pattern can be matched only if this test fails. let otherwise = options.len(); - resulting_candidates[otherwise].push(candidate.clone()); - true + Some(otherwise) } else { - false + None } } - (&TestKind::SwitchInt { .. }, _) => false, + (&TestKind::SwitchInt { .. }, _) => None, (&TestKind::Len { len: test_len, op: BinOp::Eq }, &PatternKind::Slice { ref prefix, ref slice, ref suffix }) => { @@ -546,32 +529,28 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { (Ordering::Equal, &None) => { // on true, min_len = len = $actual_length, // on false, len != $actual_length - resulting_candidates[0].push( - self.candidate_after_slice_test(match_pair_index, - candidate, - prefix, - slice.as_ref(), - suffix) - ); - true + self.candidate_after_slice_test(match_pair_index, + candidate, + prefix, + slice.as_ref(), + suffix); + Some(0) } (Ordering::Less, _) => { // test_len < pat_len. If $actual_len = test_len, // then $actual_len < pat_len and we don't have // enough elements. - resulting_candidates[1].push(candidate.clone()); - true + Some(1) } (Ordering::Equal, &Some(_)) | (Ordering::Greater, &Some(_)) => { // This can match both if $actual_len = test_len >= pat_len, // and if $actual_len > test_len. We can't advance. - false + None } (Ordering::Greater, &None) => { // test_len != pat_len, so if $actual_len = test_len, then // $actual_len != pat_len. - resulting_candidates[1].push(candidate.clone()); - true + Some(1) } } } @@ -584,32 +563,28 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { (Ordering::Equal, &Some(_)) => { // $actual_len >= test_len = pat_len, // so we can match. - resulting_candidates[0].push( - self.candidate_after_slice_test(match_pair_index, - candidate, - prefix, - slice.as_ref(), - suffix) - ); - true + self.candidate_after_slice_test(match_pair_index, + candidate, + prefix, + slice.as_ref(), + suffix); + Some(0) } (Ordering::Less, _) | (Ordering::Equal, &None) => { // test_len <= pat_len. If $actual_len < test_len, // then it is also < pat_len, so the test passing is // necessary (but insufficient). - resulting_candidates[0].push(candidate.clone()); - true + Some(0) } (Ordering::Greater, &None) => { // test_len > pat_len. If $actual_len >= test_len > pat_len, // then we know we won't have a match. - resulting_candidates[1].push(candidate.clone()); - true + Some(1) } (Ordering::Greater, &Some(_)) => { // test_len < pat_len, and is therefore less // strict. This can still go both ways. - false + None } } } @@ -617,12 +592,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { (&TestKind::Range(test), &PatternKind::Range(pat)) => { if test == pat { - resulting_candidates[0] - .push(self.candidate_without_match_pair( - match_pair_index, - candidate, - )); - return true; + self.candidate_without_match_pair( + match_pair_index, + candidate, + ); + return Some(0); } let no_overlap = (|| { @@ -649,10 +623,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if no_overlap == Some(true) { // Testing range does not overlap with pattern range, // so the pattern can be matched only if this test fails. - resulting_candidates[1].push(candidate.clone()); - true + Some(1) } else { - false + None } } @@ -660,15 +633,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if self.const_range_contains(range, value) == Some(false) { // `value` is not contained in the testing range, // so `value` can be matched only if this test fails. - resulting_candidates[1].push(candidate.clone()); - true + Some(1) } else { - false + None } } - (&TestKind::Range { .. }, _) => false, - + (&TestKind::Range { .. }, _) => None, (&TestKind::Eq { .. }, _) | (&TestKind::Len { .. }, _) => { @@ -677,73 +648,53 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // FIXME(#29623) we can be more clever here let pattern_test = self.test(&match_pair); if pattern_test.kind == test.kind { - let new_candidate = self.candidate_without_match_pair(match_pair_index, - candidate); - resulting_candidates[0].push(new_candidate); - true + self.candidate_without_match_pair(match_pair_index, candidate); + Some(0) } else { - false + None } } } } - fn candidate_without_match_pair<'pat>(&mut self, - match_pair_index: usize, - candidate: &Candidate<'pat, 'tcx>) - -> Candidate<'pat, 'tcx> { - let other_match_pairs = - candidate.match_pairs.iter() - .enumerate() - .filter(|&(index, _)| index != match_pair_index) - .map(|(_, mp)| mp.clone()) - .collect(); - Candidate { - span: candidate.span, - match_pairs: other_match_pairs, - bindings: candidate.bindings.clone(), - ascriptions: candidate.ascriptions.clone(), - guard: candidate.guard.clone(), - arm_index: candidate.arm_index, - pat_index: candidate.pat_index, - pre_binding_block: candidate.pre_binding_block, - next_candidate_pre_binding_block: candidate.next_candidate_pre_binding_block, - } + fn candidate_without_match_pair( + &mut self, + match_pair_index: usize, + candidate: &mut Candidate<'_, 'tcx>, + ) { + candidate.match_pairs.remove(match_pair_index); } fn candidate_after_slice_test<'pat>(&mut self, match_pair_index: usize, - candidate: &Candidate<'pat, 'tcx>, + candidate: &mut Candidate<'pat, 'tcx>, prefix: &'pat [Pattern<'tcx>], opt_slice: Option<&'pat Pattern<'tcx>>, - suffix: &'pat [Pattern<'tcx>]) - -> Candidate<'pat, 'tcx> { - let mut new_candidate = - self.candidate_without_match_pair(match_pair_index, candidate); + suffix: &'pat [Pattern<'tcx>]) { + let removed_place = candidate.match_pairs.remove(match_pair_index).place; self.prefix_slice_suffix( - &mut new_candidate.match_pairs, - &candidate.match_pairs[match_pair_index].place, + &mut candidate.match_pairs, + &removed_place, prefix, opt_slice, suffix); - - new_candidate } - fn candidate_after_variant_switch<'pat>(&mut self, - match_pair_index: usize, - adt_def: &'tcx ty::AdtDef, - variant_index: VariantIdx, - subpatterns: &'pat [FieldPattern<'tcx>], - candidate: &Candidate<'pat, 'tcx>) - -> Candidate<'pat, 'tcx> { - let match_pair = &candidate.match_pairs[match_pair_index]; + fn candidate_after_variant_switch<'pat>( + &mut self, + match_pair_index: usize, + adt_def: &'tcx ty::AdtDef, + variant_index: VariantIdx, + subpatterns: &'pat [FieldPattern<'tcx>], + candidate: &mut Candidate<'pat, 'tcx>, + ) { + let match_pair = candidate.match_pairs.remove(match_pair_index); // So, if we have a match-pattern like `x @ Enum::Variant(P1, P2)`, // we want to create a set of derived match-patterns like // `(x as Variant).0 @ P1` and `(x as Variant).1 @ P1`. let elem = ProjectionElem::Downcast(adt_def, variant_index); - let downcast_place = match_pair.place.clone().elem(elem); // `(x as Variant)` + let downcast_place = match_pair.place.elem(elem); // `(x as Variant)` let consequent_match_pairs = subpatterns.iter() .map(|subpattern| { @@ -754,26 +705,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { MatchPair::new(place, &subpattern.pattern) }); - // In addition, we need all the other match pairs from the old candidate. - let other_match_pairs = - candidate.match_pairs.iter() - .enumerate() - .filter(|&(index, _)| index != match_pair_index) - .map(|(_, mp)| mp.clone()); - - let all_match_pairs = consequent_match_pairs.chain(other_match_pairs).collect(); - - Candidate { - span: candidate.span, - match_pairs: all_match_pairs, - bindings: candidate.bindings.clone(), - ascriptions: candidate.ascriptions.clone(), - guard: candidate.guard.clone(), - arm_index: candidate.arm_index, - pat_index: candidate.pat_index, - pre_binding_block: candidate.pre_binding_block, - next_candidate_pre_binding_block: candidate.next_candidate_pre_binding_block, - } + candidate.match_pairs.extend(consequent_match_pairs); } fn error_simplifyable<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> ! { diff --git a/src/librustc_mir/build/matches/util.rs b/src/librustc_mir/build/matches/util.rs index b583b184a4103..3b90ff7884f01 100644 --- a/src/librustc_mir/build/matches/util.rs +++ b/src/librustc_mir/build/matches/util.rs @@ -72,7 +72,6 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { MatchPair { place, pattern, - slice_len_checked: false, } } } diff --git a/src/test/mir-opt/issue-49232.rs b/src/test/mir-opt/issue-49232.rs index 8417c2cfd137d..5b4ec483277ac 100644 --- a/src/test/mir-opt/issue-49232.rs +++ b/src/test/mir-opt/issue-49232.rs @@ -41,46 +41,46 @@ fn main() { // StorageLive(_3); // _3 = const true; // FakeRead(ForMatchedPlace, _3); -// switchInt(_3) -> [false: bb11, otherwise: bb10]; +// switchInt(_3) -> [false: bb9, otherwise: bb8]; // } // bb4: { // resume; // } // bb5: { -// _2 = const 4i32; -// goto -> bb14; +// falseEdges -> [real: bb12, imaginary: bb6]; // } // bb6: { -// _0 = (); -// goto -> bb15; +// falseEdges -> [real: bb14, imaginary: bb7]; // } // bb7: { -// falseEdges -> [real: bb12, imaginary: bb8]; +// unreachable; // } // bb8: { -// falseEdges -> [real: bb13, imaginary: bb9]; +// goto -> bb6; // } // bb9: { -// unreachable; +// goto -> bb5; // } // bb10: { -// goto -> bb8; +// FakeRead(ForLet, _2); +// StorageDead(_3); +// StorageLive(_6); +// _6 = &_2; +// _5 = const std::mem::drop(move _6) -> [return: bb19, unwind: bb4]; // } // bb11: { -// goto -> bb7; +// _2 = const 4i32; +// goto -> bb10; // } // bb12: { -// goto -> bb5; +// goto -> bb11; // } // bb13: { -// goto -> bb6; +// _0 = (); +// goto -> bb15; // } // bb14: { -// FakeRead(ForLet, _2); -// StorageDead(_3); -// StorageLive(_6); -// _6 = &_2; -// _5 = const std::mem::drop(move _6) -> [return: bb19, unwind: bb4]; +// goto -> bb13; // } // bb15: { // StorageDead(_3); @@ -96,7 +96,7 @@ fn main() { // } // bb18: { // StorageDead(_4); -// goto -> bb14; +// goto -> bb10; // } // bb19: { // StorageDead(_6); diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index b512172de64ee..9faecf5b7af46 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -44,72 +44,68 @@ fn main() { // ... // _2 = std::option::Option::Some(const 42i32,); // FakeRead(ForMatchedPlace, _2); -// _7 = discriminant(_2); -// _9 = &shallow (promoted[2]: std::option::Option); -// _10 = &(((promoted[1]: std::option::Option) as Some).0: i32); -// switchInt(move _7) -> [0isize: bb5, 1isize: bb3, otherwise: bb7]; +// _3 = discriminant(_2); +// switchInt(move _3) -> [0isize: bb4, 1isize: bb2, otherwise: bb7]; // } // bb1: { // resume; // } -// bb2: { // arm1 -// _1 = (const 3i32, const 3i32); -// goto -> bb13; +// bb2: { +// falseEdges -> [real: bb9, imaginary: bb3]; //pre_binding1 // } -// bb3: { // binding3(empty) and arm3 -// FakeRead(ForMatchGuard, _9); -// FakeRead(ForMatchGuard, _10); -// falseEdges -> [real: bb8, imaginary: bb4]; //pre_binding1 +// bb3: { +// falseEdges -> [real: bb12, imaginary: bb4]; //pre_binding2 // } // bb4: { -// FakeRead(ForMatchGuard, _9); -// FakeRead(ForMatchGuard, _10); -// falseEdges -> [real: bb12, imaginary: bb5]; //pre_binding2 +// falseEdges -> [real: bb13, imaginary: bb5]; //pre_binding3 // } // bb5: { -// FakeRead(ForMatchGuard, _9); -// FakeRead(ForMatchGuard, _10); -// falseEdges -> [real: bb2, imaginary: bb6]; //pre_binding3 -// } -// bb6: { // unreachable; // } +// bb6: { // to pre_binding2 +// falseEdges -> [real: bb3, imaginary: bb3]; +// } // bb7: { // unreachable; // } -// bb8: { // binding1 and guard -// StorageLive(_5); -// _5 = &(((promoted[0]: std::option::Option) as Some).0: i32); -// StorageLive(_8); -// _8 = const guard() -> [return: bb9, unwind: bb1]; -// } -// bb9: { -// switchInt(move _8) -> [false: bb10, otherwise: bb11]; +// bb8: { +// ... +// return; // } -// bb10: { // to pre_binding2 -// falseEdges -> [real: bb4, imaginary: bb4]; +// bb9: { // binding1 and guard +// StorageLive(_8); +// _8 = &(((promoted[2]: std::option::Option) as Some).0: i32); +// _4 = &shallow (promoted[1]: std::option::Option); +// _5 = &(((promoted[0]: std::option::Option) as Some).0: i32); +// StorageLive(_9); +// _9 = const guard() -> [return: bb10, unwind: bb1]; // } -// bb11: { // bindingNoLandingPads.before.mir2 and arm2 -// StorageLive(_3); -// _3 = ((_2 as Some).0: i32); -// StorageLive(_11); -// _11 = _3; -// _1 = (const 1i32, move _11); -// StorageDead(_11); -// goto -> bb13; +// bb10: { +// FakeRead(ForMatchGuard, _4); +// FakeRead(ForMatchGuard, _5); +// switchInt(move _9) -> [false: bb6, otherwise: bb11]; // } -// bb12: { +// bb11: { // StorageLive(_6); // _6 = ((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = _6; +// _1 = (const 1i32, move _10); +// StorageDead(_10); +// goto -> bb8; +// } +// bb12: { +// StorageLive(_11); +// _11 = ((_2 as Some).0: i32); // StorageLive(_12); -// _12 = _6; -// _1 = (const 2i32, move_12); +// _12 = _11; +// _1 = (const 2i32, move _12); // StorageDead(_12); -// goto -> bb13; +// goto -> bb8; // } // bb13: { -// ... -// return; +// _1 = (const 3i32, const 3i32); +// goto -> bb8; // } // END rustc.full_tested_match.QualifyAndPromoteConstants.after.mir // @@ -118,164 +114,158 @@ fn main() { // ... // _2 = std::option::Option::Some(const 42i32,); // FakeRead(ForMatchedPlace, _2); -// _7 = discriminant(_2); -// _9 = &shallow _2; -// _10 = &((_2 as Some).0: i32); -// switchInt(move _7) -> [0isize: bb4, 1isize: bb3, otherwise: bb7]; +// _3 = discriminant(_2); +// switchInt(move _3) -> [0isize: bb3, 1isize: bb2, otherwise: bb7]; // } // bb1: { // resume; // } -// bb2: { // arm2 -// _1 = (const 3i32, const 3i32); -// goto -> bb13; +// bb2: { +// falseEdges -> [real: bb9, imaginary: bb3]; // } // bb3: { -// FakeRead(ForMatchGuard, _9); -// FakeRead(ForMatchGuard, _10); -// falseEdges -> [real: bb8, imaginary: bb4]; //pre_binding1 +// falseEdges -> [real: bb12, imaginary: bb4]; // } // bb4: { -// FakeRead(ForMatchGuard, _9); -// FakeRead(ForMatchGuard, _10); -// falseEdges -> [real: bb2, imaginary: bb5]; //pre_binding2 +// falseEdges -> [real: bb13, imaginary: bb5]; // } // bb5: { -// FakeRead(ForMatchGuard, _9); -// FakeRead(ForMatchGuard, _10); -// falseEdges -> [real: bb12, imaginary: bb6]; //pre_binding3 -// } -// bb6: { // unreachable; // } +// bb6: { // to pre_binding3 (can skip 2 since this is `Some`) +// falseEdges -> [real: bb4, imaginary: bb3]; +// } // bb7: { // unreachable; // } -// bb8: { // binding1 and guard -// StorageLive(_5); -// _5 = &((_2 as Some).0: i32); -// StorageLive(_8); -// _8 = const guard() -> [return: bb9, unwind: bb1]; +// bb8: { +// ... +// return; // } -// bb9: { // end of guard -// switchInt(move _8) -> [false: bb10, otherwise: bb11]; +// bb9: { // binding1 and guard +// StorageLive(_8); +// _8 = &((_2 as Some).0: i32); +// _4 = &shallow _2; +// _5 = &((_2 as Some).0: i32); +// StorageLive(_9); +// _9 = const guard() -> [return: bb10, unwind: bb1]; // } -// bb10: { // to pre_binding3 (can skip 2 since this is `Some`) -// falseEdges -> [real: bb5, imaginary: bb4]; +// bb10: { // end of guard +// FakeRead(ForMatchGuard, _4); +// FakeRead(ForMatchGuard, _5); +// switchInt(move _9) -> [false: bb6, otherwise: bb11]; // } // bb11: { // arm1 -// StorageLive(_3); -// _3 = ((_2 as Some).0: i32); -// StorageLive(_11); -// _11 = _3; -// _1 = (const 1i32, move _11); -// StorageDead(_11); -// goto -> bb13; -// } -// bb12: { // binding3 and arm3 // StorageLive(_6); // _6 = ((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = _6; +// _1 = (const 1i32, move _10); +// StorageDead(_10); +// goto -> bb8; +// } +// bb12: { // arm2 +// _1 = (const 3i32, const 3i32); +// goto -> bb8; +// } +// bb13: { // binding3 and arm3 +// StorageLive(_11); +// _11 = ((_2 as Some).0: i32); // StorageLive(_12); -// _12 = _6; +// _12 = _11; // _1 = (const 2i32, move _12); // StorageDead(_12); -// goto -> bb13; -// } -// bb13: { -// ... -// return; +// goto -> bb8; // } // END rustc.full_tested_match2.QualifyAndPromoteConstants.before.mir // // START rustc.main.QualifyAndPromoteConstants.before.mir // bb0: { // ... -// _2 = std::option::Option::Some(const 1i32,); -// FakeRead(ForMatchedPlace, _2); -// _11 = discriminant(_2); -// _16 = &shallow _2; -// _17 = &((_2 as Some).0: i32); -// switchInt(move _11) -> [1isize: bb2, otherwise: bb3]; -// } -// bb1: { -// resume; -// } -// bb2: { -// FakeRead(ForMatchGuard, _16); -// FakeRead(ForMatchGuard, _17); -// falseEdges -> [real: bb7, imaginary: bb3]; //pre_binding1 -// } -// bb3: { -// FakeRead(ForMatchGuard, _16); -// FakeRead(ForMatchGuard, _17); -// falseEdges -> [real: bb11, imaginary: bb4]; //pre_binding2 -// } -// bb4: { -// FakeRead(ForMatchGuard, _16); -// FakeRead(ForMatchGuard, _17); -// falseEdges -> [real: bb12, imaginary: bb5]; //pre_binding3 -// } -// bb5: { -// FakeRead(ForMatchGuard, _16); -// FakeRead(ForMatchGuard, _17); -// falseEdges -> [real: bb16, imaginary: bb6]; //pre_binding4 -// } -// bb6: { -// unreachable; -// } -// bb7: { // binding1: Some(w) if guard() -// StorageLive(_5); -// _5 = &((_2 as Some).0: i32); -// StorageLive(_12); -// _12 = const guard() -> [return: bb8, unwind: bb1]; -// } -// bb8: { //end of guard -// switchInt(move _12) -> [false: bb9, otherwise: bb10]; -// } -// bb9: { // to pre_binding2 -// falseEdges -> [real: bb3, imaginary: bb3]; -// } -// bb10: { // set up bindings for arm1 -// StorageLive(_3); -// _3 = ((_2 as Some).0: i32); -// _1 = const 1i32; -// goto -> bb17; -// } -// bb11: { // binding2 & arm2 -// StorageLive(_6); -// _6 = _2; -// _1 = const 2i32; -// goto -> bb17; -// } -// bb12: { // binding3: Some(y) if guard2(y) -// StorageLive(_9); -// _9 = &((_2 as Some).0: i32); -// StorageLive(_14); -// StorageLive(_15); -// _15 = (*_9); -// _14 = const guard2(move _15) -> [return: bb13, unwind: bb1]; -// } -// bb13: { // end of guard2 -// StorageDead(_15); -// switchInt(move _14) -> [false: bb14, otherwise: bb15]; -// } -// bb14: { // to pre_binding4 -// falseEdges -> [real: bb5, imaginary: bb5]; -// } -// bb15: { // set up bindings for arm3 -// StorageLive(_7); -// _7 = ((_2 as Some).0: i32); -// _1 = const 3i32; -// goto -> bb17; -// } -// bb16: { // binding4 & arm4 -// StorageLive(_10); -// _10 = _2; -// _1 = const 4i32; -// goto -> bb17; -// } -// bb17: { -// ... -// return; -// } +// _2 = std::option::Option::Some(const 1i32,); +// FakeRead(ForMatchedPlace, _2); +// _3 = discriminant(_2); +// switchInt(move _3) -> [1isize: bb2, otherwise: bb3]; +// } +// bb1: { +// resume; +// } +// bb2: { +// falseEdges -> [real: bb10, imaginary: bb3]; //pre_binding1 +// } +// bb3: { +// falseEdges -> [real: bb13, imaginary: bb4]; //pre_binding2 +// } +// bb4: { +// falseEdges -> [real: bb14, imaginary: bb5]; //pre_binding3 +// } +// bb5: { +// falseEdges -> [real: bb17, imaginary: bb6]; //pre_binding4 +// } +// bb6: { +// unreachable; +// } +// bb7: { // to pre_binding2 +// falseEdges -> [real: bb3, imaginary: bb3]; +// } +// bb8: { // to pre_binding4 +// falseEdges -> [real: bb5, imaginary: bb5]; +// } +// bb9: { +// ... +// return; +// } +// bb10: { // binding1: Some(w) if guard() +// StorageLive(_9); +// _9 = &((_2 as Some).0: i32); +// _5 = &shallow _2; +// _6 = &((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = const guard() -> [return: bb11, unwind: bb1]; +// } +// bb11: { //end of guard +// FakeRead(ForMatchGuard, _5); +// FakeRead(ForMatchGuard, _6); +// switchInt(move _10) -> [false: bb7, otherwise: bb12]; +// } +// bb12: { // set up bindings for arm1 +// StorageLive(_7); +// _7 = ((_2 as Some).0: i32); +// _1 = const 1i32; +// goto -> bb9; +// } +// bb13: { // binding2 & arm2 +// StorageLive(_11); +// _11 = _2; +// _1 = const 2i32; +// goto -> bb9; +// } +// bb14: { // binding3: Some(y) if guard2(y) +// StorageLive(_14); +// _14 = &((_2 as Some).0: i32); +// _5 = &shallow _2; +// _6 = &((_2 as Some).0: i32); +// StorageLive(_15); +// StorageLive(_16); +// _16 = (*_14); +// _15 = const guard2(move _16) -> [return: bb15, unwind: bb1]; +// } +// bb15: { // end of guard2 +// StorageDead(_16); +// FakeRead(ForMatchGuard, _5); +// FakeRead(ForMatchGuard, _6); +// switchInt(move _15) -> [false: bb8, otherwise: bb16]; +// } +// bb16: { // binding4 & arm4 +// StorageLive(_12); +// _12 = ((_2 as Some).0: i32); +// _1 = const 3i32; +// goto -> bb9; +// } +// bb17: { +// StorageLive(_17); +// _17 = _2; +// _1 = const 4i32; +// goto -> bb9; +// } // END rustc.main.QualifyAndPromoteConstants.before.mir diff --git a/src/test/mir-opt/match_test.rs b/src/test/mir-opt/match_test.rs index 9bfb728e13461..835a5ce1200b9 100644 --- a/src/test/mir-opt/match_test.rs +++ b/src/test/mir-opt/match_test.rs @@ -20,66 +20,67 @@ fn main() { // START rustc.main.SimplifyCfg-initial.after.mir // bb0: { // ... -// _4 = Le(const 0i32, _1); -// switchInt(move _4) -> [false: bb10, otherwise: bb11]; +// switchInt(move _4) -> [false: bb7, otherwise: bb8]; // } // bb1: { -// _3 = const 0i32; -// goto -> bb16; +// falseEdges -> [real: bb13, imaginary: bb2]; // } // bb2: { -// _3 = const 1i32; -// goto -> bb16; +// falseEdges -> [real: bb14, imaginary: bb3]; // } // bb3: { -// _3 = const 2i32; -// goto -> bb16; +// falseEdges -> [real: bb15, imaginary: bb4]; // } // bb4: { -// _3 = const 3i32; -// goto -> bb16; +// falseEdges -> [real: bb16, imaginary: bb5]; // } // bb5: { -// falseEdges -> [real: bb12, imaginary: bb6]; +// unreachable; // } // bb6: { -// falseEdges -> [real: bb2, imaginary: bb7]; +// falseEdges -> [real: bb4, imaginary: bb2]; // } // bb7: { -// falseEdges -> [real: bb3, imaginary: bb8]; +// _6 = Le(const 10i32, _1); +// switchInt(move _6) -> [false: bb9, otherwise: bb10]; // } // bb8: { -// falseEdges -> [real: bb4, imaginary: bb9]; +// _5 = Lt(_1, const 10i32); +// switchInt(move _5) -> [false: bb7, otherwise: bb1]; // } // bb9: { -// unreachable; +// switchInt(_1) -> [-1i32: bb3, otherwise: bb4]; // } // bb10: { -// _7 = Le(const 10i32, _1); -// switchInt(move _7) -> [false: bb14, otherwise: bb15]; +// _7 = Le(_1, const 20i32); +// switchInt(move _7) -> [false: bb9, otherwise: bb2]; // } // bb11: { -// _5 = Lt(_1, const 10i32); -// switchInt(move _5) -> [false: bb10, otherwise: bb5]; +// StorageDead(_8); +// _0 = (); +// StorageDead(_2); +// StorageDead(_1); +// return; // } // bb12: { -// StorageLive(_6); -// _6 = _2; -// switchInt(move _6) -> [false: bb13, otherwise: bb1]; +// _3 = const 0i32; +// goto -> bb11; // } // bb13: { -// falseEdges -> [real: bb8, imaginary: bb6]; +// StorageLive(_8); +// _8 = _2; +// switchInt(move _8) -> [false: bb6, otherwise: bb12]; // } // bb14: { -// switchInt(_1) -> [-1i32: bb7, otherwise: bb8]; +// _3 = const 1i32; +// goto -> bb11; // } // bb15: { -// _8 = Le(_1, const 20i32); -// switchInt(move _8) -> [false: bb14, otherwise: bb6]; +// _3 = const 2i32; +// goto -> bb11; // } // bb16: { -// StorageDead(_6); -// ... -// return; +// _3 = const 3i32; +// goto -> bb11; // } // END rustc.main.SimplifyCfg-initial.after.mir diff --git a/src/test/mir-opt/remove_fake_borrows.rs b/src/test/mir-opt/remove_fake_borrows.rs index 8411fba02e977..fab4d28a936ca 100644 --- a/src/test/mir-opt/remove_fake_borrows.rs +++ b/src/test/mir-opt/remove_fake_borrows.rs @@ -4,15 +4,15 @@ #![feature(nll)] -fn match_guard(x: Option<&&i32>) -> i32 { +fn match_guard(x: Option<&&i32>, c: bool) -> i32 { match x { - Some(0) if true => 0, + Some(0) if c => 0, _ => 1, } } fn main() { - match_guard(None); + match_guard(None, true); } // END RUST SOURCE @@ -20,49 +20,48 @@ fn main() { // START rustc.match_guard.CleanFakeReadsAndBorrows.before.mir // bb0: { // FakeRead(ForMatchedPlace, _1); -// _2 = discriminant(_1); -// _3 = &shallow _1; -// _4 = &shallow ((_1 as Some).0: &' &' i32); -// _5 = &shallow (*((_1 as Some).0: &' &' i32)); -// _6 = &shallow (*(*((_1 as Some).0: &' &' i32))); -// switchInt(move _2) -> [1isize: bb6, otherwise: bb4]; +// _3 = discriminant(_1); +// switchInt(move _3) -> [1isize: bb5, otherwise: bb2]; // } // bb1: { -// _0 = const 0i32; -// goto -> bb9; +// goto -> bb8; // } // bb2: { -// _0 = const 1i32; // goto -> bb9; // } // bb3: { -// FakeRead(ForMatchGuard, _3); -// FakeRead(ForMatchGuard, _4); -// FakeRead(ForMatchGuard, _5); -// FakeRead(ForMatchGuard, _6); -// goto -> bb7; +// unreachable; // } // bb4: { -// FakeRead(ForMatchGuard, _3); -// FakeRead(ForMatchGuard, _4); -// FakeRead(ForMatchGuard, _5); -// FakeRead(ForMatchGuard, _6); // goto -> bb2; // } // bb5: { -// unreachable; +// switchInt((*(*((_1 as Some).0: &' &' i32)))) -> [0i32: bb1, otherwise: bb2]; // } // bb6: { -// switchInt((*(*((_1 as Some).0: &' &' i32)))) -> [0i32: bb3, otherwise: bb4]; +// StorageDead(_8); +// return; // } // bb7: { -// goto -> bb1; +// _0 = const 0i32; +// goto -> bb6; // } // bb8: { -// goto -> bb4; +// _4 = &shallow _1; +// _5 = &shallow ((_1 as Some).0: &' &' i32); +// _6 = &shallow (*((_1 as Some).0: &' &' i32)); +// _7 = &shallow (*(*((_1 as Some).0: &' &' i32))); +// StorageLive(_8); +// _8 = _2; +// FakeRead(ForMatchGuard, _4); +// FakeRead(ForMatchGuard, _5); +// FakeRead(ForMatchGuard, _6); +// FakeRead(ForMatchGuard, _7); +// switchInt(move _8) -> [false: bb4, otherwise: bb7]; // } // bb9: { -// return; +// _0 = const 1i32; +// goto -> bb6; // } // bb10: { // resume; @@ -72,51 +71,50 @@ fn main() { // START rustc.match_guard.CleanFakeReadsAndBorrows.after.mir // bb0: { // nop; -// _2 = discriminant(_1); -// nop; -// nop; -// nop; -// nop; -// switchInt(move _2) -> [1isize: bb6, otherwise: bb4]; +// _3 = discriminant(_1); +// switchInt(move _3) -> [1isize: bb5, otherwise: bb2]; // } // bb1: { -// _0 = const 0i32; -// goto -> bb9; +// goto -> bb8; // } // bb2: { -// _0 = const 1i32; // goto -> bb9; // } // bb3: { -// nop; -// nop; -// nop; -// nop; -// goto -> bb7; +// unreachable; // } // bb4: { -// nop; -// nop; -// nop; -// nop; // goto -> bb2; // } // bb5: { -// unreachable; +// switchInt((*(*((_1 as Some).0: &' &' i32)))) -> [0i32: bb1, otherwise: bb2]; // } // bb6: { -// switchInt((*(*((_1 as Some).0: &' &' i32)))) -> [0i32: bb3, otherwise: bb4]; +// StorageDead(_8); +// return; // } // bb7: { -// goto -> bb1; +// _0 = const 0i32; +// goto -> bb6; // } // bb8: { -// goto -> bb4; +// nop; +// nop; +// nop; +// nop; +// StorageLive(_8); +// _8 = _2; +// nop; +// nop; +// nop; +// nop; +// switchInt(move _8) -> [false: bb4, otherwise: bb7]; // } // bb9: { -// return; +// _0 = const 1i32; +// goto -> bb6; // } // bb10: { // resume; -// } +// } // END rustc.match_guard.CleanFakeReadsAndBorrows.after.mir From 5407fbdef83b552ac5ee35c9577319c59d851467 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 14:55:10 +0000 Subject: [PATCH 03/10] Match discriminant -> scrutinee --- src/librustc_mir/build/expr/into.rs | 4 +-- src/librustc_mir/build/matches/mod.rs | 36 +++++++++++++-------------- src/librustc_mir/hair/cx/expr.rs | 2 +- src/librustc_mir/hair/mod.rs | 2 +- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 05231bc7b3f16..07db67a6ae00f 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -53,8 +53,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ExprKind::Block { body: ast_block } => { this.ast_block(destination, block, ast_block, source_info) } - ExprKind::Match { discriminant, arms } => { - this.match_expr(destination, expr_span, block, discriminant, arms) + ExprKind::Match { scrutinee, arms } => { + this.match_expr(destination, expr_span, block, scrutinee, arms) } ExprKind::NeverToAny { source } => { let source = this.hir.mirror(source); diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index a287659ec8e60..ae1de50dabfdc 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -33,7 +33,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// ```text /// [ 0. Pre-match ] /// | - /// [ 1. Evaluate Scrutinee] + /// [ 1. Evaluate Scrutinee (expression being matched on) ] /// [ (fake read of scrutinee) ] /// | /// [ 2. Decision tree -- check discriminants ] <--------+ @@ -102,17 +102,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { destination: &Place<'tcx>, span: Span, mut block: BasicBlock, - discriminant: ExprRef<'tcx>, + scrutinee: ExprRef<'tcx>, arms: Vec>, ) -> BlockAnd<()> { let tcx = self.hir.tcx(); // Step 1. Evaluate the scrutinee and add the fake read of it. - let discriminant_span = discriminant.span(); - let discriminant_place = unpack!(block = self.as_place(block, discriminant)); + let scrutinee_span = scrutinee.span(); + let scrutinee_place = unpack!(block = self.as_place(block, scrutinee)); - // Matching on a `discriminant_place` with an uninhabited type doesn't + // Matching on a `scrutinee_place` with an uninhabited type doesn't // generate any memory reads by itself, and so if the place "expression" // contains unsafe operations like raw pointer dereferences or union // field projections, we wouldn't know to require an `unsafe` block @@ -120,20 +120,20 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // See issue #47412 for this hole being discovered in the wild. // // HACK(eddyb) Work around the above issue by adding a dummy inspection - // of `discriminant_place`, specifically by applying `ReadForMatch`. + // of `scrutinee_place`, specifically by applying `ReadForMatch`. // - // NOTE: ReadForMatch also checks that the discriminant is initialized. + // NOTE: ReadForMatch also checks that the scrutinee is initialized. // This is currently needed to not allow matching on an uninitialized, // uninhabited value. If we get never patterns, those will check that // the place is initialized, and so this read would only be used to // check safety. - let source_info = self.source_info(discriminant_span); + let source_info = self.source_info(scrutinee_span); self.cfg.push(block, Statement { source_info, kind: StatementKind::FakeRead( FakeReadCause::ForMatchedPlace, - discriminant_place.clone(), + scrutinee_place.clone(), ), }); @@ -175,7 +175,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { Candidate { span: pattern.span, match_pairs: vec![ - MatchPair::new(discriminant_place.clone(), pattern), + MatchPair::new(scrutinee_place.clone(), pattern), ], bindings: vec![], ascriptions: vec![], @@ -216,10 +216,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { .flat_map(|(_, candidates)| candidates) .collect::>(); - // this will generate code to test discriminant_place and + // this will generate code to test scrutinee_place and // branch to the appropriate arm block let otherwise = self.match_candidates( - discriminant_span, + scrutinee_span, candidates, block, &mut fake_borrows, @@ -245,7 +245,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // places. Create the required temporaries for them. let fake_borrow_temps = if let Some(ref borrows) = fake_borrows { - self.calculate_fake_borrows(borrows, discriminant_span) + self.calculate_fake_borrows(borrows, scrutinee_span) } else { Vec::new() }; @@ -267,7 +267,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { LintLevel::Inherited, &arm.patterns[..], ArmHasGuard(arm.guard.is_some()), - Some((Some(&discriminant_place), discriminant_span)), + Some((Some(&scrutinee_place), scrutinee_span)), ); for (pat_index, candidate) in candidates.into_iter().enumerate() { @@ -276,7 +276,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { arm.guard.clone(), arm_block, &fake_borrow_temps, - discriminant_span, + scrutinee_span, pat_index, ); } @@ -1302,7 +1302,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { guard: Option>, arm_block: BasicBlock, fake_borrows: &Vec<(&Place<'tcx>, BorrowKind, Local)>, - discriminant_span: Span, + scrutinee_span: Span, pat_index: usize, ) { debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate); @@ -1427,7 +1427,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } let re_erased = tcx.types.re_erased; - let discriminant_source_info = self.source_info(discriminant_span); + let scrutinee_source_info = self.source_info(scrutinee_span); for &(place, borrow_kind, temp) in fake_borrows { let borrow = Rvalue::Ref( re_erased, @@ -1436,7 +1436,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ); self.cfg.push_assign( block, - discriminant_source_info, + scrutinee_source_info, &Place::Local(temp), borrow, ); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 10d04a80d7341..3c38f870b04b8 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -611,7 +611,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, } hir::ExprKind::Match(ref discr, ref arms, _) => { ExprKind::Match { - discriminant: discr.to_ref(), + scrutinee: discr.to_ref(), arms: arms.iter().map(|a| convert_arm(cx, a)).collect(), } } diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index e615b009cf370..bce0a0dd0a8bd 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -203,7 +203,7 @@ pub enum ExprKind<'tcx> { body: ExprRef<'tcx>, }, Match { - discriminant: ExprRef<'tcx>, + scrutinee: ExprRef<'tcx>, arms: Vec>, }, Block { From 2c840ae18dc53d55192269adee39d77a8652b2f7 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 16:38:12 +0000 Subject: [PATCH 04/10] Use normal mutable borrows in MIR match lowering --- src/librustc/ich/impls_mir.rs | 7 +- src/librustc/mir/mod.rs | 13 +- src/librustc_mir/borrow_check/mod.rs | 4 +- .../borrow_check/nll/invalidation.rs | 13 +- src/librustc_mir/build/block.rs | 6 +- src/librustc_mir/build/matches/mod.rs | 217 ++++++++---------- src/librustc_mir/build/mod.rs | 32 +-- src/test/mir-opt/match_false_edges.rs | 124 +++++----- .../ui/nll/match-guards-partially-borrow.rs | 57 +++-- .../nll/match-guards-partially-borrow.stderr | 62 ++--- 10 files changed, 227 insertions(+), 308 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index 51fc78ffc8669..ddc091b718706 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -196,7 +196,12 @@ impl_stable_hash_for!(impl<'gcx> for enum mir::StatementKind<'gcx> [ mir::Statem }); impl_stable_hash_for!(enum mir::RetagKind { FnEntry, TwoPhase, Raw, Default }); -impl_stable_hash_for!(enum mir::FakeReadCause { ForMatchGuard, ForMatchedPlace, ForLet }); +impl_stable_hash_for!(enum mir::FakeReadCause { + ForMatchGuard, + ForMatchedPlace, + ForGuardBinding, + ForLet +}); impl<'a, 'gcx> HashStable> for mir::Place<'gcx> { fn hash_stable(&self, diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 3513d652b5346..6c72a7c31591e 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1823,17 +1823,22 @@ pub enum RetagKind { /// The `FakeReadCause` describes the type of pattern why a `FakeRead` statement exists. #[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)] pub enum FakeReadCause { - /// Inject a fake read of the borrowed input at the start of each arm's - /// pattern testing code. + /// Inject a fake read of the borrowed input at the end of each guards + /// code. /// - /// This should ensure that you cannot change the variant for an enum - /// while you are in the midst of matching on it. + /// This should ensure that you cannot change the variant for an enum while + /// you are in the midst of matching on it. ForMatchGuard, /// `let x: !; match x {}` doesn't generate any read of x so we need to /// generate a read of x to check that it is initialized and safe. ForMatchedPlace, + /// A fake read of the RefWithinGuard version of a bind-by-value variable + /// in a match guard to ensure that it's value hasn't change by the time + /// we create the OutsideGuard version. + ForGuardBinding, + /// Officially, the semantics of /// /// `let pattern = ;` diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index f7d079c5494ef..1e9dab5016f8d 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -998,7 +998,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) - | (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) => { + | (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) + | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique) + | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => { Control::Continue } diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 3255899c86cb3..9c06767762108 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -78,13 +78,8 @@ impl<'cx, 'tcx, 'gcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx, 'gcx> { JustWrite ); } - StatementKind::FakeRead(_, ref place) => { - self.access_place( - ContextKind::FakeRead.new(location), - place, - (Deep, Read(ReadKind::Borrow(BorrowKind::Shared))), - LocalMutationIsAllowed::No, - ); + StatementKind::FakeRead(_, _) => { + // Only relavent for initialized/liveness/safety checks. } StatementKind::SetDiscriminant { ref place, @@ -438,7 +433,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> { } (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) - | (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => { + | (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) + | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique) + | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => { // Reads/reservations don't invalidate shared or shallow borrows } diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 7d93e131a6ca9..627fd7d2e1667 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -6,8 +6,6 @@ use rustc::mir::*; use rustc::hir; use syntax_pos::Span; -use std::slice; - impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { pub fn ast_block(&mut self, destination: &Place<'tcx>, @@ -125,7 +123,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { None, remainder_span, lint_level, - slice::from_ref(&pattern), + &pattern, ArmHasGuard(false), Some((None, initializer_span)), ); @@ -138,7 +136,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { })); } else { scope = this.declare_bindings( - None, remainder_span, lint_level, slice::from_ref(&pattern), + None, remainder_span, lint_level, &pattern, ArmHasGuard(false), None); debug!("ast_block_stmts: pattern={:?}", pattern); diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index ae1de50dabfdc..07c179ded593f 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -6,7 +6,7 @@ //! function parameters. use crate::build::scope::{CachedBlock, DropKind}; -use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard, ValWithinGuard}; +use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::build::{GuardFrame, GuardFrameLocal, LocalsForNode}; use crate::hair::{self, *}; @@ -14,7 +14,7 @@ use rustc::mir::*; use rustc::ty::{self, CanonicalUserTypeAnnotation, Ty}; use rustc::ty::layout::VariantIdx; use rustc_data_structures::bit_set::BitSet; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use syntax::ast::{Name, NodeId}; use syntax_pos::Span; @@ -71,7 +71,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// /// ## Fake Reads and borrows /// - /// Match exhaustiveness checking is no able to handle the case where the + /// Match exhaustiveness checking is not able to handle the case where the /// place being matched on is mutated in the guards. There is an AST check /// that tries to stop this but it is buggy and overly restrictive. Instead /// we add "fake borrows" to the guards that prevent any mutation of the @@ -84,7 +84,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// borrows of `x`, so we only add fake borrows for places which are /// bound or tested by the match. /// 3. We don't want the fake borrows to conflict with `ref mut` bindings, - /// so we lower `ref mut` bindings as two-phase borrows for the guard. + /// so we use a special BorrowKind for them. /// 4. The fake borrows may be of places in inactive variants, so it would /// be UB to generate code for them. They therefore have to be removed /// by a MIR pass run after borrow checking. @@ -145,8 +145,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { .map(|_| self.cfg.start_new_block()) .collect(); - // There's one move pre_binding block that there are candidates so that - // every candidate has a next prebinding_block. + // There's one more pre_binding block than there are candidates so that + // every candidate can have a `next_candidate_pre_binding_block`. let outer_source_info = self.source_info(span); self.cfg.terminate( *pre_binding_blocks.last().unwrap(), @@ -197,13 +197,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Step 3. Create the decision tree and record the places that we bind or test. - // Maps a place to the kind of Fake borrow that we want to perform on - // it: either Shallow or Shared, depending on whether the place is - // bound in the match, or just switched on. - // If there are no match guards then we don't need any fake borrows, - // so don't track them. + // The set of places that we are creating fake borrows of. If there are + // no match guards then we don't need any fake borrows, so don't track + // them. let mut fake_borrows = if match_has_guard && tcx.generate_borrow_of_any_match_input() { - Some(FxHashMap::default()) + Some(FxHashSet::default()) } else { None }; @@ -265,19 +263,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { None, body.span, LintLevel::Inherited, - &arm.patterns[..], + &arm.patterns[0], ArmHasGuard(arm.guard.is_some()), Some((Some(&scrutinee_place), scrutinee_span)), ); - for (pat_index, candidate) in candidates.into_iter().enumerate() { + for candidate in candidates { self.bind_and_guard_matched_candidate( candidate, arm.guard.clone(), arm_block, &fake_borrow_temps, scrutinee_span, - pat_index, ); } @@ -482,7 +479,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { mut visibility_scope: Option, scope_span: Span, lint_level: LintLevel, - patterns: &[Pattern<'tcx>], + pattern: &Pattern<'tcx>, has_guard: ArmHasGuard, opt_match_place: Option<(Option<&Place<'tcx>>, Span)>, ) -> Option { @@ -491,10 +488,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { "can't have both a visibility and a lint scope at the same time" ); let mut scope = self.source_scope; - let num_patterns = patterns.len(); - debug!("declare_bindings: patterns={:?}", patterns); + debug!("declare_bindings: pattern={:?}", pattern); self.visit_bindings( - &patterns[0], + &pattern, UserTypeProjections::none(), &mut |this, mutability, name, mode, var, span, ty, user_ty| { if visibility_scope.is_none() { @@ -515,13 +511,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { mutability, name, mode, - num_patterns, var, ty, user_ty, has_guard, opt_match_place.map(|(x, y)| (x.cloned(), y)), - patterns[0].span, + pattern.span, ); }, ); @@ -804,7 +799,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { span: Span, candidates: &mut [&mut Candidate<'pat, 'tcx>], mut block: BasicBlock, - fake_borrows: &mut Option, BorrowKind>>, + fake_borrows: &mut Option>>, ) -> Vec { debug!( "matched_candidate(span={:?}, block={:?}, candidates={:?})", @@ -901,19 +896,41 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { &mut self, matched_candidates: &mut [&mut Candidate<'_, 'tcx>], block: BasicBlock, - fake_borrows: &mut Option, BorrowKind>>, + fake_borrows: &mut Option>>, ) -> Option { debug_assert!( !matched_candidates.is_empty(), "select_matched_candidates called with no candidates", ); - // Insert a *Shared* borrow of any places that are bound. + // Insert a borrows of prefixes of places that are bound and are + // behind a dereference projection. + // + // These borrows are taken to avoid situations like the following: + // + // match x[10] { + // _ if { x = &[0]; false } => (), + // y => (), // Out of bounds array access! + // } + // + // match *x { + // // y is bound by reference in the guard and then by copy in the + // // arm, so y is 2 in the arm! + // y if { y == 1 && (x = &2) == () } => y, + // _ => 3, + // } if let Some(fake_borrows) = fake_borrows { for Binding { source, .. } in matched_candidates.iter().flat_map(|candidate| &candidate.bindings) { - fake_borrows.insert(source.clone(), BorrowKind::Shared); + let mut cursor = source; + while let Place::Projection(box Projection { base, elem }) = cursor { + cursor = base; + if let ProjectionElem::Deref = elem { + fake_borrows.insert(cursor.clone()); + break; + } + } } } @@ -959,8 +976,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } - - // if None is returned, then debug!("match_candidates: add false edges for unreachable {:?}", unreachable_candidates); for candidate in unreachable_candidates { if let Some(otherwise) = candidate.otherwise_block { @@ -1133,7 +1148,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { span: Span, mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>], block: BasicBlock, - fake_borrows: &mut Option, BorrowKind>>, + fake_borrows: &mut Option>>, ) -> (Vec, &'b mut [&'c mut Candidate<'pat, 'tcx>]) { // extract the match-pair from the highest priority candidate let match_pair = &candidates.first().unwrap().match_pairs[0]; @@ -1177,7 +1192,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Insert a Shallow borrow of any places that is switched on. fake_borrows.as_mut().map(|fb| { - fb.entry(match_place.clone()).or_insert(BorrowKind::Shallow) + fb.insert(match_place.clone()) }); // perform the test, branching to one of N blocks. For each of @@ -1236,9 +1251,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // will evaluate to the same thing until an arm has been chosen. fn calculate_fake_borrows<'b>( &mut self, - fake_borrows: &'b FxHashMap, BorrowKind>, + fake_borrows: &'b FxHashSet>, temp_span: Span, - ) -> Vec<(&'b Place<'tcx>, BorrowKind, Local)> { + ) -> Vec<(&'b Place<'tcx>, Local)> { let tcx = self.hir.tcx(); debug!("add_fake_borrows fake_borrows = {:?}", fake_borrows); @@ -1246,7 +1261,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let mut all_fake_borrows = Vec::with_capacity(fake_borrows.len()); // Insert a Shallow borrow of the prefixes of any fake borrows. - for (place, borrow_kind) in fake_borrows + for place in fake_borrows { let mut prefix_cursor = place; while let Place::Projection(box Projection { base, elem }) = prefix_cursor { @@ -1254,12 +1269,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Insert a shallow borrow after a deref. For other // projections the borrow of prefix_cursor will // conflict with any mutation of base. - all_fake_borrows.push((base, BorrowKind::Shallow)); + all_fake_borrows.push(base); } prefix_cursor = base; } - all_fake_borrows.push((place, *borrow_kind)); + all_fake_borrows.push(place); } // Deduplicate and ensure a deterministic order. @@ -1268,14 +1283,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { debug!("add_fake_borrows all_fake_borrows = {:?}", all_fake_borrows); - all_fake_borrows.into_iter().map(|(matched_place, borrow_kind)| { + all_fake_borrows.into_iter().map(|matched_place| { let fake_borrow_deref_ty = matched_place.ty(&self.local_decls, tcx).to_ty(tcx); let fake_borrow_ty = tcx.mk_imm_ref(tcx.types.re_erased, fake_borrow_deref_ty); let fake_borrow_temp = self.local_decls.push( LocalDecl::new_temp(fake_borrow_ty, temp_span) ); - (matched_place, borrow_kind, fake_borrow_temp) + (matched_place, fake_borrow_temp) }).collect() } } @@ -1301,9 +1316,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { candidate: Candidate<'pat, 'tcx>, guard: Option>, arm_block: BasicBlock, - fake_borrows: &Vec<(&Place<'tcx>, BorrowKind, Local)>, + fake_borrows: &Vec<(&Place<'tcx>, Local)>, scrutinee_span: Span, - pat_index: usize, ) { debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate); @@ -1399,18 +1413,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // // * Here, the guard expression wants a `&&` or `&&mut` // into the original input. This means we need to borrow - // a reference that we do not immediately have at hand - // (because all we have is the places associated with the - // match input itself; it is up to us to create a place - // holding a `&` or `&mut` that we can then borrow). - + // the reference that we create for the arm. + // * So we eagerly create the reference for the arm and then take a + // reference to that. let tcx = self.hir.tcx(); let autoref = tcx.all_pat_vars_are_implicit_refs_within_guards(); if let Some(guard) = guard { if autoref { self.bind_matched_candidate_for_guard( block, - pat_index, &candidate.bindings, ); let guard_frame = GuardFrame { @@ -1428,10 +1439,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let re_erased = tcx.types.re_erased; let scrutinee_source_info = self.source_info(scrutinee_span); - for &(place, borrow_kind, temp) in fake_borrows { + for &(place, temp) in fake_borrows { let borrow = Rvalue::Ref( re_erased, - borrow_kind, + BorrowKind::Shallow, place.clone(), ); self.cfg.push_assign( @@ -1458,7 +1469,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ); } - for &(_, _, temp) in fake_borrows { + for &(_, temp) in fake_borrows { self.cfg.push(block, Statement { source_info: guard_end, kind: StatementKind::FakeRead( @@ -1507,7 +1518,26 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ); if autoref { - self.bind_matched_candidate_for_arm_body(post_guard_block, &candidate.bindings); + let by_value_bindings = candidate.bindings.iter().filter(|binding| { + if let BindingMode::ByValue = binding.binding_mode { true } else { false } + }); + // Read all of the by reference bindings to ensure that the + // place they refer to can't be modified by the guard. + for binding in by_value_bindings.clone() { + let local_id = self.var_local_id(binding.var_id, RefWithinGuard); + let place = Place::Local(local_id); + self.cfg.push( + block, + Statement { + source_info: guard_end, + kind: StatementKind::FakeRead(FakeReadCause::ForGuardBinding, place), + }, + ); + } + self.bind_matched_candidate_for_arm_body( + post_guard_block, + by_value_bindings, + ); } self.cfg.terminate( @@ -1570,13 +1600,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { fn bind_matched_candidate_for_guard( &mut self, block: BasicBlock, - pat_index: usize, bindings: &[Binding<'tcx>], ) { - debug!( - "bind_matched_candidate_for_guard(block={:?}, pat_index={:?}, bindings={:?})", - block, pat_index, bindings - ); + debug!("bind_matched_candidate_for_guard(block={:?}, bindings={:?})", block, bindings); // Assign each of the bindings. Since we are binding for a // guard expression, this will never trigger moves out of the @@ -1603,49 +1629,22 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { .push_assign(block, source_info, &ref_for_guard, rvalue); } BindingMode::ByRef(borrow_kind) => { - // Tricky business: For `ref id` and `ref mut id` - // patterns, we want `id` within the guard to - // correspond to a temp of type `& &T` or `& &mut - // T` (i.e., a "borrow of a borrow") that is - // implicitly dereferenced. - // - // To borrow a borrow, we need that inner borrow - // to point to. So, create a temp for the inner - // borrow, and then take a reference to it. - // - // Note: the temp created here is *not* the one - // used by the arm body itself. This eases - // observing two-phase borrow restrictions. - let val_for_guard = self.storage_live_binding( + let value_for_arm = self.storage_live_binding( block, binding.var_id, binding.span, - ValWithinGuard(pat_index), + OutsideGuard, ); self.schedule_drop_for_binding( binding.var_id, binding.span, - ValWithinGuard(pat_index), + OutsideGuard, ); - // rust-lang/rust#27282: We reuse the two-phase - // borrow infrastructure so that the mutable - // borrow (whose mutabilty is *unusable* within - // the guard) does not conflict with the implicit - // borrow of the whole match input. See additional - // discussion on rust-lang/rust#49870. - let borrow_kind = match borrow_kind { - BorrowKind::Shared - | BorrowKind::Shallow - | BorrowKind::Unique => borrow_kind, - BorrowKind::Mut { .. } => BorrowKind::Mut { - allow_two_phase_borrow: true, - }, - }; let rvalue = Rvalue::Ref(re_erased, borrow_kind, binding.source.clone()); self.cfg - .push_assign(block, source_info, &val_for_guard, rvalue); - let rvalue = Rvalue::Ref(re_erased, BorrowKind::Shared, val_for_guard); + .push_assign(block, source_info, &value_for_arm, rvalue); + let rvalue = Rvalue::Ref(re_erased, BorrowKind::Shared, value_for_arm); self.cfg .push_assign(block, source_info, &ref_for_guard, rvalue); } @@ -1653,16 +1652,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } - fn bind_matched_candidate_for_arm_body( + fn bind_matched_candidate_for_arm_body<'b>( &mut self, block: BasicBlock, - bindings: &[Binding<'tcx>], - ) { - debug!( - "bind_matched_candidate_for_arm_body(block={:?}, bindings={:?}", - block, bindings - ); - + bindings: impl IntoIterator>, + ) where 'tcx: 'b { + debug!("bind_matched_candidate_for_arm_body(block={:?})", block); let re_erased = self.hir.tcx().types.re_erased; // Assign each of the bindings. This may trigger moves out of the candidate. @@ -1683,21 +1678,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } - /// Each binding (`ref mut var`/`ref var`/`mut var`/`var`, where - /// the bound `var` has type `T` in the arm body) in a pattern - /// maps to `2+N` locals. The first local is a binding for - /// occurrences of `var` in the guard, which will all have type - /// `&T`. The N locals are bindings for the `T` that is referenced - /// by the first local; they are not used outside of the - /// guard. The last local is a binding for occurrences of `var` in - /// the arm body, which will have type `T`. - /// - /// The reason we have N locals rather than just 1 is to - /// accommodate rust-lang/rust#51348: If the arm has N candidate - /// patterns, then in general they can correspond to distinct - /// parts of the matched data, and we want them to be distinct - /// temps in order to simplify checks performed by our internal - /// leveraging of two-phase borrows). + /// Each binding (`ref mut var`/`ref var`/`mut var`/`var`, where the bound + /// `var` has type `T` in the arm body) in a pattern maps to 2 locals. The + /// first local is a binding for occurrences of `var` in the guard, which + /// will have type `&T`. The second local is a binding for occurrences of + /// `var` in the arm body, which will have type `T`. fn declare_binding( &mut self, source_info: SourceInfo, @@ -1705,7 +1690,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { mutability: Mutability, name: Name, mode: BindingMode, - num_patterns: usize, var_id: NodeId, var_ty: Ty<'tcx>, user_ty: UserTypeProjections<'tcx>, @@ -1747,31 +1731,20 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }; let for_arm_body = self.local_decls.push(local.clone()); let locals = if has_guard.0 && tcx.all_pat_vars_are_implicit_refs_within_guards() { - let mut vals_for_guard = Vec::with_capacity(num_patterns); - for _ in 0..num_patterns { - let val_for_guard_idx = self.local_decls.push(LocalDecl { - // This variable isn't mutated but has a name, so has to be - // immutable to avoid the unused mut lint. - mutability: Mutability::Not, - ..local.clone() - }); - vals_for_guard.push(val_for_guard_idx); - } let ref_for_guard = self.local_decls.push(LocalDecl::<'tcx> { - // See previous comment. + // This variable isn't mutated but has a name, so has to be + // immutable to avoid the unused mut lint. mutability: Mutability::Not, ty: tcx.mk_imm_ref(tcx.types.re_erased, var_ty), user_ty: UserTypeProjections::none(), name: Some(name), source_info, visibility_scope, - // FIXME: should these secretly injected ref_for_guard's be marked as `internal`? internal: false, is_block_tail: None, is_user_variable: Some(ClearCrossCrate::Set(BindingForm::RefForGuard)), }); LocalsForNode::ForGuard { - vals_for_guard, ref_for_guard, for_arm_body, } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 64ab491cbd5b0..903c8f8657f3d 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -459,8 +459,7 @@ enum LocalsForNode { /// The exceptional case is identifiers in a match arm's pattern /// that are referenced in a guard of that match arm. For these, - /// we can have `2 + k` Locals, where `k` is the number of candidate - /// patterns (separated by `|`) in the arm. + /// we have `2` Locals. /// /// * `for_arm_body` is the Local used in the arm body (which is /// just like the `One` case above), @@ -468,16 +467,7 @@ enum LocalsForNode { /// * `ref_for_guard` is the Local used in the arm's guard (which /// is a reference to a temp that is an alias of /// `for_arm_body`). - /// - /// * `vals_for_guard` is the `k` Locals; at most one of them will - /// get initialized by the arm's execution, and after it is - /// initialized, `ref_for_guard` will be assigned a reference to - /// it. - /// - /// There reason we have `k` Locals rather than just 1 is to - /// accommodate some restrictions imposed by two-phase borrows, - /// which apply when we have a `ref mut` pattern. - ForGuard { vals_for_guard: Vec, ref_for_guard: Local, for_arm_body: Local }, + ForGuard { ref_for_guard: Local, for_arm_body: Local }, } #[derive(Debug)] @@ -510,16 +500,11 @@ struct GuardFrame { } /// `ForGuard` indicates whether we are talking about: -/// 1. the temp for a local binding used solely within guard expressions, -/// 2. the temp that holds reference to (1.), which is actually what the -/// guard expressions see, or -/// 3. the temp for use outside of guard expressions. +/// 1. The variable for use outside of guard expressions, or +/// 2. The temp that holds reference to (1.), which is actually what the +/// guard expressions see. #[derive(Copy, Clone, Debug, PartialEq, Eq)] enum ForGuard { - /// The `usize` identifies for which candidate pattern we want the - /// local binding. We keep a temp per-candidate to accommodate - /// two-phase borrows (see `LocalsForNode` documentation). - ValWithinGuard(usize), RefWithinGuard, OutsideGuard, } @@ -532,11 +517,6 @@ impl LocalsForNode { (&LocalsForNode::ForGuard { for_arm_body: local_id, .. }, ForGuard::OutsideGuard) => local_id, - (&LocalsForNode::ForGuard { ref vals_for_guard, .. }, - ForGuard::ValWithinGuard(pat_idx)) => - vals_for_guard[pat_idx], - - (&LocalsForNode::One(_), ForGuard::ValWithinGuard(_)) | (&LocalsForNode::One(_), ForGuard::RefWithinGuard) => bug!("anything with one local should never be within a guard."), } @@ -941,7 +921,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } _ => { scope = self.declare_bindings(scope, ast_body.span, - LintLevel::Inherited, &[pattern.clone()], + LintLevel::Inherited, &pattern, matches::ArmHasGuard(false), Some((Some(&place), span))); unpack!(block = self.place_into_pattern(block, pattern, &place, false)); diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index 9faecf5b7af46..cf650b5a0ba58 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -73,34 +73,33 @@ fn main() { // return; // } // bb9: { // binding1 and guard -// StorageLive(_8); -// _8 = &(((promoted[2]: std::option::Option) as Some).0: i32); -// _4 = &shallow (promoted[1]: std::option::Option); -// _5 = &(((promoted[0]: std::option::Option) as Some).0: i32); -// StorageLive(_9); -// _9 = const guard() -> [return: bb10, unwind: bb1]; +// StorageLive(_6); +// _6 = &(((promoted[1]: std::option::Option) as Some).0: i32); +// _4 = &shallow (promoted[0]: std::option::Option); +// StorageLive(_7); +// _7 = const guard() -> [return: bb10, unwind: bb1]; // } // bb10: { // FakeRead(ForMatchGuard, _4); -// FakeRead(ForMatchGuard, _5); -// switchInt(move _9) -> [false: bb6, otherwise: bb11]; +// FakeRead(ForGuardBinding, _6); +// switchInt(move _7) -> [false: bb6, otherwise: bb11]; // } // bb11: { -// StorageLive(_6); -// _6 = ((_2 as Some).0: i32); -// StorageLive(_10); -// _10 = _6; -// _1 = (const 1i32, move _10); -// StorageDead(_10); +// StorageLive(_5); +// _5 = ((_2 as Some).0: i32); +// StorageLive(_8); +// _8 = _5; +// _1 = (const 1i32, move _8); +// StorageDead(_8); // goto -> bb8; // } // bb12: { -// StorageLive(_11); -// _11 = ((_2 as Some).0: i32); -// StorageLive(_12); -// _12 = _11; -// _1 = (const 2i32, move _12); -// StorageDead(_12); +// StorageLive(_9); +// _9 = ((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = _9; +// _1 = (const 2i32, move _10); +// StorageDead(_10); // goto -> bb8; // } // bb13: { @@ -143,25 +142,24 @@ fn main() { // return; // } // bb9: { // binding1 and guard -// StorageLive(_8); -// _8 = &((_2 as Some).0: i32); +// StorageLive(_6); +// _6 = &((_2 as Some).0: i32); // _4 = &shallow _2; -// _5 = &((_2 as Some).0: i32); -// StorageLive(_9); -// _9 = const guard() -> [return: bb10, unwind: bb1]; +// StorageLive(_7); +// _7 = const guard() -> [return: bb10, unwind: bb1]; // } // bb10: { // end of guard // FakeRead(ForMatchGuard, _4); -// FakeRead(ForMatchGuard, _5); -// switchInt(move _9) -> [false: bb6, otherwise: bb11]; +// FakeRead(ForGuardBinding, _6); +// switchInt(move _7) -> [false: bb6, otherwise: bb11]; // } // bb11: { // arm1 -// StorageLive(_6); -// _6 = ((_2 as Some).0: i32); -// StorageLive(_10); -// _10 = _6; -// _1 = (const 1i32, move _10); -// StorageDead(_10); +// StorageLive(_5); +// _5 = ((_2 as Some).0: i32); +// StorageLive(_8); +// _8 = _5; +// _1 = (const 1i32, move _8); +// StorageDead(_8); // goto -> bb8; // } // bb12: { // arm2 @@ -169,12 +167,12 @@ fn main() { // goto -> bb8; // } // bb13: { // binding3 and arm3 -// StorageLive(_11); -// _11 = ((_2 as Some).0: i32); -// StorageLive(_12); -// _12 = _11; -// _1 = (const 2i32, move _12); -// StorageDead(_12); +// StorageLive(_9); +// _9 = ((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = _9; +// _1 = (const 2i32, move _10); +// StorageDead(_10); // goto -> bb8; // } // END rustc.full_tested_match2.QualifyAndPromoteConstants.before.mir @@ -216,55 +214,53 @@ fn main() { // return; // } // bb10: { // binding1: Some(w) if guard() -// StorageLive(_9); -// _9 = &((_2 as Some).0: i32); +// StorageLive(_7); +// _7 = &((_2 as Some).0: i32); // _5 = &shallow _2; -// _6 = &((_2 as Some).0: i32); -// StorageLive(_10); -// _10 = const guard() -> [return: bb11, unwind: bb1]; +// StorageLive(_8); +// _8 = const guard() -> [return: bb11, unwind: bb1]; // } // bb11: { //end of guard // FakeRead(ForMatchGuard, _5); -// FakeRead(ForMatchGuard, _6); -// switchInt(move _10) -> [false: bb7, otherwise: bb12]; +// FakeRead(ForGuardBinding, _7); +// switchInt(move _8) -> [false: bb7, otherwise: bb12]; // } // bb12: { // set up bindings for arm1 -// StorageLive(_7); -// _7 = ((_2 as Some).0: i32); +// StorageLive(_6); +// _6 = ((_2 as Some).0: i32); // _1 = const 1i32; // goto -> bb9; // } // bb13: { // binding2 & arm2 -// StorageLive(_11); -// _11 = _2; +// StorageLive(_9); +// _9 = _2; // _1 = const 2i32; // goto -> bb9; // } // bb14: { // binding3: Some(y) if guard2(y) -// StorageLive(_14); -// _14 = &((_2 as Some).0: i32); +// StorageLive(_11); +// _11 = &((_2 as Some).0: i32); // _5 = &shallow _2; -// _6 = &((_2 as Some).0: i32); -// StorageLive(_15); -// StorageLive(_16); -// _16 = (*_14); -// _15 = const guard2(move _16) -> [return: bb15, unwind: bb1]; +// StorageLive(_12); +// StorageLive(_13); +// _13 = (*_11); +// _12 = const guard2(move _13) -> [return: bb15, unwind: bb1]; // } // bb15: { // end of guard2 -// StorageDead(_16); +// StorageDead(_13); // FakeRead(ForMatchGuard, _5); -// FakeRead(ForMatchGuard, _6); -// switchInt(move _15) -> [false: bb8, otherwise: bb16]; +// FakeRead(ForGuardBinding, _11); +// switchInt(move _12) -> [false: bb8, otherwise: bb16]; // } // bb16: { // binding4 & arm4 -// StorageLive(_12); -// _12 = ((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = ((_2 as Some).0: i32); // _1 = const 3i32; // goto -> bb9; // } // bb17: { -// StorageLive(_17); -// _17 = _2; +// StorageLive(_14); +// _14 = _2; // _1 = const 4i32; // goto -> bb9; // } diff --git a/src/test/ui/nll/match-guards-partially-borrow.rs b/src/test/ui/nll/match-guards-partially-borrow.rs index cb8ea2c3d26bd..6e158713146f1 100644 --- a/src/test/ui/nll/match-guards-partially-borrow.rs +++ b/src/test/ui/nll/match-guards-partially-borrow.rs @@ -17,6 +17,30 @@ fn ok_mutation_in_guard(mut q: i32) { } } +fn ok_mutation_in_guard2(mut u: bool) { + // OK value of u is unused before modification + match u { + _ => (), + _ if { + u = true; + false + } => (), + x => (), + } +} + +fn ok_mutation_in_guard4(mut w: (&mut bool,)) { + // OK value of u is unused before modification + match w { + _ => (), + _ if { + *w.0 = true; + false + } => (), + x => (), + } +} + fn ok_indirect_mutation_in_guard(mut p: &bool) { match *p { // OK, mutation doesn't change which patterns s matches @@ -53,8 +77,8 @@ fn mutation_invalidates_previous_pattern_in_guard(mut r: bool) { fn match_on_borrowed_early_end(mut s: bool) { let h = &mut s; - match s { //~ ERROR - // s changes value between the start of the match and when its value is checked. + // OK value of s is unused before modification. + match s { _ if { *h = !*h; false @@ -75,19 +99,7 @@ fn bad_mutation_in_guard(mut t: bool) { } } -fn bad_mutation_in_guard2(mut u: bool) { - match u { - // Guard changes the value bound in the last pattern. - _ => (), - _ if { - u = true; //~ ERROR - false - } => (), - x => (), - } -} - -pub fn bad_mutation_in_guard3(mut x: Option>) { +fn bad_mutation_in_guard2(mut x: Option>) { // Check that nested patterns are checked. match x { None => (), @@ -103,20 +115,7 @@ pub fn bad_mutation_in_guard3(mut x: Option>) { } } - -fn bad_mutation_in_guard4(mut w: (&mut bool,)) { - match w { - // Guard changes the value bound in the last pattern. - _ => (), - _ if { - *w.0 = true; //~ ERROR - false - } => (), - x => (), - } -} - -fn bad_mutation_in_guard5(mut t: bool) { +fn bad_mutation_in_guard3(mut t: bool) { match t { s if { t = !t; //~ ERROR diff --git a/src/test/ui/nll/match-guards-partially-borrow.stderr b/src/test/ui/nll/match-guards-partially-borrow.stderr index b6302b3a65d3c..baff2fda9f5d1 100644 --- a/src/test/ui/nll/match-guards-partially-borrow.stderr +++ b/src/test/ui/nll/match-guards-partially-borrow.stderr @@ -1,5 +1,5 @@ error[E0510]: cannot assign `q` in match guard - --> $DIR/match-guards-partially-borrow.rs:35:13 + --> $DIR/match-guards-partially-borrow.rs:59:13 | LL | match q { | - value is immutable in match guard @@ -8,7 +8,7 @@ LL | q = true; //~ ERROR | ^^^^^^^^ cannot assign error[E0510]: cannot assign `r` in match guard - --> $DIR/match-guards-partially-borrow.rs:47:13 + --> $DIR/match-guards-partially-borrow.rs:71:13 | LL | match r { | - value is immutable in match guard @@ -16,19 +16,8 @@ LL | match r { LL | r = true; //~ ERROR | ^^^^^^^^ cannot assign -error[E0503]: cannot use `s` because it was mutably borrowed - --> $DIR/match-guards-partially-borrow.rs:56:11 - | -LL | let h = &mut s; - | ------ borrow of `s` occurs here -LL | match s { //~ ERROR - | ^ use of borrowed `s` -... -LL | *h = !*h; - | -- borrow later used here - error[E0510]: cannot assign `t` in match guard - --> $DIR/match-guards-partially-borrow.rs:71:13 + --> $DIR/match-guards-partially-borrow.rs:95:13 | LL | match t { | - value is immutable in match guard @@ -36,20 +25,8 @@ LL | match t { LL | t = true; //~ ERROR | ^^^^^^^^ cannot assign -error[E0506]: cannot assign to `u` because it is borrowed - --> $DIR/match-guards-partially-borrow.rs:83:13 - | -LL | match u { - | - borrow of `u` occurs here -... -LL | u = true; //~ ERROR - | ^^^^^^^^ assignment to borrowed `u` occurs here -LL | false -LL | } => (), - | - borrow later used here - error[E0510]: cannot mutably borrow `x.0` in match guard - --> $DIR/match-guards-partially-borrow.rs:97:22 + --> $DIR/match-guards-partially-borrow.rs:109:22 | LL | match x { | - value is immutable in match guard @@ -57,24 +34,11 @@ LL | match x { LL | Some(ref mut r) => *r = None, //~ ERROR | ^^^^^^^^^ cannot mutably borrow -error[E0506]: cannot assign to `*w.0` because it is borrowed - --> $DIR/match-guards-partially-borrow.rs:112:13 - | -LL | match w { - | - borrow of `*w.0` occurs here -... -LL | *w.0 = true; //~ ERROR - | ^^^^^^^^^^^ assignment to borrowed `*w.0` occurs here -LL | false -LL | } => (), - | - borrow later used here - error[E0506]: cannot assign to `t` because it is borrowed - --> $DIR/match-guards-partially-borrow.rs:122:13 + --> $DIR/match-guards-partially-borrow.rs:121:13 | -LL | match t { - | - borrow of `t` occurs here LL | s if { + | - borrow of `t` occurs here LL | t = !t; //~ ERROR | ^^^^^^ assignment to borrowed `t` occurs here LL | false @@ -82,7 +46,7 @@ LL | } => (), // What value should `s` have in the arm? | - borrow later used here error[E0510]: cannot assign `y` in match guard - --> $DIR/match-guards-partially-borrow.rs:133:13 + --> $DIR/match-guards-partially-borrow.rs:132:13 | LL | match *y { | -- value is immutable in match guard @@ -91,7 +55,7 @@ LL | y = &true; //~ ERROR | ^^^^^^^^^ cannot assign error[E0510]: cannot assign `z` in match guard - --> $DIR/match-guards-partially-borrow.rs:144:13 + --> $DIR/match-guards-partially-borrow.rs:143:13 | LL | match z { | - value is immutable in match guard @@ -100,7 +64,7 @@ LL | z = &true; //~ ERROR | ^^^^^^^^^ cannot assign error[E0510]: cannot assign `a` in match guard - --> $DIR/match-guards-partially-borrow.rs:156:13 + --> $DIR/match-guards-partially-borrow.rs:155:13 | LL | match a { | - value is immutable in match guard @@ -109,7 +73,7 @@ LL | a = &true; //~ ERROR | ^^^^^^^^^ cannot assign error[E0510]: cannot assign `b` in match guard - --> $DIR/match-guards-partially-borrow.rs:167:13 + --> $DIR/match-guards-partially-borrow.rs:166:13 | LL | match b { | - value is immutable in match guard @@ -117,7 +81,7 @@ LL | match b { LL | b = &true; //~ ERROR | ^^^^^^^^^ cannot assign -error: aborting due to 12 previous errors +error: aborting due to 9 previous errors -Some errors occurred: E0503, E0506, E0510. -For more information about an error, try `rustc --explain E0503`. +Some errors occurred: E0506, E0510. +For more information about an error, try `rustc --explain E0506`. From 87ec3b24f62edc6324281a2b851a87cf1d94c0dc Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 17:56:14 +0000 Subject: [PATCH 05/10] Activate two phase borrows on all uses Two phase borrows are only used for adjustments now, so there's no need to not activate them for shared borrows. --- src/librustc_mir/borrow_check/borrow_set.rs | 44 ++++++++------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index 53e4ffc8bd6e7..2a3a616317c17 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -3,9 +3,7 @@ use crate::borrow_check::nll::ToRegionVid; use crate::dataflow::indexes::BorrowIndex; use crate::dataflow::move_paths::MoveData; use rustc::mir::traversal; -use rustc::mir::visit::{ - PlaceContext, Visitor, NonUseContext, MutatingUseContext, NonMutatingUseContext -}; +use rustc::mir::visit::{PlaceContext, Visitor, NonUseContext, MutatingUseContext}; use rustc::mir::{self, Location, Mir, Local}; use rustc::ty::{RegionVid, TyCtxt}; use rustc::util::nodemap::{FxHashMap, FxHashSet}; @@ -257,31 +255,21 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { ); } - // Otherwise, this is the unique later use - // that we expect. - borrow_data.activation_location = match context { - // The use of TMP in a shared borrow does not - // count as an actual activation. - PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) | - PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) => - TwoPhaseActivation::NotActivated, - _ => { - // Double check: This borrow is indeed a two-phase borrow (that is, - // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and - // we've not found any other activations (checked above). - assert_eq!( - borrow_data.activation_location, - TwoPhaseActivation::NotActivated, - "never found an activation for this borrow!", - ); - - self.activation_map - .entry(location) - .or_default() - .push(borrow_index); - TwoPhaseActivation::ActivatedAt(location) - } - }; + // Otherwise, this is the unique later use that we expect. + // Double check: This borrow is indeed a two-phase borrow (that is, + // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and + // we've not found any other activations (checked above). + assert_eq!( + borrow_data.activation_location, + TwoPhaseActivation::NotActivated, + "never found an activation for this borrow!", + ); + self.activation_map + .entry(location) + .or_default() + .push(borrow_index); + + borrow_data.activation_location = TwoPhaseActivation::ActivatedAt(location); } } From bf446c80c2792c7ec234ecf4107d75c80826a5f5 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 18:25:10 +0000 Subject: [PATCH 06/10] Add address stability test for matches --- src/test/ui/match/match-ref-mut-stability.rs | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 src/test/ui/match/match-ref-mut-stability.rs diff --git a/src/test/ui/match/match-ref-mut-stability.rs b/src/test/ui/match/match-ref-mut-stability.rs new file mode 100644 index 0000000000000..795a3fc210f6a --- /dev/null +++ b/src/test/ui/match/match-ref-mut-stability.rs @@ -0,0 +1,39 @@ +// Check that `ref mut` variables don't change address between the match guard +// and the arm expression. + +// run-pass + +#![feature(nll, bind_by_move_pattern_guards)] + +// Test that z always point to the same temporary. +fn referent_stability() { + let p; + match 0 { + ref mut z if { p = z as *const _; true } => assert_eq!(p, z as *const _), + _ => unreachable!(), + }; +} + +// Test that z is always effectively the same variable. +fn variable_stability() { + let p; + match 0 { + ref mut z if { p = &z as *const _; true } => assert_eq!(p, &z as *const _), + _ => unreachable!(), + }; +} + +// Test that a borrow of *z can cross from the guard to the arm. +fn persist_borrow() { + let r; + match 0 { + ref mut z if { r = z as &_; true } => assert_eq!(*r, 0), + _ => unreachable!(), + } +} + +fn main() { + referent_stability(); + variable_stability(); + persist_borrow(); +} From 9c601611a02bd2026065a50ba640697488f064b6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 19 Jan 2019 19:13:34 +0000 Subject: [PATCH 07/10] Simplify the cleanup_post_borrowck passes --- .../transform/cleanup_post_borrowck.rs | 97 ++++--------------- src/librustc_mir/transform/mod.rs | 8 +- src/test/mir-opt/remove_fake_borrows.rs | 10 +- 3 files changed, 25 insertions(+), 90 deletions(-) diff --git a/src/librustc_mir/transform/cleanup_post_borrowck.rs b/src/librustc_mir/transform/cleanup_post_borrowck.rs index 890d2c56f42b2..349b27523a0a1 100644 --- a/src/librustc_mir/transform/cleanup_post_borrowck.rs +++ b/src/librustc_mir/transform/cleanup_post_borrowck.rs @@ -1,9 +1,9 @@ -//! This module provides two passes: +//! This module provides a pass to replacing the following statements with +//! [`Nop`]s //! -//! - [`CleanAscribeUserType`], that replaces all [`AscribeUserType`] -//! statements with [`Nop`]. -//! - [`CleanFakeReadsAndBorrows`], that replaces all [`FakeRead`] statements -//! and borrows that are read by [`ForMatchGuard`] fake reads with [`Nop`]. +//! - [`AscribeUserType`] +//! - [`FakeRead`] +//! - [`Assign`] statements with a [`Shallow`] borrow //! //! The `CleanFakeReadsAndBorrows` "pass" is actually implemented as two //! traversals (aka visits) of the input MIR. The first traversal, @@ -11,102 +11,41 @@ //! temporaries read by [`ForMatchGuard`] reads, and [`DeleteFakeBorrows`] //! deletes the initialization of those temporaries. //! -//! [`CleanAscribeUserType`]: cleanup_post_borrowck::CleanAscribeUserType -//! [`CleanFakeReadsAndBorrows`]: cleanup_post_borrowck::CleanFakeReadsAndBorrows -//! [`DeleteAndRecordFakeReads`]: cleanup_post_borrowck::DeleteAndRecordFakeReads -//! [`DeleteFakeBorrows`]: cleanup_post_borrowck::DeleteFakeBorrows //! [`AscribeUserType`]: rustc::mir::StatementKind::AscribeUserType -//! [`Nop`]: rustc::mir::StatementKind::Nop +//! [`Shallow`]: rustc::mir::BorrowKind::Shallow //! [`FakeRead`]: rustc::mir::StatementKind::FakeRead -//! [`ForMatchGuard`]: rustc::mir::FakeReadCause::ForMatchGuard - -use rustc_data_structures::fx::FxHashSet; +//! [`Nop`]: rustc::mir::StatementKind::Nop -use rustc::mir::{BasicBlock, FakeReadCause, Local, Location, Mir, Place}; +use rustc::mir::{BasicBlock, BorrowKind, Rvalue, Location, Mir}; use rustc::mir::{Statement, StatementKind}; use rustc::mir::visit::MutVisitor; use rustc::ty::TyCtxt; use crate::transform::{MirPass, MirSource}; -pub struct CleanAscribeUserType; +pub struct CleanupNonCodegenStatements; -pub struct DeleteAscribeUserType; +pub struct DeleteNonCodegenStatements; -impl MirPass for CleanAscribeUserType { +impl MirPass for CleanupNonCodegenStatements { fn run_pass<'a, 'tcx>(&self, _tcx: TyCtxt<'a, 'tcx, 'tcx>, _source: MirSource<'tcx>, mir: &mut Mir<'tcx>) { - let mut delete = DeleteAscribeUserType; + let mut delete = DeleteNonCodegenStatements; delete.visit_mir(mir); } } -impl<'tcx> MutVisitor<'tcx> for DeleteAscribeUserType { - fn visit_statement(&mut self, - block: BasicBlock, - statement: &mut Statement<'tcx>, - location: Location) { - if let StatementKind::AscribeUserType(..) = statement.kind { - statement.make_nop(); - } - self.super_statement(block, statement, location); - } -} - -pub struct CleanFakeReadsAndBorrows; - -#[derive(Default)] -pub struct DeleteAndRecordFakeReads { - fake_borrow_temporaries: FxHashSet, -} - -pub struct DeleteFakeBorrows { - fake_borrow_temporaries: FxHashSet, -} - -// Removes any FakeReads from the MIR -impl MirPass for CleanFakeReadsAndBorrows { - fn run_pass<'a, 'tcx>(&self, - _tcx: TyCtxt<'a, 'tcx, 'tcx>, - _source: MirSource<'tcx>, - mir: &mut Mir<'tcx>) { - let mut delete_reads = DeleteAndRecordFakeReads::default(); - delete_reads.visit_mir(mir); - let mut delete_borrows = DeleteFakeBorrows { - fake_borrow_temporaries: delete_reads.fake_borrow_temporaries, - }; - delete_borrows.visit_mir(mir); - } -} - -impl<'tcx> MutVisitor<'tcx> for DeleteAndRecordFakeReads { - fn visit_statement(&mut self, - block: BasicBlock, - statement: &mut Statement<'tcx>, - location: Location) { - if let StatementKind::FakeRead(cause, ref place) = statement.kind { - if let FakeReadCause::ForMatchGuard = cause { - match *place { - Place::Local(local) => self.fake_borrow_temporaries.insert(local), - _ => bug!("Fake match guard read of non-local: {:?}", place), - }; - } - statement.make_nop(); - } - self.super_statement(block, statement, location); - } -} - -impl<'tcx> MutVisitor<'tcx> for DeleteFakeBorrows { +impl<'tcx> MutVisitor<'tcx> for DeleteNonCodegenStatements { fn visit_statement(&mut self, block: BasicBlock, statement: &mut Statement<'tcx>, location: Location) { - if let StatementKind::Assign(Place::Local(local), _) = statement.kind { - if self.fake_borrow_temporaries.contains(&local) { - statement.make_nop(); - } + match statement.kind { + StatementKind::AscribeUserType(..) + | StatementKind::Assign(_, box Rvalue::Ref(_, BorrowKind::Shallow, _)) + | StatementKind::FakeRead(..) => statement.make_nop(), + _ => (), } self.super_statement(block, statement, location); } diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 28b9e082851c0..48884872a01ed 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -241,15 +241,11 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx let mut mir = tcx.mir_validated(def_id).steal(); run_passes(tcx, &mut mir, InstanceDef::Item(def_id), MirPhase::Optimized, &[ - // Remove all things not needed by analysis + // Remove all things only needed by analysis &no_landing_pads::NoLandingPads, &simplify_branches::SimplifyBranches::new("initial"), &remove_noop_landing_pads::RemoveNoopLandingPads, - // Remove all `AscribeUserType` statements. - &cleanup_post_borrowck::CleanAscribeUserType, - // Remove all `FakeRead` statements and the borrows that are only - // used for checking matches - &cleanup_post_borrowck::CleanFakeReadsAndBorrows, + &cleanup_post_borrowck::CleanupNonCodegenStatements, &simplify::SimplifyCfg::new("early-opt"), diff --git a/src/test/mir-opt/remove_fake_borrows.rs b/src/test/mir-opt/remove_fake_borrows.rs index fab4d28a936ca..ebb1ef2f430b5 100644 --- a/src/test/mir-opt/remove_fake_borrows.rs +++ b/src/test/mir-opt/remove_fake_borrows.rs @@ -17,7 +17,7 @@ fn main() { // END RUST SOURCE -// START rustc.match_guard.CleanFakeReadsAndBorrows.before.mir +// START rustc.match_guard.CleanupNonCodegenStatements.before.mir // bb0: { // FakeRead(ForMatchedPlace, _1); // _3 = discriminant(_1); @@ -66,9 +66,9 @@ fn main() { // bb10: { // resume; // } -// END rustc.match_guard.CleanFakeReadsAndBorrows.before.mir +// END rustc.match_guard.CleanupNonCodegenStatements.before.mir -// START rustc.match_guard.CleanFakeReadsAndBorrows.after.mir +// START rustc.match_guard.CleanupNonCodegenStatements.after.mir // bb0: { // nop; // _3 = discriminant(_1); @@ -116,5 +116,5 @@ fn main() { // } // bb10: { // resume; -// } -// END rustc.match_guard.CleanFakeReadsAndBorrows.after.mir +// } +// END rustc.match_guard.CleanupNonCodegenStatements.after.mir From fcfeb068171656ab6212134ddd773fff5b9cc9b2 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 9 Feb 2019 20:31:55 +0000 Subject: [PATCH 08/10] Fix codegen test --- src/test/codegen/match.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/codegen/match.rs b/src/test/codegen/match.rs index 35eac2e7fd50a..1b46bb3b25f90 100644 --- a/src/test/codegen/match.rs +++ b/src/test/codegen/match.rs @@ -14,12 +14,12 @@ pub fn exhaustive_match(e: E, unit: ()) { // CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]] // CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]] // CHECK-NEXT: ] +// CHECK: [[OTHERWISE]]: +// CHECK-NEXT: unreachable // CHECK: [[A]]: // CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]] // CHECK: [[B]]: // CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]] -// CHECK: [[OTHERWISE]]: -// CHECK-NEXT: unreachable match e { E::A => unit, E::B => unit, From 90f40cd0fb592bbdc74ef6324e378dd566191386 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 9 Feb 2019 20:35:34 +0000 Subject: [PATCH 09/10] Fix error index example --- src/librustc_mir/diagnostics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/diagnostics.rs b/src/librustc_mir/diagnostics.rs index 4df3004a9ada6..d8384e17ef025 100644 --- a/src/librustc_mir/diagnostics.rs +++ b/src/librustc_mir/diagnostics.rs @@ -1978,8 +1978,8 @@ could cause the match to be non-exhaustive: let mut x = Some(0); match x { None => (), - Some(v) if { x = None; false } => (), - Some(_) => (), // No longer matches + Some(_) if { x = None; false } => (), + Some(v) => (), // No longer matches } ``` From 5ffc9197264a07a50bfb27e436fe284ae8c0687c Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 10 Feb 2019 16:49:26 +0000 Subject: [PATCH 10/10] Move the exit block of the match to the end --- src/librustc_mir/build/matches/mod.rs | 14 ++- src/test/mir-opt/issue-49232.rs | 38 ++++---- src/test/mir-opt/match_false_edges.rs | 120 ++++++++++++------------ src/test/mir-opt/match_test.rs | 40 ++++---- src/test/mir-opt/remove_fake_borrows.rs | 44 ++++----- 5 files changed, 130 insertions(+), 126 deletions(-) diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 07c179ded593f..2c4eb0bc091c3 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -250,12 +250,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Step 5. Create everything else: the guards and the arms. - // all the arm blocks will rejoin here - let end_block = self.cfg.start_new_block(); - let outer_source_info = self.source_info(span); - - for (arm, candidates) in arm_candidates { + let arm_end_blocks: Vec<_> = arm_candidates.into_iter().map(|(arm, candidates)| { let mut arm_block = self.cfg.start_new_block(); let body = self.hir.mirror(arm.body.clone()); @@ -283,6 +279,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } unpack!(arm_block = self.into(destination, arm_block, body)); + + arm_block + }).collect(); + + // all the arm blocks will rejoin here + let end_block = self.cfg.start_new_block(); + + for arm_block in arm_end_blocks { self.cfg.terminate( arm_block, outer_source_info, diff --git a/src/test/mir-opt/issue-49232.rs b/src/test/mir-opt/issue-49232.rs index 5b4ec483277ac..0f0401a55eaca 100644 --- a/src/test/mir-opt/issue-49232.rs +++ b/src/test/mir-opt/issue-49232.rs @@ -47,10 +47,10 @@ fn main() { // resume; // } // bb5: { -// falseEdges -> [real: bb12, imaginary: bb6]; +// falseEdges -> [real: bb11, imaginary: bb6]; // } // bb6: { -// falseEdges -> [real: bb14, imaginary: bb7]; +// falseEdges -> [real: bb13, imaginary: bb7]; // } // bb7: { // unreachable; @@ -62,41 +62,41 @@ fn main() { // goto -> bb5; // } // bb10: { -// FakeRead(ForLet, _2); -// StorageDead(_3); -// StorageLive(_6); -// _6 = &_2; -// _5 = const std::mem::drop(move _6) -> [return: bb19, unwind: bb4]; +// _2 = const 4i32; +// goto -> bb18; // } // bb11: { -// _2 = const 4i32; // goto -> bb10; // } // bb12: { -// goto -> bb11; +// _0 = (); +// goto -> bb14; // } // bb13: { -// _0 = (); -// goto -> bb15; +// goto -> bb12; // } // bb14: { -// goto -> bb13; -// } -// bb15: { // StorageDead(_3); -// goto -> bb16; +// goto -> bb15; // } -// bb16: { +// bb15: { // StorageDead(_2); // goto -> bb2; // } -// bb17: { +// bb16: { // _4 = (); // unreachable; // } -// bb18: { +// bb17: { // StorageDead(_4); -// goto -> bb10; +// goto -> bb18; +// } +// bb18: { +// FakeRead(ForLet, _2); +// StorageDead(_3); +// StorageLive(_6); +// _6 = &_2; +// _5 = const std::mem::drop(move _6) -> [return: bb19, unwind: bb4]; // } // bb19: { // StorageDead(_6); diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index cf650b5a0ba58..ab6de71d2894d 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -51,13 +51,13 @@ fn main() { // resume; // } // bb2: { -// falseEdges -> [real: bb9, imaginary: bb3]; //pre_binding1 +// falseEdges -> [real: bb8, imaginary: bb3]; //pre_binding1 // } // bb3: { -// falseEdges -> [real: bb12, imaginary: bb4]; //pre_binding2 +// falseEdges -> [real: bb11, imaginary: bb4]; //pre_binding2 // } // bb4: { -// falseEdges -> [real: bb13, imaginary: bb5]; //pre_binding3 +// falseEdges -> [real: bb12, imaginary: bb5]; //pre_binding3 // } // bb5: { // unreachable; @@ -68,43 +68,43 @@ fn main() { // bb7: { // unreachable; // } -// bb8: { -// ... -// return; -// } -// bb9: { // binding1 and guard +// bb8: { // binding1 and guard // StorageLive(_6); // _6 = &(((promoted[1]: std::option::Option) as Some).0: i32); // _4 = &shallow (promoted[0]: std::option::Option); // StorageLive(_7); -// _7 = const guard() -> [return: bb10, unwind: bb1]; +// _7 = const guard() -> [return: bb9, unwind: bb1]; // } -// bb10: { +// bb9: { // FakeRead(ForMatchGuard, _4); // FakeRead(ForGuardBinding, _6); -// switchInt(move _7) -> [false: bb6, otherwise: bb11]; +// switchInt(move _7) -> [false: bb6, otherwise: bb10]; // } -// bb11: { +// bb10: { // StorageLive(_5); // _5 = ((_2 as Some).0: i32); // StorageLive(_8); // _8 = _5; // _1 = (const 1i32, move _8); // StorageDead(_8); -// goto -> bb8; +// goto -> bb13; // } -// bb12: { +// bb11: { // StorageLive(_9); // _9 = ((_2 as Some).0: i32); // StorageLive(_10); // _10 = _9; // _1 = (const 2i32, move _10); // StorageDead(_10); -// goto -> bb8; +// goto -> bb13; // } -// bb13: { +// bb12: { // _1 = (const 3i32, const 3i32); -// goto -> bb8; +// goto -> bb13; +// } +// bb13: { +// ... +// return; // } // END rustc.full_tested_match.QualifyAndPromoteConstants.after.mir // @@ -120,13 +120,13 @@ fn main() { // resume; // } // bb2: { -// falseEdges -> [real: bb9, imaginary: bb3]; +// falseEdges -> [real: bb8, imaginary: bb3]; // } // bb3: { -// falseEdges -> [real: bb12, imaginary: bb4]; +// falseEdges -> [real: bb11, imaginary: bb4]; // } // bb4: { -// falseEdges -> [real: bb13, imaginary: bb5]; +// falseEdges -> [real: bb12, imaginary: bb5]; // } // bb5: { // unreachable; @@ -137,43 +137,43 @@ fn main() { // bb7: { // unreachable; // } -// bb8: { -// ... -// return; -// } -// bb9: { // binding1 and guard +// bb8: { // binding1 and guard // StorageLive(_6); // _6 = &((_2 as Some).0: i32); // _4 = &shallow _2; // StorageLive(_7); -// _7 = const guard() -> [return: bb10, unwind: bb1]; +// _7 = const guard() -> [return: bb9, unwind: bb1]; // } -// bb10: { // end of guard +// bb9: { // end of guard // FakeRead(ForMatchGuard, _4); // FakeRead(ForGuardBinding, _6); -// switchInt(move _7) -> [false: bb6, otherwise: bb11]; +// switchInt(move _7) -> [false: bb6, otherwise: bb10]; // } -// bb11: { // arm1 +// bb10: { // arm1 // StorageLive(_5); // _5 = ((_2 as Some).0: i32); // StorageLive(_8); // _8 = _5; // _1 = (const 1i32, move _8); // StorageDead(_8); -// goto -> bb8; +// goto -> bb13; // } -// bb12: { // arm2 +// bb11: { // arm2 // _1 = (const 3i32, const 3i32); -// goto -> bb8; +// goto -> bb13; // } -// bb13: { // binding3 and arm3 +// bb12: { // binding3 and arm3 // StorageLive(_9); // _9 = ((_2 as Some).0: i32); // StorageLive(_10); // _10 = _9; // _1 = (const 2i32, move _10); // StorageDead(_10); -// goto -> bb8; +// goto -> bb13; +// } +// bb13: { +// ... +// return; // } // END rustc.full_tested_match2.QualifyAndPromoteConstants.before.mir // @@ -189,79 +189,79 @@ fn main() { // resume; // } // bb2: { -// falseEdges -> [real: bb10, imaginary: bb3]; //pre_binding1 +// falseEdges -> [real: bb9, imaginary: bb3]; // } // bb3: { -// falseEdges -> [real: bb13, imaginary: bb4]; //pre_binding2 +// falseEdges -> [real: bb12, imaginary: bb4]; // } // bb4: { -// falseEdges -> [real: bb14, imaginary: bb5]; //pre_binding3 +// falseEdges -> [real: bb13, imaginary: bb5]; // } // bb5: { -// falseEdges -> [real: bb17, imaginary: bb6]; //pre_binding4 +// falseEdges -> [real: bb16, imaginary: bb6]; // } // bb6: { // unreachable; // } -// bb7: { // to pre_binding2 +// bb7: { // falseEdges -> [real: bb3, imaginary: bb3]; // } -// bb8: { // to pre_binding4 +// bb8: { // falseEdges -> [real: bb5, imaginary: bb5]; // } -// bb9: { -// ... -// return; -// } -// bb10: { // binding1: Some(w) if guard() +// bb9: { // binding1: Some(w) if guard() // StorageLive(_7); // _7 = &((_2 as Some).0: i32); // _5 = &shallow _2; // StorageLive(_8); -// _8 = const guard() -> [return: bb11, unwind: bb1]; +// _8 = const guard() -> [return: bb10, unwind: bb1]; // } -// bb11: { //end of guard +// bb10: { //end of guard // FakeRead(ForMatchGuard, _5); // FakeRead(ForGuardBinding, _7); -// switchInt(move _8) -> [false: bb7, otherwise: bb12]; +// switchInt(move _8) -> [false: bb7, otherwise: bb11]; // } -// bb12: { // set up bindings for arm1 +// bb11: { // set up bindings for arm1 // StorageLive(_6); // _6 = ((_2 as Some).0: i32); // _1 = const 1i32; -// goto -> bb9; +// goto -> bb17; // } -// bb13: { // binding2 & arm2 +// bb12: { // binding2 & arm2 // StorageLive(_9); // _9 = _2; // _1 = const 2i32; -// goto -> bb9; +// goto -> bb17; // } -// bb14: { // binding3: Some(y) if guard2(y) +// bb13: { // binding3: Some(y) if guard2(y) // StorageLive(_11); // _11 = &((_2 as Some).0: i32); // _5 = &shallow _2; // StorageLive(_12); // StorageLive(_13); // _13 = (*_11); -// _12 = const guard2(move _13) -> [return: bb15, unwind: bb1]; +// _12 = const guard2(move _13) -> [return: bb14, unwind: bb1]; // } -// bb15: { // end of guard2 +// bb14: { // end of guard2 // StorageDead(_13); // FakeRead(ForMatchGuard, _5); // FakeRead(ForGuardBinding, _11); -// switchInt(move _12) -> [false: bb8, otherwise: bb16]; +// switchInt(move _12) -> [false: bb8, otherwise: bb15]; // } -// bb16: { // binding4 & arm4 +// bb15: { // binding4 & arm4 // StorageLive(_10); // _10 = ((_2 as Some).0: i32); // _1 = const 3i32; -// goto -> bb9; +// goto -> bb17; // } -// bb17: { +// bb16: { // StorageLive(_14); // _14 = _2; // _1 = const 4i32; -// goto -> bb9; +// goto -> bb17; +// } +// bb17: { +// ... +// return; // } // END rustc.main.QualifyAndPromoteConstants.before.mir diff --git a/src/test/mir-opt/match_test.rs b/src/test/mir-opt/match_test.rs index 835a5ce1200b9..3f248f3d41a64 100644 --- a/src/test/mir-opt/match_test.rs +++ b/src/test/mir-opt/match_test.rs @@ -23,16 +23,16 @@ fn main() { // switchInt(move _4) -> [false: bb7, otherwise: bb8]; // } // bb1: { -// falseEdges -> [real: bb13, imaginary: bb2]; +// falseEdges -> [real: bb12, imaginary: bb2]; // } // bb2: { -// falseEdges -> [real: bb14, imaginary: bb3]; +// falseEdges -> [real: bb13, imaginary: bb3]; // } // bb3: { -// falseEdges -> [real: bb15, imaginary: bb4]; +// falseEdges -> [real: bb14, imaginary: bb4]; // } // bb4: { -// falseEdges -> [real: bb16, imaginary: bb5]; +// falseEdges -> [real: bb15, imaginary: bb5]; // } // bb5: { // unreachable; @@ -56,31 +56,31 @@ fn main() { // switchInt(move _7) -> [false: bb9, otherwise: bb2]; // } // bb11: { -// StorageDead(_8); -// _0 = (); -// StorageDead(_2); -// StorageDead(_1); -// return; -// } -// bb12: { // _3 = const 0i32; -// goto -> bb11; +// goto -> bb16; // } -// bb13: { +// bb12: { // StorageLive(_8); // _8 = _2; -// switchInt(move _8) -> [false: bb6, otherwise: bb12]; +// switchInt(move _8) -> [false: bb6, otherwise: bb11]; // } -// bb14: { +// bb13: { // _3 = const 1i32; -// goto -> bb11; +// goto -> bb16; // } -// bb15: { +// bb14: { // _3 = const 2i32; -// goto -> bb11; +// goto -> bb16; // } -// bb16: { +// bb15: { // _3 = const 3i32; -// goto -> bb11; +// goto -> bb16; +// } +// bb16: { +// StorageDead(_8); +// _0 = (); +// StorageDead(_2); +// StorageDead(_1); +// return; // } // END rustc.main.SimplifyCfg-initial.after.mir diff --git a/src/test/mir-opt/remove_fake_borrows.rs b/src/test/mir-opt/remove_fake_borrows.rs index ebb1ef2f430b5..48d1c991b623e 100644 --- a/src/test/mir-opt/remove_fake_borrows.rs +++ b/src/test/mir-opt/remove_fake_borrows.rs @@ -24,10 +24,10 @@ fn main() { // switchInt(move _3) -> [1isize: bb5, otherwise: bb2]; // } // bb1: { -// goto -> bb8; +// goto -> bb7; // } // bb2: { -// goto -> bb9; +// goto -> bb8; // } // bb3: { // unreachable; @@ -39,14 +39,10 @@ fn main() { // switchInt((*(*((_1 as Some).0: &' &' i32)))) -> [0i32: bb1, otherwise: bb2]; // } // bb6: { -// StorageDead(_8); -// return; -// } -// bb7: { // _0 = const 0i32; -// goto -> bb6; +// goto -> bb9; // } -// bb8: { +// bb7: { // _4 = &shallow _1; // _5 = &shallow ((_1 as Some).0: &' &' i32); // _6 = &shallow (*((_1 as Some).0: &' &' i32)); @@ -57,11 +53,15 @@ fn main() { // FakeRead(ForMatchGuard, _5); // FakeRead(ForMatchGuard, _6); // FakeRead(ForMatchGuard, _7); -// switchInt(move _8) -> [false: bb4, otherwise: bb7]; +// switchInt(move _8) -> [false: bb4, otherwise: bb6]; // } -// bb9: { +// bb8: { // _0 = const 1i32; -// goto -> bb6; +// goto -> bb9; +// } +// bb9: { +// StorageDead(_8); +// return; // } // bb10: { // resume; @@ -75,10 +75,10 @@ fn main() { // switchInt(move _3) -> [1isize: bb5, otherwise: bb2]; // } // bb1: { -// goto -> bb8; +// goto -> bb7; // } // bb2: { -// goto -> bb9; +// goto -> bb8; // } // bb3: { // unreachable; @@ -90,14 +90,10 @@ fn main() { // switchInt((*(*((_1 as Some).0: &' &' i32)))) -> [0i32: bb1, otherwise: bb2]; // } // bb6: { -// StorageDead(_8); -// return; -// } -// bb7: { // _0 = const 0i32; -// goto -> bb6; +// goto -> bb9; // } -// bb8: { +// bb7: { // nop; // nop; // nop; @@ -108,11 +104,15 @@ fn main() { // nop; // nop; // nop; -// switchInt(move _8) -> [false: bb4, otherwise: bb7]; +// switchInt(move _8) -> [false: bb4, otherwise: bb6]; // } -// bb9: { +// bb8: { // _0 = const 1i32; -// goto -> bb6; +// goto -> bb9; +// } +// bb9: { +// StorageDead(_8); +// return; // } // bb10: { // resume;