From 75c0c8c6dc174cdef49fbc4cbd9ca084f98bd3f4 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 19 Jul 2019 19:04:01 +0200 Subject: [PATCH] Avoid cloning Place in describe_place_for_conflicting_borrow --- .../borrow_check/conflict_errors.rs | 51 ++++++++++--------- .../borrow_check/error_reporting.rs | 4 +- src/librustc_mir/borrow_check/move_errors.rs | 17 ++++--- .../borrow_check/mutability_errors.rs | 4 +- .../borrow_check/nll/explain_borrow/mod.rs | 3 +- 5 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index ce4460e8ded..83ae87bc166 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -275,11 +275,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { "report_move_out_while_borrowed: location={:?} place={:?} span={:?} borrow={:?}", location, place, span, borrow ); - let value_msg = match self.describe_place(place) { + let value_msg = match self.describe_place(place.as_place_ref()) { Some(name) => format!("`{}`", name), None => "value".to_owned(), }; - let borrow_msg = match self.describe_place(&borrow.borrowed_place) { + let borrow_msg = match self.describe_place(borrow.borrowed_place.as_place_ref()) { Some(name) => format!("`{}`", name), None => "value".to_owned(), }; @@ -292,7 +292,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let mut err = self.cannot_move_when_borrowed( span, - &self.describe_place(place).unwrap_or_else(|| "_".to_owned()), + &self.describe_place(place.as_place_ref()).unwrap_or_else(|| "_".to_owned()), ); err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_msg)); err.span_label(span, format!("move out of {} occurs here", value_msg)); @@ -331,15 +331,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let mut err = self.cannot_use_when_mutably_borrowed( span, - &self.describe_place(place).unwrap_or_else(|| "_".to_owned()), + &self.describe_place(place.as_place_ref()).unwrap_or_else(|| "_".to_owned()), borrow_span, - &self.describe_place(&borrow.borrowed_place) + &self.describe_place(borrow.borrowed_place.as_place_ref()) .unwrap_or_else(|| "_".to_owned()), ); borrow_spans.var_span_label(&mut err, { let place = &borrow.borrowed_place; - let desc_place = self.describe_place(place).unwrap_or_else(|| "_".to_owned()); + let desc_place = + self.describe_place(place.as_place_ref()).unwrap_or_else(|| "_".to_owned()); format!("borrow occurs due to use of `{}`{}", desc_place, borrow_spans.describe()) }); @@ -516,7 +517,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); } else { let borrow_place = &issued_borrow.borrowed_place; - let borrow_place_desc = self.describe_place(borrow_place) + let borrow_place_desc = self.describe_place(borrow_place.as_place_ref()) .unwrap_or_else(|| "_".to_owned()); issued_spans.var_span_label( &mut err, @@ -615,9 +616,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { while let Some(box Projection { base: base_proj, elem }) = current { match elem { ProjectionElem::Field(field, _) if union_ty(base, base_proj).is_some() => { - return Some((Place { - base: base.clone(), - projection: base_proj.clone(), + return Some((PlaceRef { + base: base, + projection: base_proj, }, field)); }, _ => current = base_proj, @@ -639,18 +640,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let ProjectionElem::Field(field, _) = elem { if let Some(union_ty) = union_ty(base, proj_base) { if field != target_field - && *base == target_base.base - && *proj_base == target_base.projection { + && base == target_base.base + && proj_base == target_base.projection { // FIXME when we avoid clone reuse describe_place closure - let describe_base_place = self.describe_place(&Place { - base: base.clone(), - projection: proj_base.clone(), + let describe_base_place = self.describe_place(PlaceRef { + base: base, + projection: proj_base, }).unwrap_or_else(|| "_".to_owned()); return Some(( describe_base_place, - describe_place(first_borrowed_place), - describe_place(second_borrowed_place), + describe_place(first_borrowed_place.as_place_ref()), + describe_place(second_borrowed_place.as_place_ref()), union_ty.to_string(), )); } @@ -665,7 +666,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // If we didn't find a field access into a union, or both places match, then // only return the description of the first place. ( - describe_place(first_borrowed_place), + describe_place(first_borrowed_place.as_place_ref()), "".to_string(), "".to_string(), "".to_string(), @@ -743,7 +744,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - let place_desc = self.describe_place(&borrow.borrowed_place); + let place_desc = self.describe_place(borrow.borrowed_place.as_place_ref()); let kind_place = kind.filter(|_| place_desc.is_some()).map(|k| (k, place_span.0)); let explanation = self.explain_why_borrow_contains_point(location, &borrow, kind_place); @@ -950,12 +951,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let mut err = self.cannot_borrow_across_destructor(borrow_span); - let what_was_dropped = match self.describe_place(place) { + let what_was_dropped = match self.describe_place(place.as_place_ref()) { Some(name) => format!("`{}`", name.as_str()), None => String::from("temporary value"), }; - let label = match self.describe_place(&borrow.borrowed_place) { + let label = match self.describe_place(borrow.borrowed_place.as_place_ref()) { Some(borrowed) => format!( "here, drop of {D} needs exclusive access to `{B}`, \ because the type `{T}` implements the `Drop` trait", @@ -1389,7 +1390,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let mut err = self.cannot_mutate_in_match_guard( span, loan_span, - &self.describe_place(place).unwrap_or_else(|| "_".to_owned()), + &self.describe_place(place.as_place_ref()).unwrap_or_else(|| "_".to_owned()), "assign", ); loan_spans.var_span_label( @@ -1405,7 +1406,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let mut err = self.cannot_assign_to_borrowed( span, loan_span, - &self.describe_place(place).unwrap_or_else(|| "_".to_owned()), + &self.describe_place(place.as_place_ref()).unwrap_or_else(|| "_".to_owned()), ); loan_spans.var_span_label( @@ -1465,8 +1466,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { is_user_variable: None, .. }) - | None => (self.describe_place(place), assigned_span), - Some(decl) => (self.describe_place(err_place), decl.source_info.span), + | None => (self.describe_place(place.as_place_ref()), assigned_span), + Some(decl) => (self.describe_place(err_place.as_place_ref()), decl.source_info.span), }; let mut err = self.cannot_reassign_immutable( diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index aa772e64aa2..06ef802a4cf 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -121,8 +121,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { /// End-user visible description of `place` if one can be found. If the /// place is a temporary for instance, None will be returned. - pub(super) fn describe_place(&self, place: &Place<'tcx>) -> Option { - self.describe_place_with_options(place.as_place_ref(), IncludingDowncast(false)) + pub(super) fn describe_place(&self, place_ref: PlaceRef<'cx, 'tcx>) -> Option { + self.describe_place_with_options(place_ref, IncludingDowncast(false)) } /// End-user visible description of `place` if one can be found. If the diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index d933932309e..cbeedd31730 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -277,7 +277,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { span: Span ) -> DiagnosticBuilder<'a> { let description = if place.projection.is_none() { - format!("static item `{}`", self.describe_place(place).unwrap()) + format!("static item `{}`", self.describe_place(place.as_place_ref()).unwrap()) } else { let mut base_static = &place.projection; while let Some(box Projection { base: Some(ref proj), .. }) = base_static { @@ -290,8 +290,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { format!( "`{:?}` as `{:?}` is a static item", - self.describe_place(place).unwrap(), - self.describe_place(&base_static).unwrap(), + self.describe_place(place.as_place_ref()).unwrap(), + self.describe_place(base_static.as_place_ref()).unwrap(), ) }; @@ -363,7 +363,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let upvar_name = upvar.name; let upvar_span = self.infcx.tcx.hir().span(upvar_hir_id); - let place_name = self.describe_place(move_place).unwrap(); + let place_name = self.describe_place(move_place.as_place_ref()).unwrap(); let place_description = if self .is_upvar_field_projection(move_place.as_place_ref()) @@ -392,7 +392,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } _ => { let source = self.borrowed_content_source(&deref_base); - match (self.describe_place(move_place), source.describe_for_named_place()) { + match ( + self.describe_place(move_place.as_place_ref()), + source.describe_for_named_place(), + ) { (Some(place_desc), Some(source_desc)) => { self.cannot_move_out_of( span, @@ -452,7 +455,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { if binds_to.is_empty() { let place_ty = move_from.ty(self.body, self.infcx.tcx).ty; - let place_desc = match self.describe_place(&move_from) { + let place_desc = match self.describe_place(move_from.as_place_ref()) { Some(desc) => format!("`{}`", desc), None => format!("value"), }; @@ -480,7 +483,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { GroupedMoveError::OtherIllegalMove { ref original_path, use_spans, .. } => { let span = use_spans.var_or_use(); let place_ty = original_path.ty(self.body, self.infcx.tcx).ty; - let place_desc = match self.describe_place(original_path) { + let place_desc = match self.describe_place(original_path.as_place_ref()) { Some(desc) => format!("`{}`", desc), None => format!("value"), }; diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index adde4ac85c8..cc9a31db98b 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -40,7 +40,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let item_msg; let reason; let mut opt_source = None; - let access_place_desc = self.describe_place(access_place); + let access_place_desc = self.describe_place(access_place.as_place_ref()); debug!("report_mutability_error: access_place_desc={:?}", access_place_desc); match the_place_err { @@ -236,7 +236,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { format!( "mutable borrow occurs due to use of `{}` in closure", // always Some() if the message is printed. - self.describe_place(access_place).unwrap_or_default(), + self.describe_place(access_place.as_place_ref()).unwrap_or_default(), ) ); borrow_span diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs index 6885baf8223..abb84c59d9b 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -304,7 +304,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { region, ); if let Some(region_name) = region_name { - let opt_place_desc = self.describe_place(&borrow.borrowed_place); + let opt_place_desc = + self.describe_place(borrow.borrowed_place.as_place_ref()); BorrowExplanation::MustBeValidFor { category, from_closure,