Skip to content

Commit 375645a

Browse files
committed
Add by-value captured variable note on second use.
This commit adds a note that was present in the AST borrow checker when closures are invoked more than once and have captured variables by-value.
1 parent aa70115 commit 375645a

File tree

5 files changed

+140
-47
lines changed

5 files changed

+140
-47
lines changed

src/librustc_mir/borrow_check/error_reporting.rs

+122-14
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ use rustc::hir;
1515
use rustc::hir::def_id::DefId;
1616
use rustc::middle::region::ScopeTree;
1717
use rustc::mir::{
18-
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, Field, Local,
18+
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, Constant, Field, Local,
1919
LocalDecl, LocalKind, Location, Operand, Place, PlaceProjection, ProjectionElem,
2020
Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm,
2121
};
22-
use rustc::ty;
22+
use rustc::ty::{self, DefIdTree};
2323
use rustc::util::ppaux::with_highlight_region_for_bound_region;
2424
use rustc_data_structures::fx::FxHashSet;
2525
use rustc_data_structures::sync::Lrc;
@@ -131,6 +131,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
131131
Origin::Mir,
132132
);
133133

134+
self.add_closure_invoked_twice_with_moved_variable_suggestion(
135+
context.loc,
136+
used_place,
137+
&mut err,
138+
);
139+
134140
let mut is_loop_move = false;
135141
for move_site in &move_site_vec {
136142
let move_out = self.move_data.moves[(*move_site).moi];
@@ -1056,16 +1062,118 @@ enum StorageDeadOrDrop<'tcx> {
10561062
}
10571063

10581064
impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
1059-
// End-user visible description of `place` if one can be found. If the
1060-
// place is a temporary for instance, None will be returned.
1065+
1066+
/// Adds a suggestion when a closure is invoked twice with a moved variable.
1067+
///
1068+
/// ```text
1069+
/// note: closure cannot be invoked more than once because it moves the variable `dict` out of
1070+
/// its environment
1071+
/// --> $DIR/issue-42065.rs:16:29
1072+
/// |
1073+
/// LL | for (key, value) in dict {
1074+
/// | ^^^^
1075+
/// ```
1076+
pub(super) fn add_closure_invoked_twice_with_moved_variable_suggestion(
1077+
&self,
1078+
location: Location,
1079+
place: &Place<'tcx>,
1080+
diag: &mut DiagnosticBuilder<'_>,
1081+
) {
1082+
let mut target = place.local();
1083+
debug!(
1084+
"add_closure_invoked_twice_with_moved_variable_suggestion: location={:?} place={:?} \
1085+
target={:?}",
1086+
location, place, target,
1087+
);
1088+
for stmt in &self.mir[location.block].statements[location.statement_index..] {
1089+
debug!(
1090+
"add_closure_invoked_twice_with_moved_variable_suggestion: stmt={:?} \
1091+
target={:?}",
1092+
stmt, target,
1093+
);
1094+
if let StatementKind::Assign(into, box Rvalue::Use(from)) = &stmt.kind {
1095+
debug!(
1096+
"add_closure_invoked_twice_with_moved_variable_suggestion: into={:?} \
1097+
from={:?}",
1098+
into, from,
1099+
);
1100+
match from {
1101+
Operand::Copy(ref place) |
1102+
Operand::Move(ref place) if target == place.local() =>
1103+
target = into.local(),
1104+
_ => {},
1105+
}
1106+
}
1107+
}
1108+
1109+
1110+
let terminator = self.mir[location.block].terminator();
1111+
debug!(
1112+
"add_closure_invoked_twice_with_moved_variable_suggestion: terminator={:?}",
1113+
terminator,
1114+
);
1115+
if let TerminatorKind::Call {
1116+
func: Operand::Constant(box Constant {
1117+
literal: ty::Const { ty: &ty::TyS { sty: ty::TyKind::FnDef(id, _), .. }, .. },
1118+
..
1119+
}),
1120+
args,
1121+
..
1122+
} = &terminator.kind {
1123+
debug!("add_closure_invoked_twice_with_moved_variable_suggestion: id={:?}", id);
1124+
if self.infcx.tcx.parent(id) == self.infcx.tcx.lang_items().fn_once_trait() {
1125+
let closure = match args.first() {
1126+
Some(Operand::Copy(ref place)) |
1127+
Some(Operand::Move(ref place)) if target == place.local() =>
1128+
place.local().unwrap(),
1129+
_ => return,
1130+
};
1131+
debug!(
1132+
"add_closure_invoked_twice_with_moved_variable_suggestion: closure={:?}",
1133+
closure,
1134+
);
1135+
1136+
if let ty::TyKind::Closure(did, substs) = self.mir.local_decls[closure].ty.sty {
1137+
let upvar_tys = substs.upvar_tys(did, self.infcx.tcx);
1138+
let node_id = match self.infcx.tcx.hir.as_local_node_id(did) {
1139+
Some(node_id) => node_id,
1140+
_ => return,
1141+
};
1142+
1143+
self.infcx.tcx.with_freevars(node_id, |freevars| {
1144+
for (freevar, upvar_ty) in freevars.iter().zip(upvar_tys) {
1145+
debug!(
1146+
"add_closure_invoked_twice_with_moved_variable_suggestion: \
1147+
freevar={:?} upvar_ty={:?}",
1148+
freevar, upvar_ty,
1149+
);
1150+
if !upvar_ty.is_region_ptr() {
1151+
diag.span_note(
1152+
freevar.span,
1153+
&format!(
1154+
"closure cannot be invoked more than once because it \
1155+
moves the variable `{}` out of its environment",
1156+
self.infcx.tcx.hir.name(freevar.var_id()),
1157+
),
1158+
);
1159+
}
1160+
}
1161+
});
1162+
}
1163+
}
1164+
}
1165+
}
1166+
1167+
/// End-user visible description of `place` if one can be found. If the
1168+
/// place is a temporary for instance, None will be returned.
10611169
pub(super) fn describe_place(&self, place: &Place<'tcx>) -> Option<String> {
10621170
self.describe_place_with_options(place, IncludingDowncast(false))
10631171
}
10641172

