Auto merge of #52359 - matthewjasper:combine-move-error-reporting, r=pnkfelix

[NLL] Small move error reporting improvements

* Use a MirBorrowckContext when reporting errors to be more uniform with other error reporting
* Add a special message for the case of trying to move from capture variables in `Fn` and `FnMut` closures.

part of #51028
This commit is contained in:
bors 2018-07-22 08:52:05 +00:00
commit aeca042f84
12 changed files with 101 additions and 88 deletions

View File

@ -34,7 +34,7 @@ use std::rc::Rc;
use syntax_pos::Span;
use dataflow::indexes::BorrowIndex;
use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex};
use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError, MovePathIndex};
use dataflow::Borrows;
use dataflow::DataflowResultsConsumer;
use dataflow::FlowAtLocation;
@ -148,13 +148,11 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
let mir = &mir; // no further changes
let location_table = &LocationTable::new(mir);
let move_data: MoveData<'tcx> = match MoveData::gather_moves(mir, tcx) {
Ok(move_data) => move_data,
Err((move_data, move_errors)) => {
move_errors::report_move_errors(&mir, tcx, move_errors, &move_data);
move_data
}
};
let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<MoveError<'tcx>>>) =
match MoveData::gather_moves(mir, tcx) {
Ok(move_data) => (move_data, None),
Err((move_data, move_errors)) => (move_data, Some(move_errors)),
};
let mdpe = MoveDataParamEnv {
move_data: move_data,
@ -271,6 +269,9 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
polonius_output,
);
if let Some(errors) = move_errors {
mbcx.report_move_errors(errors);
}
mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer
// For each non-user used mutable variable, check if it's been assigned from
@ -1975,7 +1976,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
ProjectionElem::Field(field, _ty) => {
let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
if base_ty.is_closure() || base_ty.is_generator() {
if base_ty.is_closure() || base_ty.is_generator() {
Some(field)
} else {
None

View File

@ -10,34 +10,16 @@
use rustc::hir;
use rustc::mir::*;
use rustc::ty::{self, TyCtxt};
use rustc::ty;
use rustc_data_structures::indexed_vec::Idx;
use rustc_errors::DiagnosticBuilder;
use syntax_pos::Span;
use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind, MoveData};
use borrow_check::MirBorrowckCtxt;
use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind};
use dataflow::move_paths::{LookupResult, MoveError, MovePathIndex};
use util::borrowck_errors::{BorrowckErrors, Origin};
pub(crate) fn report_move_errors<'gcx, 'tcx>(
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, 'gcx, 'tcx>,
move_errors: Vec<MoveError<'tcx>>,
move_data: &MoveData<'tcx>,
) {
MoveErrorCtxt {
mir,
tcx,
move_data,
}.report_errors(move_errors);
}
#[derive(Copy, Clone)]
struct MoveErrorCtxt<'a, 'gcx: 'tcx, 'tcx: 'a> {
mir: &'a Mir<'tcx>,
tcx: TyCtxt<'a, 'gcx, 'tcx>,
move_data: &'a MoveData<'tcx>,
}
// Often when desugaring a pattern match we may have many individual moves in
// MIR that are all part of one operation from the user's point-of-view. For
// example:
@ -76,15 +58,15 @@ enum GroupedMoveError<'tcx> {
},
}
impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
fn report_errors(self, move_errors: Vec<MoveError<'tcx>>) {
impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
pub(crate) fn report_move_errors(&self, move_errors: Vec<MoveError<'tcx>>) {
let grouped_errors = self.group_move_errors(move_errors);
for error in grouped_errors {
self.report(error);
}
}
fn group_move_errors(self, errors: Vec<MoveError<'tcx>>) -> Vec<GroupedMoveError<'tcx>> {
fn group_move_errors(&self, errors: Vec<MoveError<'tcx>>) -> Vec<GroupedMoveError<'tcx>> {
let mut grouped_errors = Vec::new();
for error in errors {
self.append_to_grouped_errors(&mut grouped_errors, error);
@ -93,7 +75,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
}
fn append_to_grouped_errors(
self,
&self,
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
error: MoveError<'tcx>,
) {
@ -114,19 +96,19 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
.map(|stmt| &stmt.kind)
{
let local_decl = &self.mir.local_decls[*local];
// opt_match_place is the
// match_span is the span of the expression being matched on
// match *x.y { ... } match_place is Some(*x.y)
// ^^^^ match_span is the span of *x.y
//
// opt_match_place is None for let [mut] x = ... statements,
// whether or not the right-hand side is a place expression
if let Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
opt_match_place: Some((ref opt_match_place, match_span)),
binding_mode: _,
opt_ty_info: _,
}))) = local_decl.is_user_variable
{
// opt_match_place is the
// match_span is the span of the expression being matched on
// match *x.y { ... } match_place is Some(*x.y)
// ^^^^ match_span is the span of *x.y
// opt_match_place is None for let [mut] x = ... statements,
// whether or not the right-hand side is a place expression
// HACK use scopes to determine if this assignment is
// the initialization of a variable.
// FIXME(matthewjasper) This would probably be more
@ -145,8 +127,8 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
opt_match_place,
match_span,
);
return;
}
return;
}
}
grouped_errors.push(GroupedMoveError::OtherIllegalMove {
@ -158,7 +140,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
}
fn append_binding_error(
self,
&self,
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
kind: IllegalMoveOriginKind<'tcx>,
move_from: &Place<'tcx>,
@ -236,7 +218,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
};
}
fn report(self, error: GroupedMoveError<'tcx>) {
fn report(&self, error: GroupedMoveError<'tcx>) {
let (mut err, err_span) = {
let (span, kind): (Span, &IllegalMoveOriginKind) = match error {
GroupedMoveError::MovesFromMatchPlace { span, ref kind, .. }
@ -249,14 +231,43 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
IllegalMoveOriginKind::Static => {
self.tcx.cannot_move_out_of(span, "static item", origin)
}
IllegalMoveOriginKind::BorrowedContent { target_ty: ty } => {
IllegalMoveOriginKind::BorrowedContent { target_place: place } => {
// Inspect the type of the content behind the
// borrow to provide feedback about why this
// was a move rather than a copy.
let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx);
match ty.sty {
ty::TyArray(..) | ty::TySlice(..) => self
.tcx
.cannot_move_out_of_interior_noncopy(span, ty, None, origin),
ty::TyClosure(def_id, closure_substs)
if !self.mir.upvar_decls.is_empty()
&& {
match place {
Place::Projection(ref proj) => {
proj.base == Place::Local(Local::new(1))
}
Place::Local(_) | Place::Static(_) => unreachable!(),
}
} =>
{
let closure_kind_ty =
closure_substs.closure_kind_ty(def_id, self.tcx);
let closure_kind = closure_kind_ty.to_opt_closure_kind();
let place_description = match closure_kind {
Some(ty::ClosureKind::Fn) => {
"captured variable in an `Fn` closure"
}
Some(ty::ClosureKind::FnMut) => {
"captured variable in an `FnMut` closure"
}
Some(ty::ClosureKind::FnOnce) => {
bug!("closure kind does not match first argument type")
}
None => bug!("closure kind not inferred by borrowck"),
};
self.tcx.cannot_move_out_of(span, place_description, origin)
}
_ => self
.tcx
.cannot_move_out_of(span, "borrowed content", origin),
@ -279,7 +290,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
}
fn add_move_hints(
self,
&self,
error: GroupedMoveError<'tcx>,
err: &mut DiagnosticBuilder<'a>,
span: Span,
@ -365,7 +376,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
}
}
fn suitable_to_remove_deref(self, proj: &PlaceProjection<'tcx>, snippet: &str) -> bool {
fn suitable_to_remove_deref(&self, proj: &PlaceProjection<'tcx>, snippet: &str) -> bool {
let is_shared_ref = |ty: ty::Ty| match ty.sty {
ty::TypeVariants::TyRef(.., hir::Mutability::MutImmutable) => true,
_ => false,

View File

@ -132,11 +132,11 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
let mir = self.builder.mir;
let tcx = self.builder.tcx;
let place_ty = proj.base.ty(mir, tcx).to_ty(tcx);
match place_ty.sty {
match place_ty.sty {
ty::TyRef(..) | ty::TyRawPtr(..) =>
return Err(MoveError::cannot_move_out_of(
self.loc,
BorrowedContent { target_ty: place.ty(mir, tcx).to_ty(tcx) })),
BorrowedContent { target_place: place.clone() })),
ty::TyAdt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() =>
return Err(MoveError::cannot_move_out_of(self.loc,
InteriorOfTypeWithDestructor {

View File

@ -282,9 +282,9 @@ pub(crate) enum IllegalMoveOriginKind<'tcx> {
/// Illegal move due to attempt to move from behind a reference.
BorrowedContent {
/// The content's type: if erroneous code was trying to move
/// from `*x` where `x: &T`, then this will be `T`.
target_ty: ty::Ty<'tcx>,
/// The place the reference refers to: if erroneous code was trying to
/// move from `(*x).f` this will be `*x`.
target_place: Place<'tcx>,
},
/// Illegal move due to attempt to move from field of an ADT that

View File

@ -1,8 +1,8 @@
error[E0507]: cannot move out of borrowed content
error[E0507]: cannot move out of captured variable in an `Fn` closure
--> $DIR/borrowck-in-static.rs:15:17
|
LL | Box::new(|| x) //~ ERROR cannot move out of captured outer variable
| ^ cannot move out of borrowed content
| ^ cannot move out of captured variable in an `Fn` closure
error: aborting due to previous error

View File

@ -1,8 +1,8 @@
error[E0507]: cannot move out of borrowed content
error[E0507]: cannot move out of captured variable in an `Fn` closure
--> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:21:9
|
LL | y.into_iter();
| ^ cannot move out of borrowed content
| ^ cannot move out of captured variable in an `Fn` closure
error: aborting due to previous error

View File

@ -1,16 +0,0 @@
error[E0507]: cannot move out of borrowed content
--> $DIR/E0161.rs:14:28
|
LL | let _x: Box<str> = box *"hello"; //~ ERROR E0161
| ^^^^^^^^ cannot move out of borrowed content
error[E0161]: cannot move a value of type str: the size of str cannot be statically determined
--> $DIR/E0161.rs:14:28
|
LL | let _x: Box<str> = box *"hello"; //~ ERROR E0161
| ^^^^^^^^
error: aborting due to 2 previous errors
Some errors occurred: E0161, E0507.
For more information about an error, try `rustc --explain E0161`.

View File

@ -1,8 +1,27 @@
error: internal compiler error: Accessing `(*_8)` with the kind `Write(Move)` shouldn't be possible
error[E0507]: cannot move out of borrowed content
--> $DIR/issue-20801.rs:36:22
|
LL | let a = unsafe { *mut_ref() };
| ^^^^^^^^^^ cannot move out of borrowed content
error[E0507]: cannot move out of borrowed content
--> $DIR/issue-20801.rs:39:22
|
LL | let b = unsafe { *imm_ref() };
| ^^^^^^^^^^ cannot move out of borrowed content
error[E0507]: cannot move out of borrowed content
--> $DIR/issue-20801.rs:42:22
|
LL | let c = unsafe { *mut_ptr() };
| ^^^^^^^^^^ cannot move out of borrowed content
error[E0507]: cannot move out of borrowed content
--> $DIR/issue-20801.rs:45:22
|
LL | let d = unsafe { *const_ptr() };
| ^^^^^^^^^^^^
| ^^^^^^^^^^^^ cannot move out of borrowed content
error: aborting due to previous error
error: aborting due to 4 previous errors
For more information about this error, try `rustc --explain E0507`.

View File

@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-test currently ICEs when using NLL (#52416)
// We used to ICE when moving out of a `*mut T` or `*const T`.
struct T(u8);

View File

@ -1,9 +1,3 @@
error[E0508]: cannot move out of type `[u8]`, a non-copy slice
--> $DIR/issue-30355.rs:15:8
|
LL | &X(*Y)
| ^^ cannot move out of here
error[E0161]: cannot move a value of type X: the size of X cannot be statically determined
--> $DIR/issue-30355.rs:15:6
|
@ -16,6 +10,12 @@ error[E0161]: cannot move a value of type [u8]: the size of [u8] cannot be stati
LL | &X(*Y)
| ^^
error[E0508]: cannot move out of type `[u8]`, a non-copy slice
--> $DIR/issue-30355.rs:15:8
|
LL | &X(*Y)
| ^^ cannot move out of here
error: aborting due to 3 previous errors
Some errors occurred: E0161, E0508.

View File

@ -1,8 +1,8 @@
error[E0507]: cannot move out of borrowed content
error[E0507]: cannot move out of captured variable in an `FnMut` closure
--> $DIR/issue-4335.rs:16:20
|
LL | id(Box::new(|| *v))
| ^^ cannot move out of borrowed content
| ^^ cannot move out of captured variable in an `FnMut` closure
error[E0597]: `v` does not live long enough
--> $DIR/issue-4335.rs:16:17

View File

@ -28,11 +28,11 @@ LL | fn test4(f: &Test) {
LL | f.f.call_mut(())
| ^^^ `f` is a `&` reference, so the data it refers to cannot be borrowed as mutable
error[E0507]: cannot move out of borrowed content
error[E0507]: cannot move out of captured variable in an `FnMut` closure
--> $DIR/borrowck-call-is-borrow-issue-12224.rs:66:13
|
LL | foo(f);
| ^ cannot move out of borrowed content
| ^ cannot move out of captured variable in an `FnMut` closure
error[E0505]: cannot move out of `f` because it is borrowed
--> $DIR/borrowck-call-is-borrow-issue-12224.rs:65:16