Skip to content

Commit d679b57

Browse files
borsbors
committed
Auto merge of #53164 - davidtwco:issue-52663-span-decl-captured-variables, r=nikomatsakis
Provide span for declaration of captured variables Part of #52663. r? @nikomatsakis
2 parents 0f4b498 + 5532e9d commit d679b57

15 files changed

+153
-63
lines changed

‎src/librustc/mir/tcx.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -127,29 +127,31 @@ 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
149151
}
150152
},
151153
_ => None,
152-
},
154+
}
153155
_ => None,
154156
}
155157
}

‎src/librustc_mir/borrow_check/mod.rs

+1-1
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

+62-22
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@
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;
19+
use borrow_check::prefixes::PrefixSet;
1920
use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind};
2021
use dataflow::move_paths::{LookupResult, MoveError, MovePathIndex};
2122
use util::borrowck_errors::{BorrowckErrors, Origin};
@@ -38,6 +39,7 @@ enum GroupedMoveError<'tcx> {
3839
// Match place can't be moved from
3940
// e.g. match x[0] { s => (), } where x: &[String]
4041
MovesFromMatchPlace {
42+
original_path: Place<'tcx>,
4143
span: Span,
4244
move_from: Place<'tcx>,
4345
kind: IllegalMoveOriginKind<'tcx>,
@@ -46,37 +48,43 @@ enum GroupedMoveError<'tcx> {
4648
// Part of a pattern can't be moved from,
4749
// e.g. match &String::new() { &x => (), }
4850
MovesFromPattern {
51+
original_path: Place<'tcx>,
4952
span: Span,
5053
move_from: MovePathIndex,
5154
kind: IllegalMoveOriginKind<'tcx>,
5255
binds_to: Vec<Local>,
5356
},
5457
// Everything that isn't from pattern matching.
5558
OtherIllegalMove {
59+
original_path: Place<'tcx>,
5660
span: Span,
5761
kind: IllegalMoveOriginKind<'tcx>,
5862
},
5963
}
6064

