Skip to content

Commit 2c6b0e5

Browse files
davidtwcodavidtwco
committed
Label definition of captured variables in errors.
1 parent 51b0552 commit 2c6b0e5

File tree

9 files changed

+95
-63
lines changed

9 files changed

+95
-63
lines changed

‎src/librustc/mir/tcx.rs‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,22 +127,24 @@ impl<'tcx> Place<'tcx> {
127127
/// of a closure type.
128128
pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>,
129129
tcx: &TyCtxt<'cx, 'gcx, 'tcx>) -> Option<Field> {
130-
let place = if let Place::Projection(ref proj) = self {
130+
let (place, by_ref) = if let Place::Projection(ref proj) = self {
131131
if let ProjectionElem::Deref = proj.elem {
132-
&proj.base
132+
(&proj.base, true)
133133
} else {
134-
self
134+
(self, false)
135135
}
136136
} else {
137-
self
137+
(self, false)
138138
};
139139

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

145-
if base_ty.is_closure() || base_ty.is_generator() {
145+
if (base_ty.is_closure() || base_ty.is_generator()) &&
146+
(!by_ref || mir.upvar_decls[field.index()].by_ref)
147+
{
146148
Some(field)
147149
} else {
148150
None
@@ -153,6 +155,21 @@ impl<'tcx> Place<'tcx> {
153155
_ => None,
154156
}
155157
}
158+
159+
/// Strip the deref projections from a `Place`. For example, given `(*(*((*_1).0: &&T)))`, this
160+
/// will return `((*_1).0)`. Once stripped of any deref projections, places can then be
161+
/// checked as upvar field projections using `is_upvar_field_projection`.
162+
pub fn strip_deref_projections(&self) -> &Place<'tcx> {
163+
let mut current = self;
164+
while let Place::Projection(ref proj) = current {
165+
if let ProjectionElem::Deref = proj.elem {
166+
current = &proj.base;
167+
} else {
168+
break;
169+
}
170+
}
171+
current
172+
}
156173
}
157174

158175
pub enum RvalueInitializationState {

‎src/librustc_mir/borrow_check/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
151151
let location_table = &LocationTable::new(mir);
152152

153153
let mut errors_buffer = Vec::new();
154-
let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<MoveError<'tcx>>>) =
154+
let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<(Place<'tcx>, MoveError<'tcx>)>>) =
155155
match MoveData::gather_moves(mir, tcx) {
156156
Ok(move_data) => (move_data, None),
157157
Err((move_data, move_errors)) => (move_data, Some(move_errors)),

‎src/librustc_mir/borrow_check/move_errors.rs‎

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
use rustc::hir;
1212
use rustc::mir::*;
1313
use rustc::ty;
14-
use rustc_data_structures::indexed_vec::Idx;
1514
use rustc_errors::DiagnosticBuilder;
15+
use rustc_data_structures::indexed_vec::Idx;
1616
use syntax_pos::Span;
1717

1818
use borrow_check::MirBorrowckCtxt;
@@ -38,6 +38,7 @@ enum GroupedMoveError<'tcx> {
3838
// Match place can't be moved from
3939
// e.g. match x[0] { s => (), } where x: &[String]
4040
MovesFromMatchPlace {
41+
original_path: Place<'tcx>,
4142
span: Span,
4243
move_from: Place<'tcx>,
4344
kind: IllegalMoveOriginKind<'tcx>,
@@ -46,37 +47,43 @@ enum GroupedMoveError<'tcx> {
4647
// Part of a pattern can't be moved from,
4748
// e.g. match &String::new() { &x => (), }
4849
MovesFromPattern {
50+
original_path: Place<'tcx>,
4951
span: Span,
5052
move_from: MovePathIndex,
5153
kind: IllegalMoveOriginKind<'tcx>,
5254
binds_to: Vec<Local>,
5355
},
5456
// Everything that isn't from pattern matching.
5557
OtherIllegalMove {
58+
original_path: Place<'tcx>,
5659
span: Span,
5760
kind: IllegalMoveOriginKind<'tcx>,
5861
},
5962
}
6063

6164
impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
62-
pub(crate) fn report_move_errors(&mut self, move_errors: Vec<MoveError<'tcx>>) {
65+
pub(crate) fn report_move_errors(&mut self, move_errors: Vec<(Place<'tcx>, MoveError<'tcx>)>) {
6366
let grouped_errors = self.group_move_errors(move_errors);
6467
for error in grouped_errors {
6568
self.report(error);
6669
}
6770
}
6871

69-
fn group_move_errors(&self, errors: Vec<MoveError<'tcx>>) -> Vec<GroupedMoveError<'tcx>> {
72+
fn group_move_errors(
73+
&self,
74+
errors: Vec<(Place<'tcx>, MoveError<'tcx>)>
75+
) -> Vec<GroupedMoveError<'tcx>> {
7076
let mut grouped_errors = Vec::new();
71-
for error in errors {
72-
self.append_to_grouped_errors(&mut grouped_errors, error);
77+
for (original_path, error) in errors {
78+
self.append_to_grouped_errors(&mut grouped_errors, original_path, error);
7379
}
7480
grouped_errors
7581
}
7682

7783
fn append_to_grouped_errors(
7884
&self,
7985
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
86+
original_path: Place<'tcx>,
8087
error: MoveError<'tcx>,
8188
) {
8289
match error {
@@ -116,6 +123,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
116123
self.append_binding_error(
117124
grouped_errors,
118125
kind,
126+
original_path,
119127
move_from,
120128
*local,
121129
opt_match_place,
@@ -127,6 +135,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
127135
}
128136
grouped_errors.push(GroupedMoveError::OtherIllegalMove {
129137
span: stmt_source_info.span,
138+
original_path,
130139
kind,
131140
});
132141
}
@@ -137,6 +146,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
137146
&self,
138147
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
139148
kind: IllegalMoveOriginKind<'tcx>,
149+
original_path: Place<'tcx>,
140150
move_from: &Place<'tcx>,
141151
bind_to: Local,
142152
match_place: &Option<Place<'tcx>>,
@@ -176,6 +186,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
176186
grouped_errors.push(GroupedMoveError::MovesFromMatchPlace {
177187
span,
178188
move_from: match_place.clone(),
189+
original_path,
179190
kind,
180191
binds_to,
181192
});
@@ -206,6 +217,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
206217
grouped_errors.push(GroupedMoveError::MovesFromPattern {
207218
span: match_span,
208219
move_from: mpi,
220+
original_path,
209221
kind,
210222
binds_to: vec![bind_to],
211223
});
@@ -215,13 +227,23 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
215227

216228
fn report(&mut self, error: GroupedMoveError<'tcx>) {
217229
let (mut err, err_span) = {
218-
let (span, kind): (Span, &IllegalMoveOriginKind) = match error {
219-
GroupedMoveError::MovesFromMatchPlace { span, ref kind, .. }
220-
| GroupedMoveError::MovesFromPattern { span, ref kind, .. }
221-
| GroupedMoveError::OtherIllegalMove { span, ref kind } => (span, kind),
222-
};
230+
let (span, original_path, kind): (Span, &Place<'tcx>, &IllegalMoveOriginKind) =
231+
match error {
232+
GroupedMoveError::MovesFromMatchPlace {
233+
span,
234+
ref original_path,
235+
ref kind,
236+
..
237+
} |
238+
GroupedMoveError::MovesFromPattern { span, ref original_path, ref kind, .. } |
239+
GroupedMoveError::OtherIllegalMove { span, ref original_path, ref kind } => {
240+
(span, original_path, kind)
241+
},
242+
};
223243
let origin = Origin::Mir;
224-
debug!("report: span={:?}, kind={:?}", span, kind);
244+
debug!("report: original_path={:?} span={:?}, kind={:?} \
245+
original_path.is_upvar_field_projection={:?}", original_path, span, kind,
246+
original_path.is_upvar_field_projection(self.mir, &self.tcx));
225247
(
226248
match kind {
227249
IllegalMoveOriginKind::Static => {
@@ -237,17 +259,11 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
237259
.tcx
238260
.cannot_move_out_of_interior_noncopy(span, ty, None, origin),
239261
ty::TyClosure(def_id, closure_substs)
240-
if !self.mir.upvar_decls.is_empty()
241-
&& {
242-
match place {
243-
Place::Projection(ref proj) => {
244-
proj.base == Place::Local(Local::new(1))
245-
}
246-
Place::Promoted(_) |
247-
Place::Local(_) | Place::Static(_) => unreachable!(),
248-
}
249-
} =>
250-
{
262+
if !self.mir.upvar_decls.is_empty() &&
263+
original_path.strip_deref_projections()
264+
.is_upvar_field_projection(self.mir, &self.tcx)
265+
.is_some()
266+
=> {
251267
let closure_kind_ty =
252268
closure_substs.closure_kind_ty(def_id, self.tcx);
253269
let closure_kind = closure_kind_ty.to_opt_closure_kind();
@@ -267,7 +283,19 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
267283
place_description={:?}", closure_kind_ty, closure_kind,
268284
place_description);
269285

270-
self.tcx.cannot_move_out_of(span, place_description, origin)
286+
let mut diag = self.tcx.cannot_move_out_of(
287+
span, place_description, origin);
288+
289+
if let Some(field) = original_path.is_upvar_field_projection(
290+
self.mir, &self.tcx) {
291+
let upvar_decl = &self.mir.upvar_decls[field.index()];
292+
let upvar_hir_id = upvar_decl.var_hir_id.assert_crate_local();
293+
let upvar_node_id = self.tcx.hir.hir_to_node_id(upvar_hir_id);
294+
let upvar_span = self.tcx.hir.span(upvar_node_id);
295+
diag.span_label(upvar_span, "captured outer variable");
296+
}
297+
298+
diag
271299
}
272300
_ => self
273301
.tcx

‎src/librustc_mir/borrow_check/mutability_errors.rs‎

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
7171
));
7272

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

93-
reason = if self.is_upvar(access_place) {
93+
reason = if access_place.is_upvar_field_projection(self.mir,
94+
&self.tcx).is_some() {
9495
", as it is a captured variable in a `Fn` closure".to_string()
9596
} else {
9697
", as `Fn` closures cannot mutate their captured variables".to_string()
@@ -394,31 +395,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
394395

395396
err.buffer(&mut self.errors_buffer);
396397
}
397-
398-
// Does this place refer to what the user sees as an upvar
399-
fn is_upvar(&self, place: &Place<'tcx>) -> bool {
400-
match *place {
401-
Place::Projection(box Projection {
402-
ref base,
403-
elem: ProjectionElem::Field(_, _),
404-
}) => {
405-
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
406-
is_closure_or_generator(base_ty)
407-
}
408-
Place::Projection(box Projection {
409-
base:
410-
Place::Projection(box Projection {
411-
ref base,
412-
elem: ProjectionElem::Field(upvar_index, _),
413-
}),
414-
elem: ProjectionElem::Deref,
415-
}) => {
416-
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
417-
is_closure_or_generator(base_ty) && self.mir.upvar_decls[upvar_index.index()].by_ref
418-
}
419-
_ => false,
420-
}
421-
}
422398
}
423399

424400
fn suggest_ampmut_self<'cx, 'gcx, 'tcx>(

‎src/librustc_mir/dataflow/move_paths/builder.rs‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ struct MoveDataBuilder<'a, 'gcx: 'tcx, 'tcx: 'a> {
2727
mir: &'a Mir<'tcx>,
2828
tcx: TyCtxt<'a, 'gcx, 'tcx>,
2929
data: MoveData<'tcx>,
30-
errors: Vec<MoveError<'tcx>>,
30+
errors: Vec<(Place<'tcx>, MoveError<'tcx>)>,
3131
}
3232

3333
impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
@@ -186,7 +186,9 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
186186
}
187187

188188
impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
189-
fn finalize(self) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
189+
fn finalize(
190+
self
191+
) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
190192
debug!("{}", {
191193
debug!("moves for {:?}:", self.mir.span);
192194
for (j, mo) in self.data.moves.iter_enumerated() {
@@ -207,9 +209,10 @@ impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
207209
}
208210
}
209211

210-
pub(super) fn gather_moves<'a, 'gcx, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>)
211-
-> Result<MoveData<'tcx>,
212-
(MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
212+
pub(super) fn gather_moves<'a, 'gcx, 'tcx>(
213+
mir: &Mir<'tcx>,
214+
tcx: TyCtxt<'a, 'gcx, 'tcx>
215+
) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
213216
let mut builder = MoveDataBuilder::new(mir, tcx);
214217

215218
builder.gather_args();
@@ -407,7 +410,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
407410
let path = match self.move_path_for(place) {
408411
Ok(path) | Err(MoveError::UnionMove { path }) => path,
409412
Err(error @ MoveError::IllegalMove { .. }) => {
410-
self.builder.errors.push(error);
413+
self.builder.errors.push((place.clone(), error));
411414
return;
412415
}
413416
};

‎src/librustc_mir/dataflow/move_paths/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ impl<'tcx> MoveError<'tcx> {
313313

314314
impl<'a, 'gcx, 'tcx> MoveData<'tcx> {
315315
pub fn gather_moves(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>)
316-
-> Result<Self, (Self, Vec<MoveError<'tcx>>)> {
316+
-> Result<Self, (Self, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
317317
builder::gather_moves(mir, tcx)
318318
}
319319
}

‎src/test/ui/borrowck/borrowck-in-static.nll.stderr‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
error[E0507]: cannot move out of captured variable in an `Fn` closure
22
--> $DIR/borrowck-in-static.rs:15:17
33
|
4+
LL | let x = Box::new(0);
5+
| - captured outer variable
46
LL | Box::new(|| x) //~ ERROR cannot move out of captured outer variable
57
| ^ cannot move out of captured variable in an `Fn` closure
68

‎src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.nll.stderr‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
error[E0507]: cannot move out of captured variable in an `Fn` closure
22
--> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:21:9
33
|
4+
LL | let y = vec![format!("World")];
5+
| - captured outer variable
6+
LL | call(|| {
47
LL | y.into_iter();
58
| ^ cannot move out of captured variable in an `Fn` closure
69

‎src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ LL | f.f.call_mut(())
3131
error[E0507]: cannot move out of captured variable in an `FnMut` closure
3232
--> $DIR/borrowck-call-is-borrow-issue-12224.rs:66:13
3333
|
34+
LL | let mut f = move |g: Box<FnMut(isize)>, b: isize| {
35+
| ----- captured outer variable
36+
...
3437
LL | foo(f);
3538
| ^ cannot move out of captured variable in an `FnMut` closure
3639

0 commit comments

Comments
 (0)