From 375645abb89d1b98490c744dc980fb96aa076e73 Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 11 Oct 2018 01:19:55 +0200 Subject: [PATCH] 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. --- .../borrow_check/error_reporting.rs | 136 ++++++++++++++++-- .../ui/closure_context/issue-42065.nll.stderr | 11 -- src/test/ui/issues/issue-12127.nll.stderr | 18 +++ ...losures-infer-fnonce-call-twice.nll.stderr | 11 -- ...es-infer-fnonce-move-call-twice.nll.stderr | 11 -- 5 files changed, 140 insertions(+), 47 deletions(-) delete mode 100644 src/test/ui/closure_context/issue-42065.nll.stderr create mode 100644 src/test/ui/issues/issue-12127.nll.stderr delete mode 100644 src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-call-twice.nll.stderr delete mode 100644 src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-move-call-twice.nll.stderr diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 99077ce8b09b..3903506068ed 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -15,11 +15,11 @@ use rustc::hir; use rustc::hir::def_id::DefId; use rustc::middle::region::ScopeTree; use rustc::mir::{ - self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, Field, Local, + self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, Constant, Field, Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceProjection, ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, }; -use rustc::ty; +use rustc::ty::{self, DefIdTree}; use rustc::util::ppaux::with_highlight_region_for_bound_region; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; @@ -131,6 +131,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Origin::Mir, ); + self.add_closure_invoked_twice_with_moved_variable_suggestion( + context.loc, + used_place, + &mut err, + ); + let mut is_loop_move = false; for move_site in &move_site_vec { let move_out = self.move_data.moves[(*move_site).moi]; @@ -1056,16 +1062,118 @@ enum StorageDeadOrDrop<'tcx> { } impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { - // End-user visible description of `place` if one can be found. If the - // place is a temporary for instance, None will be returned. + + /// Adds a suggestion when a closure is invoked twice with a moved variable. + /// + /// ```text + /// note: closure cannot be invoked more than once because it moves the variable `dict` out of + /// its environment + /// --> $DIR/issue-42065.rs:16:29 + /// | + /// LL | for (key, value) in dict { + /// | ^^^^ + /// ``` + pub(super) fn add_closure_invoked_twice_with_moved_variable_suggestion( + &self, + location: Location, + place: &Place<'tcx>, + diag: &mut DiagnosticBuilder<'_>, + ) { + let mut target = place.local(); + debug!( + "add_closure_invoked_twice_with_moved_variable_suggestion: location={:?} place={:?} \ + target={:?}", + location, place, target, + ); + for stmt in &self.mir[location.block].statements[location.statement_index..] { + debug!( + "add_closure_invoked_twice_with_moved_variable_suggestion: stmt={:?} \ + target={:?}", + stmt, target, + ); + if let StatementKind::Assign(into, box Rvalue::Use(from)) = &stmt.kind { + debug!( + "add_closure_invoked_twice_with_moved_variable_suggestion: into={:?} \ + from={:?}", + into, from, + ); + match from { + Operand::Copy(ref place) | + Operand::Move(ref place) if target == place.local() => + target = into.local(), + _ => {}, + } + } + } + + + let terminator = self.mir[location.block].terminator(); + debug!( + "add_closure_invoked_twice_with_moved_variable_suggestion: terminator={:?}", + terminator, + ); + if let TerminatorKind::Call { + func: Operand::Constant(box Constant { + literal: ty::Const { ty: &ty::TyS { sty: ty::TyKind::FnDef(id, _), .. }, .. }, + .. + }), + args, + .. + } = &terminator.kind { + debug!("add_closure_invoked_twice_with_moved_variable_suggestion: id={:?}", id); + if self.infcx.tcx.parent(id) == self.infcx.tcx.lang_items().fn_once_trait() { + let closure = match args.first() { + Some(Operand::Copy(ref place)) | + Some(Operand::Move(ref place)) if target == place.local() => + place.local().unwrap(), + _ => return, + }; + debug!( + "add_closure_invoked_twice_with_moved_variable_suggestion: closure={:?}", + closure, + ); + + if let ty::TyKind::Closure(did, substs) = self.mir.local_decls[closure].ty.sty { + let upvar_tys = substs.upvar_tys(did, self.infcx.tcx); + let node_id = match self.infcx.tcx.hir.as_local_node_id(did) { + Some(node_id) => node_id, + _ => return, + }; + + self.infcx.tcx.with_freevars(node_id, |freevars| { + for (freevar, upvar_ty) in freevars.iter().zip(upvar_tys) { + debug!( + "add_closure_invoked_twice_with_moved_variable_suggestion: \ + freevar={:?} upvar_ty={:?}", + freevar, upvar_ty, + ); + if !upvar_ty.is_region_ptr() { + diag.span_note( + freevar.span, + &format!( + "closure cannot be invoked more than once because it \ + moves the variable `{}` out of its environment", + self.infcx.tcx.hir.name(freevar.var_id()), + ), + ); + } + } + }); + } + } + } + } + + /// End-user visible description of `place` if one can be found. If the + /// place is a temporary for instance, None will be returned. pub(super) fn describe_place(&self, place: &Place<'tcx>) -> Option { self.describe_place_with_options(place, IncludingDowncast(false)) } - // End-user visible description of `place` if one can be found. If the - // place is a temporary for instance, None will be returned. - // `IncludingDowncast` parameter makes the function return `Err` if `ProjectionElem` is - // `Downcast` and `IncludingDowncast` is true + /// End-user visible description of `place` if one can be found. If the + /// place is a temporary for instance, None will be returned. + /// `IncludingDowncast` parameter makes the function return `Err` if `ProjectionElem` is + /// `Downcast` and `IncludingDowncast` is true pub(super) fn describe_place_with_options( &self, place: &Place<'tcx>, @@ -1078,7 +1186,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - // Appends end-user visible description of `place` to `buf`. + /// Appends end-user visible description of `place` to `buf`. fn append_place_to_string( &self, place: &Place<'tcx>, @@ -1213,8 +1321,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Ok(()) } - // Appends end-user visible description of the `local` place to `buf`. If `local` doesn't have - // a name, then `Err` is returned + /// Appends end-user visible description of the `local` place to `buf`. If `local` doesn't have + /// a name, then `Err` is returned fn append_local_to_string(&self, local_index: Local, buf: &mut String) -> Result<(), ()> { let local = &self.mir.local_decls[local_index]; match local.name { @@ -1226,7 +1334,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - // End-user visible description of the `field`nth field of `base` + /// End-user visible description of the `field`nth field of `base` fn describe_field(&self, base: &Place, field: Field) -> String { match *base { Place::Local(local) => { @@ -1251,7 +1359,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - // End-user visible description of the `field_index`nth field of `ty` + /// End-user visible description of the `field_index`nth field of `ty` fn describe_field_from_ty(&self, ty: &ty::Ty, field: Field) -> String { if ty.is_box() { // 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> { } } - // Retrieve type of a place for the current MIR representation + /// Retrieve type of a place for the current MIR representation fn retrieve_type_for_place(&self, place: &Place<'tcx>) -> Option { match place { Place::Local(local) => { diff --git a/src/test/ui/closure_context/issue-42065.nll.stderr b/src/test/ui/closure_context/issue-42065.nll.stderr deleted file mode 100644 index bda8a3b85f75..000000000000 --- a/src/test/ui/closure_context/issue-42065.nll.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0382]: use of moved value: `debug_dump_dict` - --> $DIR/issue-42065.rs:21:5 - | -LL | debug_dump_dict(); - | --------------- value moved here -LL | debug_dump_dict(); - | ^^^^^^^^^^^^^^^ value used here after move - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/issues/issue-12127.nll.stderr b/src/test/ui/issues/issue-12127.nll.stderr new file mode 100644 index 000000000000..51dd2e72832b --- /dev/null +++ b/src/test/ui/issues/issue-12127.nll.stderr @@ -0,0 +1,18 @@ +error[E0382]: use of moved value: `f` + --> $DIR/issue-12127.rs:21:9 + | +LL | f(); + | - value moved here +LL | f(); + | ^ value used here after move + | +note: closure cannot be invoked more than once because it moves the variable `x` out of its environment + --> $DIR/issue-12127.rs:18:39 + | +LL | let f = to_fn_once(move|| do_it(&*x)); + | ^ + = note: move occurs because `f` has type `[closure@$DIR/issue-12127.rs:18:24: 18:41 x:std::boxed::Box]`, which does not implement the `Copy` trait + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-call-twice.nll.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-call-twice.nll.stderr deleted file mode 100644 index f4756696b6b0..000000000000 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-call-twice.nll.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0382]: use of moved value: `tick` - --> $DIR/unboxed-closures-infer-fnonce-call-twice.rs:20:5 - | -LL | tick(); - | ---- value moved here -LL | tick(); //~ ERROR use of moved value: `tick` - | ^^^^ value used here after move - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-move-call-twice.nll.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-move-call-twice.nll.stderr deleted file mode 100644 index 95ed67362787..000000000000 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnonce-move-call-twice.nll.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0382]: use of moved value: `tick` - --> $DIR/unboxed-closures-infer-fnonce-move-call-twice.rs:20:5 - | -LL | tick(); - | ---- value moved here -LL | tick(); //~ ERROR use of moved value: `tick` - | ^^^^ value used here after move - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0382`.