Skip to content

Commit

Permalink
Rollup merge of rust-lang#53164 - davidtwco:issue-52663-span-decl-cap…
Browse files Browse the repository at this point in the history
…tured-variables, r=nikomatsakis

Provide span for declaration of captured variables

Part of rust-lang#52663.

r? @nikomatsakis
  • Loading branch information
cramertj committed Aug 8, 2018
2 parents 60e809e + 56232c6 commit acbe0ad
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 63 deletions.
14 changes: 8 additions & 6 deletions src/librustc/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,29 +127,31 @@ impl<'tcx> Place<'tcx> {
/// of a closure type.
pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>,
tcx: &TyCtxt<'cx, 'gcx, 'tcx>) -> Option<Field> {
let place = if let Place::Projection(ref proj) = self {
let (place, by_ref) = if let Place::Projection(ref proj) = self {
if let ProjectionElem::Deref = proj.elem {
&proj.base
(&proj.base, true)
} else {
self
(self, false)
}
} else {
self
(self, false)
};

match place {
Place::Projection(ref proj) => match proj.elem {
ProjectionElem::Field(field, _ty) => {
let base_ty = proj.base.ty(mir, *tcx).to_ty(*tcx);

if base_ty.is_closure() || base_ty.is_generator() {
if (base_ty.is_closure() || base_ty.is_generator()) &&
(!by_ref || mir.upvar_decls[field.index()].by_ref)
{
Some(field)
} else {
None
}
},
_ => None,
},
}
_ => None,
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
let location_table = &LocationTable::new(mir);

let mut errors_buffer = Vec::new();
let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<MoveError<'tcx>>>) =
let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<(Place<'tcx>, MoveError<'tcx>)>>) =
match MoveData::gather_moves(mir, tcx) {
Ok(move_data) => (move_data, None),
Err((move_data, move_errors)) => (move_data, Some(move_errors)),
Expand Down
84 changes: 62 additions & 22 deletions src/librustc_mir/borrow_check/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
use rustc::hir;
use rustc::mir::*;
use rustc::ty;
use rustc_data_structures::indexed_vec::Idx;
use rustc_errors::DiagnosticBuilder;
use rustc_data_structures::indexed_vec::Idx;
use syntax_pos::Span;

use borrow_check::MirBorrowckCtxt;
use borrow_check::prefixes::PrefixSet;
use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind};
use dataflow::move_paths::{LookupResult, MoveError, MovePathIndex};
use util::borrowck_errors::{BorrowckErrors, Origin};
Expand All @@ -38,6 +39,7 @@ enum GroupedMoveError<'tcx> {
// Match place can't be moved from
// e.g. match x[0] { s => (), } where x: &[String]
MovesFromMatchPlace {
original_path: Place<'tcx>,
span: Span,
move_from: Place<'tcx>,
kind: IllegalMoveOriginKind<'tcx>,
Expand All @@ -46,37 +48,43 @@ enum GroupedMoveError<'tcx> {
// Part of a pattern can't be moved from,
// e.g. match &String::new() { &x => (), }
MovesFromPattern {
original_path: Place<'tcx>,
span: Span,
move_from: MovePathIndex,
kind: IllegalMoveOriginKind<'tcx>,
binds_to: Vec<Local>,
},
// Everything that isn't from pattern matching.
OtherIllegalMove {
original_path: Place<'tcx>,
span: Span,
kind: IllegalMoveOriginKind<'tcx>,
},
}

impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
pub(crate) fn report_move_errors(&mut self, move_errors: Vec<MoveError<'tcx>>) {
pub(crate) fn report_move_errors(&mut self, move_errors: Vec<(Place<'tcx>, 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<(Place<'tcx>, MoveError<'tcx>)>
) -> Vec<GroupedMoveError<'tcx>> {
let mut grouped_errors = Vec::new();
for error in errors {
self.append_to_grouped_errors(&mut grouped_errors, error);
for (original_path, error) in errors {
self.append_to_grouped_errors(&mut grouped_errors, original_path, error);
}
grouped_errors
}

fn append_to_grouped_errors(
&self,
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
original_path: Place<'tcx>,
error: MoveError<'tcx>,
) {
match error {
Expand Down Expand Up @@ -116,6 +124,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
self.append_binding_error(
grouped_errors,
kind,
original_path,
move_from,
*local,
opt_match_place,
Expand All @@ -127,6 +136,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
}
grouped_errors.push(GroupedMoveError::OtherIllegalMove {
span: stmt_source_info.span,
original_path,
kind,
});
}
Expand All @@ -137,6 +147,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
&self,
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
kind: IllegalMoveOriginKind<'tcx>,
original_path: Place<'tcx>,
move_from: &Place<'tcx>,
bind_to: Local,
match_place: &Option<Place<'tcx>>,
Expand Down Expand Up @@ -176,6 +187,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
grouped_errors.push(GroupedMoveError::MovesFromMatchPlace {
span,
move_from: match_place.clone(),
original_path,
kind,
binds_to,
});
Expand Down Expand Up @@ -206,6 +218,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
grouped_errors.push(GroupedMoveError::MovesFromPattern {
span: match_span,
move_from: mpi,
original_path,
kind,
binds_to: vec![bind_to],
});
Expand All @@ -215,12 +228,23 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {

fn report(&mut self, error: GroupedMoveError<'tcx>) {
let (mut err, err_span) = {
let (span, kind): (Span, &IllegalMoveOriginKind) = match error {
GroupedMoveError::MovesFromMatchPlace { span, ref kind, .. }
| GroupedMoveError::MovesFromPattern { span, ref kind, .. }
| GroupedMoveError::OtherIllegalMove { span, ref kind } => (span, kind),
};
let (span, original_path, kind): (Span, &Place<'tcx>, &IllegalMoveOriginKind) =
match error {
GroupedMoveError::MovesFromMatchPlace {
span,
ref original_path,
ref kind,
..
} |
GroupedMoveError::MovesFromPattern { span, ref original_path, ref kind, .. } |
GroupedMoveError::OtherIllegalMove { span, ref original_path, ref kind } => {
(span, original_path, kind)
},
};
let origin = Origin::Mir;
debug!("report: original_path={:?} span={:?}, kind={:?} \
original_path.is_upvar_field_projection={:?}", original_path, span, kind,
original_path.is_upvar_field_projection(self.mir, &self.tcx));
(
match kind {
IllegalMoveOriginKind::Static => {
Expand All @@ -231,22 +255,17 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
// 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);
let is_upvar_field_projection =
self.prefixes(&original_path, PrefixSet::All)
.any(|p| p.is_upvar_field_projection(self.mir, &self.tcx)
.is_some());
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::Promoted(_) |
Place::Local(_) | Place::Static(_) => unreachable!(),
}
} =>
{
if !self.mir.upvar_decls.is_empty() && is_upvar_field_projection
=> {
let closure_kind_ty =
closure_substs.closure_kind_ty(def_id, self.tcx);
let closure_kind = closure_kind_ty.to_opt_closure_kind();
Expand All @@ -262,7 +281,28 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
}
None => bug!("closure kind not inferred by borrowck"),
};
self.tcx.cannot_move_out_of(span, place_description, origin)
debug!("report: closure_kind_ty={:?} closure_kind={:?} \
place_description={:?}", closure_kind_ty, closure_kind,
place_description);

let mut diag = self.tcx.cannot_move_out_of(
span, place_description, origin);

for prefix in self.prefixes(&original_path, PrefixSet::All) {
if let Some(field) = prefix.is_upvar_field_projection(
self.mir, &self.tcx) {
let upvar_decl = &self.mir.upvar_decls[field.index()];
let upvar_hir_id =
upvar_decl.var_hir_id.assert_crate_local();
let upvar_node_id =
self.tcx.hir.hir_to_node_id(upvar_hir_id);
let upvar_span = self.tcx.hir.span(upvar_node_id);
diag.span_label(upvar_span, "captured outer variable");
break;
}
}

diag
}
_ => self
.tcx
Expand Down
42 changes: 15 additions & 27 deletions src/librustc_mir/borrow_check/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,18 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
error_access: AccessKind,
location: Location,
) {
debug!(
"report_mutability_error(\
access_place={:?}, span={:?}, the_place_err={:?}, error_access={:?}, location={:?},\
)",
access_place, span, the_place_err, error_access, location,
);

let mut err;
let item_msg;
let reason;
let access_place_desc = self.describe_place(access_place);
debug!("report_mutability_error: access_place_desc={:?}", access_place_desc);

match the_place_err {
Place::Local(local) => {
Expand All @@ -63,7 +71,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
));

item_msg = format!("`{}`", access_place_desc.unwrap());
if self.is_upvar(access_place) {
if access_place.is_upvar_field_projection(self.mir, &self.tcx).is_some() {
reason = ", as it is not declared as mutable".to_string();
} else {
let name = self.mir.upvar_decls[upvar_index.index()].debug_name;
Expand All @@ -82,7 +90,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
the_place_err.ty(self.mir, self.tcx).to_ty(self.tcx)
));

reason = if self.is_upvar(access_place) {
reason = if access_place.is_upvar_field_projection(self.mir,
&self.tcx).is_some() {
", as it is a captured variable in a `Fn` closure".to_string()
} else {
", as `Fn` closures cannot mutate their captured variables".to_string()
Expand Down Expand Up @@ -155,6 +164,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
}) => bug!("Unexpected immutable place."),
}

debug!("report_mutability_error: item_msg={:?}, reason={:?}", item_msg, reason);

// `act` and `acted_on` are strings that let us abstract over
// the verbs used in some diagnostic messages.
let act;
Expand Down Expand Up @@ -199,6 +210,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
}
};

debug!("report_mutability_error: act={:?}, acted_on={:?}", act, acted_on);

match the_place_err {
// We want to suggest users use `let mut` for local (user
// variable) mutations...
Expand Down Expand Up @@ -382,31 +395,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {

err.buffer(&mut self.errors_buffer);
}

// Does this place refer to what the user sees as an upvar
fn is_upvar(&self, place: &Place<'tcx>) -> bool {
match *place {
Place::Projection(box Projection {
ref base,
elem: ProjectionElem::Field(_, _),
}) => {
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
is_closure_or_generator(base_ty)
}
Place::Projection(box Projection {
base:
Place::Projection(box Projection {
ref base,
elem: ProjectionElem::Field(upvar_index, _),
}),
elem: ProjectionElem::Deref,
}) => {
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
is_closure_or_generator(base_ty) && self.mir.upvar_decls[upvar_index.index()].by_ref
}
_ => false,
}
}
}

fn suggest_ampmut_self<'cx, 'gcx, 'tcx>(
Expand Down
15 changes: 9 additions & 6 deletions src/librustc_mir/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct MoveDataBuilder<'a, 'gcx: 'tcx, 'tcx: 'a> {
mir: &'a Mir<'tcx>,
tcx: TyCtxt<'a, 'gcx, 'tcx>,
data: MoveData<'tcx>,
errors: Vec<MoveError<'tcx>>,
errors: Vec<(Place<'tcx>, MoveError<'tcx>)>,
}

impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -186,7 +186,9 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
}

impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
fn finalize(self) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
fn finalize(
self
) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
debug!("{}", {
debug!("moves for {:?}:", self.mir.span);
for (j, mo) in self.data.moves.iter_enumerated() {
Expand All @@ -207,9 +209,10 @@ impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
}
}

pub(super) fn gather_moves<'a, 'gcx, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>)
-> Result<MoveData<'tcx>,
(MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
pub(super) fn gather_moves<'a, 'gcx, 'tcx>(
mir: &Mir<'tcx>,
tcx: TyCtxt<'a, 'gcx, 'tcx>
) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
let mut builder = MoveDataBuilder::new(mir, tcx);

builder.gather_args();
Expand Down Expand Up @@ -407,7 +410,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
let path = match self.move_path_for(place) {
Ok(path) | Err(MoveError::UnionMove { path }) => path,
Err(error @ MoveError::IllegalMove { .. }) => {
self.builder.errors.push(error);
self.builder.errors.push((place.clone(), error));
return;
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/dataflow/move_paths/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl<'tcx> MoveError<'tcx> {

impl<'a, 'gcx, 'tcx> MoveData<'tcx> {
pub fn gather_moves(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>)
-> Result<Self, (Self, Vec<MoveError<'tcx>>)> {
-> Result<Self, (Self, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
builder::gather_moves(mir, tcx)
}
}
2 changes: 2 additions & 0 deletions src/test/ui/borrowck/borrowck-in-static.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
error[E0507]: cannot move out of captured variable in an `Fn` closure
--> $DIR/borrowck-in-static.rs:15:17
|
LL | let x = Box::new(0);
| - captured outer variable
LL | Box::new(|| x) //~ ERROR cannot move out of captured outer variable
| ^ cannot move out of captured variable in an `Fn` closure

Expand Down
Loading

0 comments on commit acbe0ad

Please sign in to comment.