6165
impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
62-
pub(crate) fn report_move_errors(&mut self, move_errors: Vec<MoveError<'tcx>>) {
66+
pub(crate) fn report_move_errors(&mut self, move_errors: Vec<(Place<'tcx>, MoveError<'tcx>)>) {
6367
let grouped_errors = self.group_move_errors(move_errors);
6468
for error in grouped_errors {
6569
self.report(error);
6670
}
6771
}
6872

69-
fn group_move_errors(&self, errors: Vec<MoveError<'tcx>>) -> Vec<GroupedMoveError<'tcx>> {
73+
fn group_move_errors(
74+
&self,
75+
errors: Vec<(Place<'tcx>, MoveError<'tcx>)>
76+
) -> Vec<GroupedMoveError<'tcx>> {
7077
let mut grouped_errors = Vec::new();
71-
for error in errors {
72-
self.append_to_grouped_errors(&mut grouped_errors, error);
78+
for (original_path, error) in errors {
79+
self.append_to_grouped_errors(&mut grouped_errors, original_path, error);
7380
}
7481
grouped_errors
7582
}
7683

7784
fn append_to_grouped_errors(
7885
&self,
7986
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
87+
original_path: Place<'tcx>,
8088
error: MoveError<'tcx>,
8189
) {
8290
match error {
@@ -116,6 +124,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
116124
self.append_binding_error(
117125
grouped_errors,
118126
kind,
127+
original_path,
119128
move_from,
120129
*local,
121130
opt_match_place,
@@ -127,6 +136,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
127136
}
128137
grouped_errors.push(GroupedMoveError::OtherIllegalMove {
129138
span: stmt_source_info.span,
139+
original_path,
130140
kind,
131141
});
132142
}
@@ -137,6 +147,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
137147
&self,
138148
grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
139149
kind: IllegalMoveOriginKind<'tcx>,
150+
original_path: Place<'tcx>,
140151
move_from: &Place<'tcx>,
141152
bind_to: Local,
142153
match_place: &Option<Place<'tcx>>,
@@ -176,6 +187,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
176187
grouped_errors.push(GroupedMoveError::MovesFromMatchPlace {
177188
span,
178189
move_from: match_place.clone(),
190+
original_path,
179191
kind,
180192
binds_to,
181193
});
@@ -206,6 +218,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
206218
grouped_errors.push(GroupedMoveError::MovesFromPattern {
207219
span: match_span,
208220
move_from: mpi,
221+
original_path,
209222
kind,
210223
binds_to: vec![bind_to],
211224
});
@@ -215,12 +228,23 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
215228

216229
fn report(&mut self, error: GroupedMoveError<'tcx>) {
217230
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-
};
231+
let (span, original_path, kind): (Span, &Place<'tcx>, &IllegalMoveOriginKind) =
232+
match error {
233+
GroupedMoveError::MovesFromMatchPlace {
234+
span,
235+
ref original_path,
236+
ref kind,
237+
..
238+
} |
239+
GroupedMoveError::MovesFromPattern { span, ref original_path, ref kind, .. } |
240+
GroupedMoveError::OtherIllegalMove { span, ref original_path, ref kind } => {
241+
(span, original_path, kind)
242+
},
243+
};
223244
let origin = Origin::Mir;
245+
debug!("report: original_path={:?} span={:?}, kind={:?} \
246+
original_path.is_upvar_field_projection={:?}", original_path, span, kind,
247+
original_path.is_upvar_field_projection(self.mir, &self.tcx));
224248
(
225249
match kind {
226250
IllegalMoveOriginKind::Static => {
@@ -231,22 +255,17 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
231255
// borrow to provide feedback about why this
232256
// was a move rather than a copy.
233257
let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx);
258+
let is_upvar_field_projection =
259+
self.prefixes(&original_path, PrefixSet::All)
260+
.any(|p| p.is_upvar_field_projection(self.mir, &self.tcx)
261+
.is_some());
234262
match ty.sty {
235263
ty::TyArray(..) | ty::TySlice(..) => self
236264
.tcx
237265
.cannot_move_out_of_interior_noncopy(span, ty, None, origin),
238266
ty::TyClosure(def_id, closure_substs)
239-
if !self.mir.upvar_decls.is_empty()
240-
&& {
241-
match place {
242-
Place::Projection(ref proj) => {
243-
proj.base == Place::Local(Local::new(1))
244-
}
245-
Place::Promoted(_) |
246-
Place::Local(_) | Place::Static(_) => unreachable!(),
247-
}
248-
} =>
249-
{
267+
if !self.mir.upvar_decls.is_empty() && is_upvar_field_projection
268+
=> {
250269
let closure_kind_ty =
251270
closure_substs.closure_kind_ty(def_id, self.tcx);
252271
let closure_kind = closure_kind_ty.to_opt_closure_kind();
@@ -262,7 +281,28 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
262281
}
263282
None => bug!("closure kind not inferred by borrowck"),
264283
};
265-
self.tcx.cannot_move_out_of(span, place_description, origin)
284+
debug!("report: closure_kind_ty={:?} closure_kind={:?} \
285+
place_description={:?}", closure_kind_ty, closure_kind,
286+
place_description);
287+
288+
let mut diag = self.tcx.cannot_move_out_of(
289+
span, place_description, origin);
290+
291+
for prefix in self.prefixes(&original_path, PrefixSet::All) {
292+
if let Some(field) = prefix.is_upvar_field_projection(
293+
self.mir, &self.tcx) {
294+
let upvar_decl = &self.mir.upvar_decls[field.index()];
295+
let upvar_hir_id =
296+
upvar_decl.var_hir_id.assert_crate_local();
297+
let upvar_node_id =
298+
self.tcx.hir.hir_to_node_id(upvar_hir_id);
299+
let upvar_span = self.tcx.hir.span(upvar_node_id);
300+
diag.span_label(upvar_span, "captured outer variable");
301+
break;
302+
}
303+
}
304+
305+
diag
266306
}
267307
_ => self
268308
.tcx

‎src/librustc_mir/borrow_check/mutability_errors.rs

+15-27
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,18 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
3636
error_access: AccessKind,
3737
location: Location,
3838
) {
39+
debug!(
40+
"report_mutability_error(\
41+
access_place={:?}, span={:?}, the_place_err={:?}, error_access={:?}, location={:?},\
42+
)",
43+
access_place, span, the_place_err, error_access, location,
44+
);
45+
3946
let mut err;
4047
let item_msg;
4148
let reason;
4249
let access_place_desc = self.describe_place(access_place);
50+
debug!("report_mutability_error: access_place_desc={:?}", access_place_desc);
4351

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

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

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

167+
debug!("report_mutability_error: item_msg={:?}, reason={:?}", item_msg, reason);
168+
158169
// `act` and `acted_on` are strings that let us abstract over
159170
// the verbs used in some diagnostic messages.
160171
let act;
@@ -199,6 +210,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
199210
}
200211
};
201212

213+
debug!("report_mutability_error: act={:?}, acted_on={:?}", act, acted_on);
214+
202215
match the_place_err {
203216
// We want to suggest users use `let mut` for local (user
204217
// variable) mutations...
@@ -382,31 +395,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
382395

383396
err.buffer(&mut self.errors_buffer);
384397
}
385-
386-
// Does this place refer to what the user sees as an upvar
387-
fn is_upvar(&self, place: &Place<'tcx>) -> bool {
388-
match *place {
389-
Place::Projection(box Projection {
390-
ref base,
391-
elem: ProjectionElem::Field(_, _),
392-
}) => {
393-
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
394-
is_closure_or_generator(base_ty)
395-
}
396-
Place::Projection(box Projection {
397-
base:
398-
Place::Projection(box Projection {
399-
ref base,
400-
elem: ProjectionElem::Field(upvar_index, _),
401-
}),
402-
elem: ProjectionElem::Deref,
403-
}) => {
404-
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
405-
is_closure_or_generator(base_ty) && self.mir.upvar_decls[upvar_index.index()].by_ref
406-
}
407-
_ => false,
408-
}
409-
}
410398
}
411399

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

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

+9-6
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

+1-1
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

+2
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

0 commit comments

Comments
 (0)