Skip to content

Commit

Permalink
Auto merge of #75302 - Aaron1011:feature/partial-move-diag, r=estebank
Browse files Browse the repository at this point in the history
Be consistent when describing a move as a 'partial' in diagnostics

When an error occurs due to a partial move, we would use the world
"partial" in some parts of the error message, but not in others. This
commit ensures that we use the word 'partial' in either all or none of
the diagnostic messages.

Additionally, we no longer describe a move out of a `Box` via `*` as
a 'partial move'. This was a pre-existing issue, but became more
noticable when the word 'partial' is used in more places.
  • Loading branch information
bors committed Aug 25, 2020
2 parents 8ba2250 + 5f7436b commit bf43421
Show file tree
Hide file tree
Showing 31 changed files with 300 additions and 257 deletions.
77 changes: 56 additions & 21 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,32 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

let msg = ""; //FIXME: add "partially " or "collaterally "
let is_partial_move = move_site_vec.iter().any(|move_site| {
let move_out = self.move_data.moves[(*move_site).moi];
let moved_place = &self.move_data.move_paths[move_out.path].place;
// `*(_1)` where `_1` is a `Box` is actually a move out.
let is_box_move = moved_place.as_ref().projection == &[ProjectionElem::Deref]
&& self.body.local_decls[moved_place.local].ty.is_box();

!is_box_move
&& used_place != moved_place.as_ref()
&& used_place.is_prefix_of(moved_place.as_ref())
});

let partial_str = if is_partial_move { "partial " } else { "" };
let partially_str = if is_partial_move { "partially " } else { "" };

let mut err = self.cannot_act_on_moved_value(
span,
desired_action.as_noun(),
msg,
partially_str,
self.describe_place_with_options(moved_place, IncludingDowncast(true)),
);

self.add_moved_or_invoked_closure_note(location, used_place, &mut err);

let mut is_loop_move = false;
let is_partial_move = move_site_vec.iter().any(|move_site| {
let move_out = self.move_data.moves[(*move_site).moi];
let moved_place = &self.move_data.move_paths[move_out.path].place;
used_place != moved_place.as_ref() && used_place.is_prefix_of(moved_place.as_ref())
});