1065-
// End-user visible description of `place` if one can be found. If the
1066-
// place is a temporary for instance, None will be returned.
1067-
// `IncludingDowncast` parameter makes the function return `Err` if `ProjectionElem` is
1068-
// `Downcast` and `IncludingDowncast` is true
1173+
/// End-user visible description of `place` if one can be found. If the
1174+
/// place is a temporary for instance, None will be returned.
1175+
/// `IncludingDowncast` parameter makes the function return `Err` if `ProjectionElem` is
1176+
/// `Downcast` and `IncludingDowncast` is true
10691177
pub(super) fn describe_place_with_options(
10701178
&self,
10711179
place: &Place<'tcx>,
@@ -1078,7 +1186,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
10781186
}
10791187
}
10801188

1081-
// Appends end-user visible description of `place` to `buf`.
1189+
/// Appends end-user visible description of `place` to `buf`.
10821190
fn append_place_to_string(
10831191
&self,
10841192
place: &Place<'tcx>,
@@ -1213,8 +1321,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
12131321
Ok(())
12141322
}
12151323

1216-
// Appends end-user visible description of the `local` place to `buf`. If `local` doesn't have
1217-
// a name, then `Err` is returned
1324+
/// Appends end-user visible description of the `local` place to `buf`. If `local` doesn't have
1325+
/// a name, then `Err` is returned
12181326
fn append_local_to_string(&self, local_index: Local, buf: &mut String) -> Result<(), ()> {
12191327
let local = &self.mir.local_decls[local_index];
12201328
match local.name {
@@ -1226,7 +1334,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
12261334
}
12271335
}
12281336

1229-
// End-user visible description of the `field`nth field of `base`
1337+
/// End-user visible description of the `field`nth field of `base`
12301338
fn describe_field(&self, base: &Place, field: Field) -> String {
12311339
match *base {
12321340
Place::Local(local) => {
@@ -1251,7 +1359,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
12511359
}
12521360
}
12531361

1254-
// End-user visible description of the `field_index`nth field of `ty`
1362+
/// End-user visible description of the `field_index`nth field of `ty`
12551363
fn describe_field_from_ty(&self, ty: &ty::Ty, field: Field) -> String {
12561364
if ty.is_box() {
12571365
// If the type is a box, the field is described from the boxed type
@@ -1294,7 +1402,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
12941402
}
12951403
}
12961404

1297-
// Retrieve type of a place for the current MIR representation
1405+
/// Retrieve type of a place for the current MIR representation
12981406
fn retrieve_type_for_place(&self, place: &Place<'tcx>) -> Option<ty::Ty> {
12991407
match place {
13001408
Place::Local(local) => {

src/test/ui/closure_context/issue-42065.nll.stderr

-11
This file was deleted.
+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error[E0382]: use of moved value: `f`
2+
--> $DIR/issue-12127.rs:21:9
3+
|
4+
LL | f();
5+
| - value moved here
6+
LL | f();
7+
| ^ value used here after move
8+
|
9+
note: closure cannot be invoked more than once because it moves the variable `x` out of its environment
10+
--> $DIR/issue-12127.rs:18:39
11+
|
12+
LL | let f = to_fn_once(move|| do_it(&*x));
13+
| ^
14+
= note: move occurs because `f` has type `[closure@$DIR/issue-12127.rs:18:24: 18:41 x:std::boxed::Box<isize>]`, which does not implement the `Copy` trait
15+
16+
error: aborting due to previous error
17+
18+
For more information about this error, try `rustc --explain E0382`.

src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-call-twice.nll.stderr

-11
This file was deleted.

src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-move-call-twice.nll.stderr

-11
This file was deleted.

0 commit comments

Comments
 (0)