Skip to content

Commit

Permalink
Rollup merge of rust-lang#70389 - Centril:borrowck-no-underscores, r=…
Browse files Browse the repository at this point in the history 
…mark-i-m

borrowck: prefer "value" over "`_`" in diagnostics

Fixes rust-lang#67565.

r? @pnkfelix @matthewjasper
cc @mark-i-m
  • Loading branch information
@Centril
Centril committed Mar 26, 2020
2 parents ca7dfb1 + 632c0af commit 7db4825
Show file tree
Hide file tree
Showing 13 changed files with 119 additions and 135 deletions.
89 changes: 31 additions & 58 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,25 +256,17 @@ 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.as_ref()) {
Some(name) => format!("`{}`", name),
None => "value".to_owned(),
};
let borrow_msg = match self.describe_place(borrow.borrowed_place.as_ref()) {
Some(name) => format!("`{}`", name),
None => "value".to_owned(),
};
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();

let move_spans = self.move_spans(place.as_ref(), location);
let span = move_spans.args_or_use();

let mut err = self.cannot_move_when_borrowed(
span,
&self.describe_place(place.as_ref()).unwrap_or_else(|| "_".to_owned()),
);
let mut err =
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));

Expand Down Expand Up @@ -314,16 +306,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

let mut err = self.cannot_use_when_mutably_borrowed(
span,
&self.describe_place(place.as_ref()).unwrap_or_else(|| "_".to_owned()),
&self.describe_any_place(place.as_ref()),
borrow_span,
&self.describe_place(borrow.borrowed_place.as_ref()).unwrap_or_else(|| "_".to_owned()),
&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(place.as_ref()).unwrap_or_else(|| "_".to_owned());

format!("borrow occurs due to use of `{}`{}", desc_place, borrow_spans.describe())
let desc_place = self.describe_any_place(place.as_ref());
format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe())
});

