From 632c0af38ffec6644ec10746ee700f353f7f09b6 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 26 Mar 2020 01:55:16 +0100 Subject: [PATCH] borrowck diagnostics: address review comments. --- .../diagnostics/conflict_errors.rs | 28 +++++++++---------- .../borrow_check/diagnostics/mod.rs | 12 ++++++-- .../borrow_check/diagnostics/move_errors.rs | 8 +++--- .../diagnostics/mutability_errors.rs | 2 +- src/librustc_mir/util/borrowck_errors.rs | 7 ++--- 5 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs index 3848dd2ee3b4..8f18fb4a30ed 100644 --- a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs @@ -256,8 +256,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { "report_move_out_while_borrowed: location={:?} place={:?} span={:?} borrow={:?}", location, place, span, borrow ); - let value_msg = self.describe_place_str(place.as_ref()); - let borrow_msg = self.describe_place_str(borrow.borrowed_place.as_ref()); + let value_msg = self.describe_any_place(place.as_ref()); + let borrow_msg = self.describe_any_place(borrow.borrowed_place.as_ref()); let borrow_spans = self.retrieve_borrow_spans(borrow); let borrow_span = borrow_spans.args_or_use(); @@ -266,7 +266,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let span = move_spans.args_or_use(); let mut err = - self.cannot_move_when_borrowed(span, &self.describe_place_str(place.as_ref())); + self.cannot_move_when_borrowed(span, &self.describe_any_place(place.as_ref())); err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_msg)); err.span_label(span, format!("move out of {} occurs here", value_msg)); @@ -306,14 +306,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let mut err = self.cannot_use_when_mutably_borrowed( span, - &self.describe_place_str(place.as_ref()), + &self.describe_any_place(place.as_ref()), borrow_span, - &self.describe_place_str(borrow.borrowed_place.as_ref()), + &self.describe_any_place(borrow.borrowed_place.as_ref()), ); borrow_spans.var_span_label(&mut err, { let place = &borrow.borrowed_place; - let desc_place = self.describe_place_str(place.as_ref()); + let desc_place = self.describe_any_place(place.as_ref()); format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe()) }); @@ -506,7 +506,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); } else { let borrow_place = &issued_borrow.borrowed_place; - let borrow_place_desc = self.describe_place_str(borrow_place.as_ref()); + let borrow_place_desc = self.describe_any_place(borrow_place.as_ref()); issued_spans.var_span_label( &mut err, format!( @@ -647,12 +647,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { && proj_base == target_base.projection { return Some(( - self.describe_place_str(PlaceRef { + self.describe_any_place(PlaceRef { local, projection: proj_base, }), - self.describe_place_str(first_borrowed_place.as_ref()), - self.describe_place_str(second_borrowed_place.as_ref()), + self.describe_any_place(first_borrowed_place.as_ref()), + self.describe_any_place(second_borrowed_place.as_ref()), union_ty.to_string(), )); } @@ -665,7 +665,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. ( - self.describe_place_str(first_borrowed_place.as_ref()), + self.describe_any_place(first_borrowed_place.as_ref()), "".to_string(), "".to_string(), "".to_string(), @@ -1388,7 +1388,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let loan_spans = self.retrieve_borrow_spans(loan); let loan_span = loan_spans.args_or_use(); - let descr_place = self.describe_place_str(place.as_ref()); + let descr_place = self.describe_any_place(place.as_ref()); if loan.kind == BorrowKind::Shallow { if let Some(section) = self.classify_immutable_section(&loan.assigned_place) { let mut err = self.cannot_mutate_in_immutable_section( @@ -1463,8 +1463,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { }) | Some(LocalDecl { local_info: LocalInfo::StaticRef { .. }, .. }) | Some(LocalDecl { local_info: LocalInfo::Other, .. }) - | None => (self.describe_place_str(place.as_ref()), assigned_span), - Some(decl) => (self.describe_place_str(err_place.as_ref()), decl.source_info.span), + | None => (self.describe_any_place(place.as_ref()), assigned_span), + Some(decl) => (self.describe_any_place(err_place.as_ref()), decl.source_info.span), }; let mut err = self.cannot_reassign_immutable(span, &place_description, from_arg); diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index e5850d642b53..605093d8acad 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -138,10 +138,16 @@ 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, `value` will be returned. - pub(super) fn describe_place_str(&self, place_ref: PlaceRef<'tcx>) -> String { + /// If the place is a temporary for instance, `"value"` will be returned. + pub(super) fn describe_any_place(&self, place_ref: PlaceRef<'tcx>) -> String { match self.describe_place(place_ref) { - Some(descr) => format!("`{}`", descr), + Some(mut descr) => { + // Surround descr with `backticks`. + descr.reserve(2); + descr.insert_str(0, "`"); + descr.push_str("`"); + descr + } None => "value".to_string(), } } diff --git a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs index 6146b3abc9cc..2cdc1ced0bbe 100644 --- a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs @@ -272,14 +272,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { span: Span, ) -> DiagnosticBuilder<'a> { let description = if place.projection.len() == 1 { - format!("static item {}", self.describe_place_str(place.as_ref())) + format!("static item {}", self.describe_any_place(place.as_ref())) } else { let base_static = PlaceRef { local: place.local, projection: &[ProjectionElem::Deref] }; format!( "{} as {} is a static item", - self.describe_place_str(place.as_ref()), - self.describe_place_str(base_static), + self.describe_any_place(place.as_ref()), + self.describe_any_place(base_static), ) }; @@ -349,7 +349,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_str(move_place.as_ref()); + let place_name = self.describe_any_place(move_place.as_ref()); let place_description = if self.is_upvar_field_projection(move_place.as_ref()).is_some() { diff --git a/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs b/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs index e6c25c053a26..f224041270dc 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs @@ -170,7 +170,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { &mut err, format!( "mutable borrow occurs due to use of {} in closure", - self.describe_place_str(access_place.as_ref()), + self.describe_any_place(access_place.as_ref()), ), ); borrow_span diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index b39f998ec29a..6e6bbabd35b3 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -53,7 +53,7 @@ impl<'cx, 'tcx> crate::borrow_check::MirBorrowckCtxt<'cx, 'tcx> { old_load_end_span: Option, ) -> DiagnosticBuilder<'cx> { let via = - |msg: &str| if msg.is_empty() { msg.to_string() } else { format!(" (via {})", msg) }; + |msg: &str| if msg.is_empty() { "".to_string() } else { format!(" (via {})", msg) }; let mut err = struct_span_err!( self, new_loan_span, @@ -201,13 +201,12 @@ impl<'cx, 'tcx> crate::borrow_check::MirBorrowckCtxt<'cx, 'tcx> { old_load_end_span: Option, ) -> DiagnosticBuilder<'cx> { let via = - |msg: &str| if msg.is_empty() { msg.to_string() } else { format!(" (via {})", msg) }; + |msg: &str| if msg.is_empty() { "".to_string() } else { format!(" (via {})", msg) }; let mut err = struct_span_err!( self, span, E0502, - "cannot borrow {}{} as {} because {} is also borrowed \ - as {}{}", + "cannot borrow {}{} as {} because {} is also borrowed as {}{}", desc_new, via(msg_new), kind_new,