nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-04-28 02:57:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve cannot move errors

Browse Source 
* Show the place and type being moved
* Give a special error for variables in match guard
* Simplify search for overloaded deref
* Search for overloaded index
This commit is contained in:
Matthew Jasper 2019-05-05 11:16:56 +01:00
parent f309f917c2
commit f7e86a5a49
2 changed files with 320 additions and 209 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

517
src/librustc_mir/borrow_check/move_errors.rs
View File

@ -1,8 +1,7 @@
use core::unicode::property::Pattern_White_Space;
use std::fmt::{self, Display};
use rustc::mir::*;
use rustc::ty;
use rustc::ty::{self, Ty, TyCtxt};
use rustc_errors::{DiagnosticBuilder,Applicability};
use syntax_pos::Span;
@ -55,20 +54,66 @@ enum GroupedMoveError<'tcx> {
},
}
enum BorrowedContentSource {
Arc,
Rc,
enum BorrowedContentSource<'tcx> {
DerefRawPointer,
Other,
DerefMutableRef,
DerefSharedRef,
OverloadedDeref(Ty<'tcx>),
OverloadedIndex(Ty<'tcx>),
}
impl Display for BorrowedContentSource {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
impl BorrowedContentSource<'tcx> {
fn describe_for_unnamed_place(&self) -> String {
match *self {
BorrowedContentSource::Arc => write!(f, "an `Arc`"),
BorrowedContentSource::Rc => write!(f, "an `Rc`"),
BorrowedContentSource::DerefRawPointer => write!(f, "dereference of raw pointer"),
BorrowedContentSource::Other => write!(f, "borrowed content"),
BorrowedContentSource::DerefRawPointer => format!("a raw pointer"),
BorrowedContentSource::DerefSharedRef => format!("a shared reference"),
BorrowedContentSource::DerefMutableRef => {
format!("a mutable reference")
}
BorrowedContentSource::OverloadedDeref(ty) => {
if ty.is_rc() {
format!("an `Rc`")
} else if ty.is_arc() {
format!("an `Arc`")
} else {
format!("dereference of `{}`", ty)
}
}
BorrowedContentSource::OverloadedIndex(ty) => format!("index of `{}`", ty),
}
}
fn describe_for_named_place(&self) -> Option<&'static str> {
match *self {
BorrowedContentSource::DerefRawPointer => Some("raw pointer"),
BorrowedContentSource::DerefSharedRef => Some("shared reference"),
BorrowedContentSource::DerefMutableRef => Some("mutable reference"),
// Overloaded deref and index operators should be evaluated into a
// temporary. So we don't need a description here.
BorrowedContentSource::OverloadedDeref(_)
| BorrowedContentSource::OverloadedIndex(_) => None
}
}
fn from_call(func: Ty<'tcx>, tcx: TyCtxt<'_, '_, 'tcx>) -> Option<Self> {
match func.sty {
ty::FnDef(def_id, substs) => {
let trait_id = tcx.trait_of_item(def_id)?;
let lang_items = tcx.lang_items();
if Some(trait_id) == lang_items.deref_trait()
|| Some(trait_id) == lang_items.deref_mut_trait()
{
Some(BorrowedContentSource::OverloadedDeref(substs.type_at(0)))
} else if Some(trait_id) == lang_items.index_trait()
|| Some(trait_id) == lang_items.index_mut_trait()
{
Some(BorrowedContentSource::OverloadedIndex(substs.type_at(0)))
} else {
None
}
}
_ => None,
}
}
}
@ -248,109 +293,194 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
(span, original_path, kind)
},
};
let origin = Origin::Mir;
debug!("report: original_path={:?} span={:?}, kind={:?} \
original_path.is_upvar_field_projection={:?}", original_path, span, kind,
self.is_upvar_field_projection(original_path));
let err = match kind {
IllegalMoveOriginKind::Static => {
self.infcx.tcx.cannot_move_out_of(span, "static item", origin)
}
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.infcx.tcx).ty;
let is_upvar_field_projection =
self.prefixes(&original_path, PrefixSet::All)
.any(|p| self.is_upvar_field_projection(p).is_some());
debug!("report: ty={:?}", ty);
let mut err = match ty.sty {
ty::Array(..) | ty::Slice(..) =>
self.infcx.tcx.cannot_move_out_of_interior_noncopy(
span, ty, None, origin
),
ty::Closure(def_id, closure_substs)
if def_id == self.mir_def_id && is_upvar_field_projection
=> {
let closure_kind_ty =
closure_substs.closure_kind_ty(def_id, self.infcx.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"),
};
debug!("report: closure_kind_ty={:?} closure_kind={:?} \
place_description={:?}", closure_kind_ty, closure_kind,
place_description);
let mut diag = self.infcx.tcx.cannot_move_out_of(
span, place_description, origin);
for prefix in self.prefixes(&original_path, PrefixSet::All) {
if let Some(field) = self.is_upvar_field_projection(prefix) {
let upvar_hir_id = self.upvars[field.index()].var_hir_id;
let upvar_span = self.infcx.tcx.hir().span_by_hir_id(
upvar_hir_id);
diag.span_label(upvar_span, "captured outer variable");
break;
}
}
diag
}
_ => {
let source = self.borrowed_content_source(place);
self.infcx.tcx.cannot_move_out_of(
span, &source.to_string(), origin
)
},
};
let orig_path_ty = format!(
"{:?}",
original_path.ty(self.mir, self.infcx.tcx).ty,
);
let snippet = self.infcx.tcx.sess.source_map().span_to_snippet(span).unwrap();
let is_option = orig_path_ty.starts_with("std::option::Option");
let is_result = orig_path_ty.starts_with("std::result::Result");
if is_option || is_result {
err.span_suggestion(
span,
&format!("consider borrowing the `{}`'s content", if is_option {
"Option"
} else {
"Result"
}),
format!("{}.as_ref()", snippet),
Applicability::MaybeIncorrect,
);
(
match kind {
IllegalMoveOriginKind::Static => {
self.report_cannot_move_from_static(original_path, span)
}
err
}
IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
self.infcx.tcx
.cannot_move_out_of_interior_of_drop(span, ty, origin)
}
IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } =>
self.infcx.tcx.cannot_move_out_of_interior_noncopy(
span, ty, Some(*is_index), origin
),
};
(err, span)
IllegalMoveOriginKind::BorrowedContent { target_place } => {
self.report_cannot_move_from_borrowed_content(
original_path,
target_place,
span,
)
}
IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
self.infcx.tcx
.cannot_move_out_of_interior_of_drop(span, ty, Origin::Mir)
}
IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } =>
self.infcx.tcx.cannot_move_out_of_interior_noncopy(
span, ty, Some(*is_index), Origin::Mir
),
},
span,
)
};
self.add_move_hints(error, &mut err, err_span);
err.buffer(&mut self.errors_buffer);
}
fn report_cannot_move_from_static(
&mut self,
place: &Place<'tcx>,
span: Span
) -> DiagnosticBuilder<'a> {
let mut base_static = place;
loop {
match base_static {
Place::Base(_) => break,
Place::Projection(box Projection { base, .. }) => base_static = base,
}
}
let description = if let Place::Base(_) = place {
format!("static item `{}`", self.describe_place(place).unwrap())
} else {
format!(
"`{:?}` as `{:?}` is a static item",
self.describe_place(place).unwrap(),
self.describe_place(base_static).unwrap(),
)
};
self.infcx.tcx.cannot_move_out_of(span, &description, Origin::Mir)
}
fn report_cannot_move_from_borrowed_content(
&mut self,
move_place: &Place<'tcx>,
deref_target_place: &Place<'tcx>,
span: Span,
) -> DiagnosticBuilder<'a> {
let origin = Origin::Mir;
// Inspect the type of the content behind the
// borrow to provide feedback about why this
// was a move rather than a copy.
let ty = deref_target_place.ty(self.mir, self.infcx.tcx).ty;
let upvar_field = self.prefixes(&move_place, PrefixSet::All)
.find_map(|p| self.is_upvar_field_projection(p));
let deref_base = match deref_target_place {
Place::Projection(box Projection { base, elem: ProjectionElem::Deref }) => base,
_ => bug!("deref_target_place is not a deref projection"),
};
if let Place::Base(PlaceBase::Local(local)) = *deref_base {
let decl = &self.mir.local_decls[local];
if decl.is_ref_for_guard() {
let mut err = self.infcx.tcx.cannot_move_out_of(
span,
&format!("`{}` in pattern guard", decl.name.unwrap()),
origin,
);
err.note(
"variables bound in patterns cannot be moved from \
until after the end of the pattern guard");
return err;
}
}
debug!("report: ty={:?}", ty);
let mut err = match ty.sty {
ty::Array(..) | ty::Slice(..) =>
self.infcx.tcx.cannot_move_out_of_interior_noncopy(
span, ty, None, origin
),
ty::Closure(def_id, closure_substs)
if def_id == self.mir_def_id && upvar_field.is_some()
=> {
let closure_kind_ty = closure_substs.closure_kind_ty(def_id, self.infcx.tcx);
let closure_kind = closure_kind_ty.to_opt_closure_kind();
let capture_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"),
};
let upvar = &self.upvars[upvar_field.unwrap().index()];
let upvar_hir_id = upvar.var_hir_id;
let upvar_name = upvar.name;
let upvar_span = self.infcx.tcx.hir().span_by_hir_id(upvar_hir_id);
let place_name = self.describe_place(move_place).unwrap();
let place_description = if self.is_upvar_field_projection(move_place).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={:?}",
closure_kind_ty, closure_kind, place_description,
);
let mut diag = self.infcx.tcx.cannot_move_out_of(span, &place_description, origin);
diag.span_label(upvar_span, "captured outer variable");
diag
}
_ => {
let source = self.borrowed_content_source(deref_base);
match (self.describe_place(move_place), source.describe_for_named_place()) {
(Some(place_desc), Some(source_desc)) => {
self.infcx.tcx.cannot_move_out_of(
span,
&format!("`{}` which is behind a {}", place_desc, source_desc),
origin,
)
}
(_, _) => {
self.infcx.tcx.cannot_move_out_of(
span,
&source.describe_for_unnamed_place(),
origin,
)
}
}
},
};
let move_ty = format!(
"{:?}",
move_place.ty(self.mir, self.infcx.tcx).ty,
);
let snippet = self.infcx.tcx.sess.source_map().span_to_snippet(span).unwrap();
let is_option = move_ty.starts_with("std::option::Option");
let is_result = move_ty.starts_with("std::result::Result");
if is_option || is_result {
err.span_suggestion(
span,
&format!("consider borrowing the `{}`'s content", if is_option {
"Option"
} else {
"Result"
}),
format!("{}.as_ref()", snippet),
Applicability::MaybeIncorrect,
);
}
err
}
fn add_move_hints(
&self,
error: GroupedMoveError<'tcx>,
@ -391,9 +521,25 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
);
}
binds_to.sort();
binds_to.dedup();
self.add_move_error_details(err, &binds_to);
if binds_to.is_empty() {
let place_ty = move_from.ty(self.mir, self.infcx.tcx).ty;
let place_desc = match self.describe_place(&move_from) {
Some(desc) => format!("`{}`", desc),
None => format!("value"),
};
self.note_type_does_not_implement_copy(
err,
&place_desc,
place_ty,
Some(span)
);
} else {
binds_to.sort();
binds_to.dedup();
self.add_move_error_details(err, &binds_to);
}
}
GroupedMoveError::MovesFromValue { mut binds_to, .. } => {
binds_to.sort();
@ -402,7 +548,19 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
self.add_move_error_details(err, &binds_to);
}
// No binding. Nothing to suggest.
GroupedMoveError::OtherIllegalMove { .. } => (),
GroupedMoveError::OtherIllegalMove { ref original_path, span, .. } => {
let place_ty = original_path.ty(self.mir, self.infcx.tcx).ty;
let place_desc = match self.describe_place(original_path) {
Some(desc) => format!("`{}`", desc),
None => format!("value"),
};
self.note_type_does_not_implement_copy(
err,
&place_desc,
place_ty,
Some(span),
);
},
}
}
@ -493,105 +651,64 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
}
}
fn borrowed_content_source(&self, place: &Place<'tcx>) -> BorrowedContentSource {
fn borrowed_content_source(&self, deref_base: &Place<'tcx>) -> BorrowedContentSource<'tcx> {
let tcx = self.infcx.tcx;
// Look up the provided place and work out the move path index for it,
// we'll use this to work back through where this value came from and check whether it
// was originally part of an `Rc` or `Arc`.
let initial_mpi = match self.move_data.rev_lookup.find(place) {
LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => mpi,
_ => return BorrowedContentSource::Other,
};
// we'll use this to check whether it was originally from an overloaded
// operator.
match self.move_data.rev_lookup.find(deref_base) {
LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => {
debug!("borrowed_content_source: mpi={:?}", mpi);
let mut queue = vec![initial_mpi];
let mut visited = Vec::new();
debug!("borrowed_content_source: queue={:?}", queue);
while let Some(mpi) = queue.pop() {
debug!(
"borrowed_content_source: mpi={:?} queue={:?} visited={:?}",
mpi, queue, visited
);
for i in &self.move_data.init_path_map[mpi] {
let init = &self.move_data.inits[*i];
debug!("borrowed_content_source: init={:?}", init);
// We're only interested in statements that initialized a value, not the
// initializations from arguments.
let loc = match init.location {
InitLocation::Statement(stmt) => stmt,
_ => continue,
};
// Don't visit the same path twice.
if visited.contains(&mpi) {
continue;
}
visited.push(mpi);
for i in &self.move_data.init_path_map[mpi] {
let init = &self.move_data.inits[*i];
debug!("borrowed_content_source: init={:?}", init);
// We're only interested in statements that initialized a value, not the
// initializations from arguments.
let loc = match init.location {
InitLocation::Statement(stmt) => stmt,
_ => continue,
};
let bbd = &self.mir[loc.block];
let is_terminator = bbd.statements.len() == loc.statement_index;
debug!("borrowed_content_source: loc={:?} is_terminator={:?}", loc, is_terminator);
if !is_terminator {
let stmt = &bbd.statements[loc.statement_index];
debug!("borrowed_content_source: stmt={:?}", stmt);
// We're only interested in assignments (in particular, where the
// assignment came from - was it an `Rc` or `Arc`?).
if let StatementKind::Assign(_, box Rvalue::Ref(_, _, source)) = &stmt.kind {
let ty = source.ty(self.mir, self.infcx.tcx).ty;
let ty = match ty.sty {
ty::Ref(_, ty, _) => ty,
_ => ty,
};
debug!("borrowed_content_source: ty={:?}", ty);
if ty.is_arc() {
return BorrowedContentSource::Arc;
} else if ty.is_rc() {
return BorrowedContentSource::Rc;
} else {
queue.push(init.path);
}
}
} else if let Some(Terminator {
kind: TerminatorKind::Call { args, .. },
..
}) = &bbd.terminator {
for arg in args {
let source = match arg {
Operand::Copy(place) | Operand::Move(place) => place,
_ => continue,
};
let ty = source.ty(self.mir, self.infcx.tcx).ty;
let ty = match ty.sty {
ty::Ref(_, ty, _) => ty,
_ => ty,
};
debug!("borrowed_content_source: ty={:?}", ty);
if ty.is_arc() {
return BorrowedContentSource::Arc;
} else if ty.is_rc() {
return BorrowedContentSource::Rc;
} else {
queue.push(init.path);
let bbd = &self.mir[loc.block];
let is_terminator = bbd.statements.len() == loc.statement_index;
debug!(
"borrowed_content_source: loc={:?} is_terminator={:?}",
loc,
is_terminator,
);
if !is_terminator {
continue;
} else if let Some(Terminator {
kind: TerminatorKind::Call {
ref func,
from_hir_call: false,
..
},
..
}) = bbd.terminator {
if let Some(source)
= BorrowedContentSource::from_call(func.ty(self.mir, tcx), tcx)
{
return source;
}
}
}
}
}
// Base is a `static` so won't be from an overloaded operator
_ => (),
};
// If we didn't find an `Arc` or an `Rc`, then check specifically for
// a dereference of a place that has the type of a raw pointer.
// We can't use `place.ty(..).to_ty(..)` here as that strips away the raw pointer.
if let Place::Projection(box Projection {
base,
elem: ProjectionElem::Deref,
}) = place {
if base.ty(self.mir, self.infcx.tcx).ty.is_unsafe_ptr() {
return BorrowedContentSource::DerefRawPointer;
}
// If we didn't find an overloaded deref or index, then assume it's a
// built in deref and check the type of the base.
let base_ty = deref_base.ty(self.mir, tcx).ty;
if base_ty.is_unsafe_ptr() {
BorrowedContentSource::DerefRawPointer
} else if base_ty.is_mutable_pointer() {
BorrowedContentSource::DerefMutableRef
} else {
BorrowedContentSource::DerefSharedRef
}
BorrowedContentSource::Other
}
}

12
src/librustc_mir/util/borrowck_errors.rs
View File

@ -399,7 +399,7 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
move_from_desc: &str,
o: Origin,
) -> DiagnosticBuilder<'cx> {
let mut err = struct_span_err!(
let err = struct_span_err!(
self,
move_from_span,
E0507,
@ -407,10 +407,6 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
move_from_desc,
OGN = o
);
err.span_label(
move_from_span,
format!("cannot move out of {}", move_from_desc),
);
self.cancel_if_wrong_origin(err, o)
}
@ -434,8 +430,7 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
self,
move_from_span,
E0508,
"cannot move out of type `{}`, \
a non-copy {}{OGN}",
"cannot move out of type `{}`, a non-copy {}{OGN}",
ty,
type_name,
OGN = o
@ -455,8 +450,7 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
self,
move_from_span,
E0509,
"cannot move out of type `{}`, \
which implements the `Drop` trait{OGN}",
"cannot move out of type `{}`, which implements the `Drop` trait{OGN}",
container_ty,
OGN = o
);