self.explain_why_borrow_contains_point(location, borrow, None)
Expand Down Expand Up @@ -433,7 +424,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
borrow_spans.var_span_label(
&mut err,
format!(
"borrow occurs due to use of `{}`{}",
"borrow occurs due to use of {}{}",
desc_place,
borrow_spans.describe(),
),
Expand Down Expand Up @@ -511,16 +502,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if issued_spans == borrow_spans {
borrow_spans.var_span_label(
&mut err,
format!("borrows occur due to use of `{}`{}", desc_place, borrow_spans.describe()),
format!("borrows occur due to use of {}{}", desc_place, borrow_spans.describe()),
);
} else {
let borrow_place = &issued_borrow.borrowed_place;
let borrow_place_desc =
self.describe_place(borrow_place.as_ref()).unwrap_or_else(|| "_".to_owned());
let borrow_place_desc = self.describe_any_place(borrow_place.as_ref());
issued_spans.var_span_label(
&mut err,
format!(
"first borrow occurs due to use of `{}`{}",
"first borrow occurs due to use of {}{}",
borrow_place_desc,
issued_spans.describe(),
),
Expand All @@ -529,7 +519,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
borrow_spans.var_span_label(
&mut err,
format!(
"second borrow occurs due to use of `{}`{}",
"second borrow occurs due to use of {}{}",
desc_place,
borrow_spans.describe(),
),
Expand All @@ -538,7 +528,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

if union_type_name != "" {
err.note(&format!(
"`{}` is a field of the union `{}`, so it overlaps the field `{}`",
"{} is a field of the union `{}`, so it overlaps the field {}",
msg_place, union_type_name, msg_borrow,
));
}
Expand Down Expand Up @@ -606,7 +596,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let ty = Place::ty_from(place_base, place_projection, *self.body, self.infcx.tcx).ty;
ty.ty_adt_def().filter(|adt| adt.is_union()).map(|_| ty)
};
let describe_place = |place| self.describe_place(place).unwrap_or_else(|| "_".to_owned());

// Start with an empty tuple, so we can use the functions on `Option` to reduce some
// code duplication (particularly around returning an empty description in the failure
Expand Down Expand Up @@ -645,30 +634,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
.and_then(|(target_base, target_field)| {
// With the place of a union and a field access into it, we traverse the second
// borrowed place and look for a access to a different field of the same union.
let Place { local, projection } = second_borrowed_place;
let Place { local, ref projection } = *second_borrowed_place;

let mut cursor = &projection[..];
while let [proj_base @ .., elem] = cursor {
cursor = proj_base;

if let ProjectionElem::Field(field, _) = elem {
if let Some(union_ty) = union_ty(*local, proj_base) {
if let Some(union_ty) = union_ty(local, proj_base) {
if field != target_field
&& *local == target_base.local
&& local == target_base.local
&& proj_base == target_base.projection
{
// FIXME when we avoid clone reuse describe_place closure
let describe_base_place = self
.describe_place(PlaceRef {
local: *local,
projection: proj_base,
})
.unwrap_or_else(|| "_".to_owned());

return Some((
describe_base_place,
describe_place(first_borrowed_place.as_ref()),
describe_place(second_borrowed_place.as_ref()),
self.describe_any_place(PlaceRef {
local,
projection: proj_base,
}),
self.describe_any_place(first_borrowed_place.as_ref()),
self.describe_any_place(second_borrowed_place.as_ref()),
union_ty.to_string(),
));
}
Expand All @@ -681,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.
(
describe_place(first_borrowed_place.as_ref()),
self.describe_any_place(first_borrowed_place.as_ref()),
"".to_string(),
"".to_string(),
"".to_string(),
Expand Down Expand Up @@ -1404,12 +1388,13 @@ 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_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(
span,
loan_span,
&self.describe_place(place.as_ref()).unwrap_or_else(|| "_".to_owned()),
&descr_place,
section,
"assign",
);
Expand All @@ -1424,11 +1409,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

let mut err = self.cannot_assign_to_borrowed(
span,
loan_span,
&self.describe_place(place.as_ref()).unwrap_or_else(|| "_".to_owned()),
);
let mut err = self.cannot_assign_to_borrowed(span, loan_span, &descr_place);

loan_spans
.var_span_label(&mut err, format!("borrow occurs due to use{}", loan_spans.describe()));
Expand Down Expand Up @@ -1482,27 +1463,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
})
| Some(LocalDecl { local_info: LocalInfo::StaticRef { .. }, .. })
| Some(LocalDecl { local_info: LocalInfo::Other, .. })
| None => (self.describe_place(place.as_ref()), assigned_span),
Some(decl) => (self.describe_place(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.as_ref().map(AsRef::as_ref).unwrap_or("_"),
from_arg,
);
let mut err = self.cannot_reassign_immutable(span, &place_description, from_arg);
let msg = if from_arg {
"cannot assign to immutable argument"
} else {
"cannot assign twice to immutable variable"
};
if span != assigned_span {
if !from_arg {
let value_msg = match place_description {
Some(name) => format!("`{}`", name),
None => "value".to_owned(),
};
err.span_label(assigned_span, format!("first assignment to {}", value_msg));
err.span_label(assigned_span, format!("first assignment to {}", place_description));
}
}
if let Some(decl) = local_decl {
Expand Down
19 changes: 17 additions & 2 deletions src/librustc_mir/borrow_check/diagnostics/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,23 @@ 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.
/// 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_any_place(&self, place_ref: PlaceRef<'tcx>) -> String {
match self.describe_place(place_ref) {
Some(mut descr) => {
// Surround descr with `backticks`.
descr.reserve(2);
descr.insert_str(0, "`");
descr.push_str("`");
descr
}
None => "value".to_string(),
}
}

/// 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_ref: PlaceRef<'tcx>) -> Option<String> {
self.describe_place_with_options(place_ref, IncludingDowncast(false))
}
Expand Down
24 changes: 11 additions & 13 deletions src/librustc_mir/borrow_check/diagnostics/move_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(place.as_ref()).unwrap())
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(place.as_ref()).unwrap(),
self.describe_place(base_static).unwrap(),
"{} as {} is a static item",
self.describe_any_place(place.as_ref()),
self.describe_any_place(base_static),
)
};

Expand Down Expand Up @@ -349,16 +349,14 @@ 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.as_ref()).unwrap();
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()
{
format!("`{}`, a {}", place_name, capture_description)
} else {
format!("`{}`, as `{}` is a {}", place_name, upvar_name, capture_description,)
};
let place_description =
if self.is_upvar_field_projection(move_place.as_ref()).is_some() {
format!("{}, a {}", place_name, capture_description)
} else {
format!("{}, as `{}` is a {}", place_name, upvar_name, capture_description)
};

debug!(
"report: closure_kind_ty={:?} closure_kind={:?} place_description={:?}",
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
borrow_spans.var_span_label(
&mut err,
format!(
"mutable borrow occurs due to use of `{}` in closure",
// always Some() if the message is printed.
self.describe_place(access_place.as_ref()).unwrap_or_default(),
"mutable borrow occurs due to use of {} in closure",
self.describe_any_place(access_place.as_ref()),
),
);
borrow_span
Expand Down
Loading

0 comments on commit 7db4825

Please sign in to comment.