From: ljedrz Date: Tue, 16 Oct 2018 13:06:59 +0000 (+0200) Subject: mir/borrowck: simplify common patterns X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=3ba62dd0f6d3c609a5d5ac61f0e1fc7c6861f66c;p=rust.git mir/borrowck: simplify common patterns --- diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index bcf37722130..18cc1430c2a 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -244,7 +244,7 @@ fn insert<'a, K, V>(map: &'a mut FxHashMap>, k: &K, v: V) K: Clone + Eq + Hash, V: Eq + Hash, { - map.entry(k.clone()).or_insert(FxHashSet()).insert(v); + map.entry(k.clone()).or_default().insert(v); } } @@ -261,57 +261,53 @@ fn visit_place( // ... check whether we (earlier) saw a 2-phase borrow like // // TMP = &mut place - match self.pending_activations.get(temp) { - Some(&borrow_index) => { - let borrow_data = &mut self.idx_vec[borrow_index]; - - // Watch out: the use of TMP in the borrow itself - // doesn't count as an activation. =) - if borrow_data.reserve_location == location && context == PlaceContext::Store { - return; - } + if let Some(&borrow_index) = self.pending_activations.get(temp) { + let borrow_data = &mut self.idx_vec[borrow_index]; - if let TwoPhaseActivation::ActivatedAt(other_location) = - borrow_data.activation_location { - span_bug!( - self.mir.source_info(location).span, - "found two uses for 2-phase borrow temporary {:?}: \ - {:?} and {:?}", - temp, - location, - other_location, - ); - } + // Watch out: the use of TMP in the borrow itself + // doesn't count as an activation. =) + if borrow_data.reserve_location == location && context == PlaceContext::Store { + return; + } - // 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::Borrow { kind: mir::BorrowKind::Shared, .. } - | PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => { - 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) - } - }; + if let TwoPhaseActivation::ActivatedAt(other_location) = + borrow_data.activation_location { + span_bug!( + self.mir.source_info(location).span, + "found two uses for 2-phase borrow temporary {:?}: \ + {:?} and {:?}", + temp, + location, + other_location, + ); } - None => {} + // 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::Borrow { kind: mir::BorrowKind::Shared, .. } + | PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => { + 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) + } + }; } } } diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 546746aa72e..8d5629270fe 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -188,11 +188,8 @@ pub(super) fn report_use_of_moved_or_uninitialized( let tables = self.infcx.tcx.typeck_tables_of(id); let node_id = self.infcx.tcx.hir.as_local_node_id(id).unwrap(); let hir_id = self.infcx.tcx.hir.node_to_hir_id(node_id); - if tables.closure_kind_origins().get(hir_id).is_some() { - false - } else { - true - } + + tables.closure_kind_origins().get(hir_id).is_none() } _ => true, }; @@ -1366,191 +1363,184 @@ fn annotate_argument_and_return_for_borrow( "annotate_argument_and_return_for_borrow: location={:?}", location ); - match &self.mir[location.block] - .statements - .get(location.statement_index) + if let Some(&Statement { kind: StatementKind::Assign(ref reservation, _), ..}) + = &self.mir[location.block].statements.get(location.statement_index) { - Some(&Statement { - kind: StatementKind::Assign(ref reservation, _), - .. - }) => { + debug!( + "annotate_argument_and_return_for_borrow: reservation={:?}", + reservation + ); + // Check that the initial assignment of the reserve location is into a temporary. + let mut target = *match reservation { + Place::Local(local) if self.mir.local_kind(*local) == LocalKind::Temp => local, + _ => return None, + }; + + // Next, look through the rest of the block, checking if we are assigning the + // `target` (that is, the place that contains our borrow) to anything. + let mut annotated_closure = None; + for stmt in &self.mir[location.block].statements[location.statement_index + 1..] { debug!( - "annotate_argument_and_return_for_borrow: reservation={:?}", - reservation + "annotate_argument_and_return_for_borrow: target={:?} stmt={:?}", + target, stmt ); - // Check that the initial assignment of the reserve location is into a temporary. - let mut target = *match reservation { - Place::Local(local) if self.mir.local_kind(*local) == LocalKind::Temp => local, - _ => return None, - }; - - // Next, look through the rest of the block, checking if we are assigning the - // `target` (that is, the place that contains our borrow) to anything. - let mut annotated_closure = None; - for stmt in &self.mir[location.block].statements[location.statement_index + 1..] { + if let StatementKind::Assign(Place::Local(assigned_to), box rvalue) = &stmt.kind + { debug!( - "annotate_argument_and_return_for_borrow: target={:?} stmt={:?}", - target, stmt + "annotate_argument_and_return_for_borrow: assigned_to={:?} \ + rvalue={:?}", + assigned_to, rvalue ); - if let StatementKind::Assign(Place::Local(assigned_to), box rvalue) = &stmt.kind + // Check if our `target` was captured by a closure. + if let Rvalue::Aggregate( + box AggregateKind::Closure(def_id, substs), + operands, + ) = rvalue { - debug!( - "annotate_argument_and_return_for_borrow: assigned_to={:?} \ - rvalue={:?}", - assigned_to, rvalue - ); - // Check if our `target` was captured by a closure. - if let Rvalue::Aggregate( - box AggregateKind::Closure(def_id, substs), - operands, - ) = rvalue - { - for operand in operands { - let assigned_from = match operand { - Operand::Copy(assigned_from) | Operand::Move(assigned_from) => { - assigned_from - } - _ => continue, - }; - debug!( - "annotate_argument_and_return_for_borrow: assigned_from={:?}", + for operand in operands { + let assigned_from = match operand { + Operand::Copy(assigned_from) | Operand::Move(assigned_from) => { assigned_from - ); - - // Find the local from the operand. - let assigned_from_local = match assigned_from.local() { - Some(local) => local, - None => continue, - }; - - if assigned_from_local != target { - continue; } + _ => continue, + }; + debug!( + "annotate_argument_and_return_for_borrow: assigned_from={:?}", + assigned_from + ); - // If a closure captured our `target` and then assigned - // into a place then we should annotate the closure in - // case it ends up being assigned into the return place. - annotated_closure = self.annotate_fn_sig( - *def_id, - self.infcx.closure_sig(*def_id, *substs), - ); - debug!( - "annotate_argument_and_return_for_borrow: \ - annotated_closure={:?} assigned_from_local={:?} \ - assigned_to={:?}", - annotated_closure, assigned_from_local, assigned_to - ); - - if *assigned_to == mir::RETURN_PLACE { - // If it was assigned directly into the return place, then - // return now. - return annotated_closure; - } else { - // Otherwise, update the target. - target = *assigned_to; - } + // Find the local from the operand. + let assigned_from_local = match assigned_from.local() { + Some(local) => local, + None => continue, + }; + + if assigned_from_local != target { + continue; } - // If none of our closure's operands matched, then skip to the next - // statement. - continue; + // If a closure captured our `target` and then assigned + // into a place then we should annotate the closure in + // case it ends up being assigned into the return place. + annotated_closure = self.annotate_fn_sig( + *def_id, + self.infcx.closure_sig(*def_id, *substs), + ); + debug!( + "annotate_argument_and_return_for_borrow: \ + annotated_closure={:?} assigned_from_local={:?} \ + assigned_to={:?}", + annotated_closure, assigned_from_local, assigned_to + ); + + if *assigned_to == mir::RETURN_PLACE { + // If it was assigned directly into the return place, then + // return now. + return annotated_closure; + } else { + // Otherwise, update the target. + target = *assigned_to; + } } - // Otherwise, look at other types of assignment. - let assigned_from = match rvalue { - Rvalue::Ref(_, _, assigned_from) => assigned_from, - Rvalue::Use(operand) => match operand { - Operand::Copy(assigned_from) | Operand::Move(assigned_from) => { - assigned_from - } - _ => continue, - }, - _ => continue, - }; - debug!( - "annotate_argument_and_return_for_borrow: \ - assigned_from={:?}", - assigned_from, - ); + // If none of our closure's operands matched, then skip to the next + // statement. + continue; + } - // Find the local from the rvalue. - let assigned_from_local = match assigned_from.local() { - Some(local) => local, - None => continue, - }; - debug!( - "annotate_argument_and_return_for_borrow: \ - assigned_from_local={:?}", - assigned_from_local, - ); + // Otherwise, look at other types of assignment. + let assigned_from = match rvalue { + Rvalue::Ref(_, _, assigned_from) => assigned_from, + Rvalue::Use(operand) => match operand { + Operand::Copy(assigned_from) | Operand::Move(assigned_from) => { + assigned_from + } + _ => continue, + }, + _ => continue, + }; + debug!( + "annotate_argument_and_return_for_borrow: \ + assigned_from={:?}", + assigned_from, + ); - // Check if our local matches the target - if so, we've assigned our - // borrow to a new place. - if assigned_from_local != target { - continue; - } + // Find the local from the rvalue. + let assigned_from_local = match assigned_from.local() { + Some(local) => local, + None => continue, + }; + debug!( + "annotate_argument_and_return_for_borrow: \ + assigned_from_local={:?}", + assigned_from_local, + ); - // If we assigned our `target` into a new place, then we should - // check if it was the return place. - debug!( - "annotate_argument_and_return_for_borrow: \ - assigned_from_local={:?} assigned_to={:?}", - assigned_from_local, assigned_to - ); - if *assigned_to == mir::RETURN_PLACE { - // If it was then return the annotated closure if there was one, - // else, annotate this function. - return annotated_closure.or_else(fallback); - } + // Check if our local matches the target - if so, we've assigned our + // borrow to a new place. + if assigned_from_local != target { + continue; + } - // If we didn't assign into the return place, then we just update - // the target. - target = *assigned_to; + // If we assigned our `target` into a new place, then we should + // check if it was the return place. + debug!( + "annotate_argument_and_return_for_borrow: \ + assigned_from_local={:?} assigned_to={:?}", + assigned_from_local, assigned_to + ); + if *assigned_to == mir::RETURN_PLACE { + // If it was then return the annotated closure if there was one, + // else, annotate this function. + return annotated_closure.or_else(fallback); } + + // If we didn't assign into the return place, then we just update + // the target. + target = *assigned_to; } + } - // Check the terminator if we didn't find anything in the statements. - let terminator = &self.mir[location.block].terminator(); + // Check the terminator if we didn't find anything in the statements. + let terminator = &self.mir[location.block].terminator(); + debug!( + "annotate_argument_and_return_for_borrow: target={:?} terminator={:?}", + target, terminator + ); + if let TerminatorKind::Call { + destination: Some((Place::Local(assigned_to), _)), + args, + .. + } = &terminator.kind + { debug!( - "annotate_argument_and_return_for_borrow: target={:?} terminator={:?}", - target, terminator + "annotate_argument_and_return_for_borrow: assigned_to={:?} args={:?}", + assigned_to, args ); - if let TerminatorKind::Call { - destination: Some((Place::Local(assigned_to), _)), - args, - .. - } = &terminator.kind - { + for operand in args { + let assigned_from = match operand { + Operand::Copy(assigned_from) | Operand::Move(assigned_from) => { + assigned_from + } + _ => continue, + }; debug!( - "annotate_argument_and_return_for_borrow: assigned_to={:?} args={:?}", - assigned_to, args + "annotate_argument_and_return_for_borrow: assigned_from={:?}", + assigned_from, ); - for operand in args { - let assigned_from = match operand { - Operand::Copy(assigned_from) | Operand::Move(assigned_from) => { - assigned_from - } - _ => continue, - }; + + if let Some(assigned_from_local) = assigned_from.local() { debug!( - "annotate_argument_and_return_for_borrow: assigned_from={:?}", - assigned_from, + "annotate_argument_and_return_for_borrow: assigned_from_local={:?}", + assigned_from_local, ); - if let Some(assigned_from_local) = assigned_from.local() { - debug!( - "annotate_argument_and_return_for_borrow: assigned_from_local={:?}", - assigned_from_local, - ); - - if *assigned_to == mir::RETURN_PLACE && assigned_from_local == target { - return annotated_closure.or_else(fallback); - } + if *assigned_to == mir::RETURN_PLACE && assigned_from_local == target { + return annotated_closure.or_else(fallback); } } } } - _ => {} } // If we haven't found an assignment into the return place, then we need not add @@ -1605,13 +1595,12 @@ fn annotate_fn_sig( // Need to use the `rustc::ty` types to compare against the // `return_region`. Then use the `rustc::hir` type to get only // the lifetime span. - match &fn_decl.inputs[index].node { - hir::TyKind::Rptr(lifetime, _) => { - // With access to the lifetime, we can get - // the span of it. - arguments.push((*argument, lifetime.span)); - } - _ => bug!("ty type is a ref but hir type is not"), + if let hir::TyKind::Rptr(lifetime, _) = &fn_decl.inputs[index].node { + // With access to the lifetime, we can get + // the span of it. + arguments.push((*argument, lifetime.span)); + } else { + bug!("ty type is a ref but hir type is not"); } } } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 1f8d077fb69..59bb534881e 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -284,7 +284,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let temporary_used_locals: FxHashSet = mbcx .used_mut .iter() - .filter(|&local| !mbcx.mir.local_decls[*local].is_user_variable.is_some()) + .filter(|&local| mbcx.mir.local_decls[*local].is_user_variable.is_none()) .cloned() .collect(); mbcx.gather_used_muts(temporary_used_locals); @@ -342,7 +342,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( diag.buffer(&mut mbcx.errors_buffer); } - if mbcx.errors_buffer.len() > 0 { + if !mbcx.errors_buffer.is_empty() { mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span()); if tcx.migrate_borrowck() { @@ -2171,7 +2171,7 @@ impl ContextKind { fn new(self, loc: Location) -> Context { Context { kind: self, - loc: loc, + loc, } } } diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 5ab1605d7f0..f0b4a5ac743 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -408,7 +408,6 @@ pub(super) fn report_mutability_error( .map(|replacement| (pattern_span, replacement)) } - // ClearCrossCrate::Set(mir::BindingForm::RefForGuard) => unreachable!(), ClearCrossCrate::Clear => bug!("saw cleared local state"), @@ -573,7 +572,7 @@ fn suggest_ampmut<'cx, 'gcx, 'tcx>( opt_ty_info: Option, ) -> (Span, String) { let locations = mir.find_assignments(local); - if locations.len() > 0 { + if !locations.is_empty() { let assignment_rhs_span = mir.source_info(locations[0]).span; if let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span) { if let (true, Some(ws_pos)) = (