diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index bcf3772213014..a316fc5ca1029 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -92,12 +92,12 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> { mir::BorrowKind::Mut { .. } => "mut ", }; let region = self.region.to_string(); - let region = if region.len() > 0 { - format!("{} ", region) + let separator = if !region.is_empty() { + " " } else { - region + "" }; - write!(w, "&{}{}{:?}", region, kind, self.borrowed_place) + write!(w, "&{}{}{}{:?}", region, separator, kind, self.borrowed_place) } } @@ -244,7 +244,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { 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 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { // ... 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 546746aa72ebb..759b842e9dfee 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -77,9 +77,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if move_out_indices.is_empty() { let root_place = self.prefixes(&used_place, PrefixSet::All).last().unwrap(); - if self.uninitialized_error_reported - .contains(&root_place.clone()) - { + if self.uninitialized_error_reported.contains(root_place) { debug!( "report_use_of_moved_or_uninitialized place: error about {:?} suppressed", root_place @@ -188,11 +186,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { 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, }; @@ -582,7 +577,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { fn report_local_value_does_not_live_long_enough( &mut self, context: Context, - name: &String, + name: &str, scope_tree: &Lrc, borrow: &BorrowData<'tcx>, drop_span: Span, @@ -1195,10 +1190,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Static(ref static_) => self.describe_field_from_ty(&static_.ty, field), Place::Projection(ref proj) => match proj.elem { ProjectionElem::Deref => self.describe_field(&proj.base, field), - ProjectionElem::Downcast(def, variant_index) => format!( - "{}", - def.variants[variant_index].fields[field.index()].ident - ), + ProjectionElem::Downcast(def, variant_index) => + def.variants[variant_index].fields[field.index()].ident.to_string(), ProjectionElem::Field(_, field_type) => { self.describe_field_from_ty(&field_type, field) } @@ -1366,191 +1359,184 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { "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 +1591,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // 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"); } } } @@ -1794,8 +1779,8 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> { ty::RegionKind::RePlaceholder(ty::Placeholder { name: br, .. }), _, _, - ) => with_highlight_region_for_bound_region(*br, counter, || format!("{}", ty)), - _ => format!("{}", ty), + ) => with_highlight_region_for_bound_region(*br, counter, || ty.to_string()), + _ => ty.to_string(), } } @@ -1806,9 +1791,9 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> { ty::TyKind::Ref(region, _, _) => match region { ty::RegionKind::ReLateBound(_, br) | ty::RegionKind::RePlaceholder(ty::Placeholder { name: br, .. }) => { - with_highlight_region_for_bound_region(*br, counter, || format!("{}", region)) + with_highlight_region_for_bound_region(*br, counter, || region.to_string()) } - _ => format!("{}", region), + _ => region.to_string(), }, _ => bug!("ty for annotation of borrow region is not a reference"), } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 1f8d077fb6904..aeb77c67317a0 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() { @@ -1009,13 +1009,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { return Control::Continue; } + error_reported = true; match kind { ReadKind::Copy => { - error_reported = true; this.report_use_while_mutably_borrowed(context, place_span, borrow) } ReadKind::Borrow(bk) => { - error_reported = true; this.report_conflicting_borrow(context, place_span, bk, &borrow) } } @@ -1045,13 +1044,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Read(..) | Write(..) => {} } + error_reported = true; match kind { WriteKind::MutableBorrow(bk) => { - error_reported = true; this.report_conflicting_borrow(context, place_span, bk, &borrow) } WriteKind::StorageDeadOrDrop => { - error_reported = true; this.report_borrowed_value_does_not_live_long_enough( context, borrow, @@ -1059,11 +1057,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Some(kind)) } WriteKind::Mutate => { - error_reported = true; this.report_illegal_mutation_of_borrowed(context, place_span, borrow) } WriteKind::Move => { - error_reported = true; this.report_move_out_while_borrowed(context, place_span, &borrow) } } @@ -1593,7 +1589,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Local(_) => panic!("should have move path for every Local"), Place::Projection(_) => panic!("PrefixSet::All meant don't stop for Projection"), Place::Promoted(_) | - Place::Static(_) => return Err(NoMovePathFound::ReachedStatic), + Place::Static(_) => Err(NoMovePathFound::ReachedStatic), } } @@ -1885,7 +1881,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } // at this point, we have set up the error reporting state. - if previously_initialized { + return if previously_initialized { self.report_mutability_error( place, span, @@ -1893,10 +1889,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { error_access, location, ); - return true; + true } else { - return false; - } + false + }; } fn is_local_ever_initialized(&self, @@ -1911,7 +1907,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { return Some(index); } } - return None; + None } /// Adds the place into the used mutable variables set @@ -2171,7 +2167,7 @@ impl ContextKind { fn new(self, loc: Location) -> Context { Context { kind: self, - loc: loc, + loc, } } } diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index ea62694f8be75..a556199b875bf 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -331,7 +331,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { _ => { let source = self.borrowed_content_source(place); self.infcx.tcx.cannot_move_out_of( - span, &format!("{}", source), origin + span, &source.to_string(), origin ) }, } @@ -469,9 +469,9 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { let binding_span = bind_to.source_info.span; if j == 0 { - err.span_label(binding_span, format!("data moved here")); + err.span_label(binding_span, "data moved here"); } else { - err.span_label(binding_span, format!("...and here")); + err.span_label(binding_span, "...and here"); } if binds_to.len() == 1 { diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 5ab1605d7f07a..30f4fc9d5ea23 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -408,7 +408,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { .map(|replacement| (pattern_span, replacement)) } - // ClearCrossCrate::Set(mir::BindingForm::RefForGuard) => unreachable!(), ClearCrossCrate::Clear => bug!("saw cleared local state"), @@ -505,7 +504,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { ); let extra = if found { - String::from("") + String::new() } else { format!(", but it is not implemented for `{}`", substs.type_at(0)) @@ -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)) = ( @@ -584,7 +583,7 @@ fn suggest_ampmut<'cx, 'gcx, 'tcx>( let ty = &src[ws_pos..]; return (assignment_rhs_span, format!("&{} mut {}", lt_name, ty)); } else if src.starts_with('&') { - let borrowed_expr = src[1..].to_string(); + let borrowed_expr = &src[1..]; return (assignment_rhs_span, format!("&mut {}", borrowed_expr)); } }