for move_site in &move_site_vec {
let move_out = self.move_data.moves[(*move_site).moi];
let moved_place = &self.move_data.move_paths[move_out.path].place;
Expand All @@ -142,13 +151,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if location == move_out.source {
err.span_label(
span,
format!("value moved{} here, in previous iteration of loop", move_msg),
format!(
"value {}moved{} here, in previous iteration of loop",
partially_str, move_msg
),
);
is_loop_move = true;
} else if move_site.traversed_back_edge {
err.span_label(
move_span,
format!("value moved{} here, in previous iteration of loop", move_msg),
format!(
"value {}moved{} here, in previous iteration of loop",
partially_str, move_msg
),
);
} else {
if let UseSpans::FnSelfUse { var_span, fn_call_span, fn_span, kind } =
Expand All @@ -162,7 +177,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
FnSelfUseKind::FnOnceCall => {
err.span_label(
fn_call_span,
&format!("{} moved due to this call", place_name),
&format!(
"{} {}moved due to this call",
place_name, partially_str
),
);
err.span_note(
var_span,
Expand All @@ -172,7 +190,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
FnSelfUseKind::Operator { self_arg } => {
err.span_label(
fn_call_span,
&format!("{} moved due to usage in operator", place_name),
&format!(
"{} {}moved due to usage in operator",
place_name, partially_str
),
);
if self.fn_self_span_reported.insert(fn_span) {
err.span_note(
Expand All @@ -186,14 +207,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
err.span_label(
fn_call_span,
&format!(
"{} moved due to this implicit call to `.into_iter()`",
place_name
"{} {}moved due to this implicit call to `.into_iter()`",
place_name, partially_str
),
);
} else {
err.span_label(
fn_call_span,
&format!("{} moved due to this method call", place_name),
&format!(
"{} {}moved due to this method call",
place_name, partially_str
),
);
}
// Avoid pointing to the same function in multiple different
Expand All @@ -207,10 +231,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
} else {
err.span_label(move_span, format!("value moved{} here", move_msg));
err.span_label(
move_span,
format!("value {}moved{} here", partially_str, move_msg),
);
move_spans.var_span_label(
&mut err,
format!("variable moved due to use{}", move_spans.describe()),
format!(
"variable {}moved due to use{}",
partially_str,
move_spans.describe()
),
);
}
}
Expand Down Expand Up @@ -250,9 +281,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
err.span_label(
span,
format!(
"value {} here {}",
"value {} here after {}move",
desired_action.as_verb_in_past_tense(),
if is_partial_move { "after partial move" } else { "after move" },
partial_str
),
);
}
Expand Down Expand Up @@ -321,7 +352,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} else {
None
};
self.note_type_does_not_implement_copy(&mut err, &note_msg, ty, span);
self.note_type_does_not_implement_copy(&mut err, &note_msg, ty, span, partial_str);
}

if let Some((_, mut old_err)) =
Expand Down Expand Up @@ -1398,8 +1429,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

for moi in &self.move_data.loc_map[location] {
debug!("report_use_of_moved_or_uninitialized: moi={:?}", moi);
if mpis.contains(&self.move_data.moves[*moi].path) {
debug!("report_use_of_moved_or_uninitialized: found");
let path = self.move_data.moves[*moi].path;
if mpis.contains(&path) {
debug!(
"report_use_of_moved_or_uninitialized: found {:?}",
move_paths[path].place
);
result.push(MoveSite { moi: *moi, traversed_back_edge: is_back_edge });

// Strictly speaking, we could continue our DFS here. There may be
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_mir/borrow_check/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
place_desc: &str,
ty: Ty<'tcx>,
span: Option<Span>,
move_prefix: &str,
) {
let message = format!(
"move occurs because {} has type `{}`, which does not implement the `Copy` trait",
place_desc, ty,
"{}move occurs because {} has type `{}`, which does not implement the `Copy` trait",
move_prefix, place_desc, ty,
);
if let Some(span) = span {
err.span_label(span, message);
Expand Down
11 changes: 9 additions & 2 deletions src/librustc_mir/borrow_check/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
None => "value".to_string(),
};

self.note_type_does_not_implement_copy(err, &place_desc, place_ty, Some(span));
self.note_type_does_not_implement_copy(
err,
&place_desc,
place_ty,
Some(span),
"",
);
} else {
binds_to.sort();
binds_to.dedup();
Expand All @@ -467,7 +473,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
Some(desc) => format!("`{}`", desc),
None => "value".to_string(),
};
self.note_type_does_not_implement_copy(err, &place_desc, place_ty, Some(span));
self.note_type_does_not_implement_copy(err, &place_desc, place_ty, Some(span), "");

use_spans.args_span_label(err, format!("move out of {} occurs here", place_desc));
use_spans
Expand Down Expand Up @@ -529,6 +535,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
&format!("`{}`", self.local_names[*local].unwrap()),
bind_to.ty,
Some(binding_span),
"",
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ impl<S> Ia<S> {

async fn crash(self) {
Self::partial(self.0);
Self::full(self); //~ ERROR use of moved value: `self`
Self::full(self); //~ ERROR use of partially moved value: `self`
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
error[E0382]: use of moved value: `self`
error[E0382]: use of partially moved value: `self`
--> $DIR/issue-66958-non-copy-infered-type-arg.rs:11:20
|
LL | Self::partial(self.0);
| ------ value moved here
| ------ value partially moved here
LL | Self::full(self);
| ^^^^ value used here after partial move
|
= note: move occurs because `self.0` has type `S`, which does not implement the `Copy` trait
= note: partial move occurs because `self.0` has type `S`, which does not implement the `Copy` trait

error: aborting due to previous error

Expand Down
24 changes: 12 additions & 12 deletions src/test/ui/binding/issue-53114-borrow-checks.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,26 @@ LL | drop(m);
LL | match m { _ => { } } // #53114: should eventually be accepted too
| ^ value used here after move

error[E0382]: use of moved value: `mm`
error[E0382]: use of partially moved value: `mm`
--> $DIR/issue-53114-borrow-checks.rs:27:11
|
LL | match mm { (_x, _) => { } }
| -- value moved here
| -- value partially moved here
LL | match mm { (_, _y) => { } }
| ^^ value used here after partial move
|
= note: move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait
= note: partial move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `mm`
error[E0382]: use of partially moved value: `mm`
--> $DIR/issue-53114-borrow-checks.rs:29:11
|
LL | match mm { (_, _y) => { } }
| -- value moved here
| -- value partially moved here
LL |
LL | match mm { (_, _) => { } }
| ^^ value used here after partial move
|
= note: move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait
= note: partial move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `m`
--> $DIR/issue-53114-borrow-checks.rs:36:16
Expand All @@ -39,26 +39,26 @@ LL | drop(m);
LL | if let _ = m { } // #53114: should eventually be accepted too
| ^ value used here after move

error[E0382]: use of moved value: `mm`
error[E0382]: use of partially moved value: `mm`
--> $DIR/issue-53114-borrow-checks.rs:41:22
|
LL | if let (_x, _) = mm { }
| -- value moved here
| -- value partially moved here
LL | if let (_, _y) = mm { }
| ^^ value used here after partial move
|
= note: move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait
= note: partial move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `mm`
error[E0382]: use of partially moved value: `mm`
--> $DIR/issue-53114-borrow-checks.rs:43:21
|
LL | if let (_, _y) = mm { }
| -- value moved here
| -- value partially moved here
LL |
LL | if let (_, _) = mm { }
| ^^ value used here after partial move
|
= note: move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait
= note: partial move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait

error: aborting due to 6 previous errors

Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/borrowck/borrowck-move-out-from-array-match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn move_out_from_begin_field_and_end() {
[_, _, (_x, _)] => {}
}
match a {
[.., _y] => {} //~ ERROR use of moved value
[.., _y] => {} //~ ERROR use of partially moved value
}
}

Expand All @@ -42,7 +42,7 @@ fn move_out_by_const_index_and_subslice() {
[_x, _, _] => {}
}
match a {
//~^ ERROR use of moved value
//~^ ERROR use of partially moved value
[_y @ .., _, _] => {}
}
}
Expand All @@ -53,7 +53,7 @@ fn move_out_by_const_index_end_and_subslice() {
[.., _x] => {}
}
match a {
//~^ ERROR use of moved value
//~^ ERROR use of partially moved value
[_, _, _y @ ..] => {}
}
}
Expand All @@ -64,7 +64,7 @@ fn move_out_by_const_index_field_and_subslice() {
[(_x, _), _, _] => {}
}
match a {
//~^ ERROR use of moved value
//~^ ERROR use of partially moved value
[_y @ .., _, _] => {}
}
}
Expand All @@ -75,7 +75,7 @@ fn move_out_by_const_index_end_field_and_subslice() {
[.., (_x, _)] => {}
}
match a {
//~^ ERROR use of moved value
//~^ ERROR use of partially moved value
[_, _, _y @ ..] => {}
}
}
Expand Down Expand Up @@ -108,7 +108,7 @@ fn move_out_by_subslice_and_subslice() {
[x @ .., _] => {}
}
match a {
//~^ ERROR use of moved value
//~^ ERROR use of partially moved value
[_, _y @ ..] => {}
}
}
Expand Down
Loading

0 comments on commit bf43421

Please sign in to comment.