diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 662bdce197f..ae17caf75b6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -203,13 +203,7 @@ pub(crate) fn report_use_of_moved_or_uninitialized( if !seen_spans.contains(&move_span) { if !closure { - self.suggest_ref_or_clone( - mpi, - move_span, - &mut err, - &mut in_pattern, - move_spans, - ); + self.suggest_ref_or_clone(mpi, &mut err, &mut in_pattern, move_spans); } let msg_opt = CapturedMessageOpt { @@ -351,18 +345,28 @@ pub(crate) fn report_use_of_moved_or_uninitialized( fn suggest_ref_or_clone( &self, mpi: MovePathIndex, - move_span: Span, err: &mut Diag<'tcx>, in_pattern: &mut bool, move_spans: UseSpans<'_>, ) { + let move_span = match move_spans { + UseSpans::ClosureUse { capture_kind_span, .. } => capture_kind_span, + _ => move_spans.args_or_use(), + }; struct ExpressionFinder<'hir> { expr_span: Span, expr: Option<&'hir hir::Expr<'hir>>, pat: Option<&'hir hir::Pat<'hir>>, parent_pat: Option<&'hir hir::Pat<'hir>>, + hir: rustc_middle::hir::map::Map<'hir>, } impl<'hir> Visitor<'hir> for ExpressionFinder<'hir> { + type NestedFilter = OnlyBodies; + + fn nested_visit_map(&mut self) -> Self::Map { + self.hir + } + fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) { if e.span == self.expr_span { self.expr = Some(e); @@ -397,8 +401,13 @@ fn visit_pat(&mut self, p: &'hir hir::Pat<'hir>) { let expr = hir.body(body_id).value; let place = &self.move_data.move_paths[mpi].place; let span = place.as_local().map(|local| self.body.local_decls[local].source_info.span); - let mut finder = - ExpressionFinder { expr_span: move_span, expr: None, pat: None, parent_pat: None }; + let mut finder = ExpressionFinder { + expr_span: move_span, + expr: None, + pat: None, + parent_pat: None, + hir, + }; finder.visit_expr(expr); if let Some(span) = span && let Some(expr) = finder.expr @@ -479,12 +488,10 @@ fn visit_pat(&mut self, p: &'hir hir::Pat<'hir>) { } else if let UseSpans::ClosureUse { closure_kind: ClosureKind::Coroutine(CoroutineKind::Desugared(_, CoroutineSource::Block)), - args_span: _, - capture_kind_span: _, - path_span, + .. } = move_spans { - self.suggest_cloning(err, ty, expr, path_span); + self.suggest_cloning(err, ty, expr, None); } else if self.suggest_hoisting_call_outside_loop(err, expr) { // The place where the the type moves would be misleading to suggest clone. // #121466 @@ -1233,6 +1240,18 @@ pub(crate) fn clone_on_reference(&self, expr: &hir::Expr<'_>) -> Option { } } + fn in_move_closure(&self, expr: &hir::Expr<'_>) -> bool { + for (_, node) in self.infcx.tcx.hir().parent_iter(expr.hir_id) { + if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(closure), .. }) = node + && let hir::CaptureBy::Value { .. } = closure.capture_clause + { + // `move || x.clone()` will not work. FIXME: suggest `let y = x.clone(); move || y` + return true; + } + } + false + } + fn suggest_cloning_inner( &self, err: &mut Diag<'_>, @@ -1245,6 +1264,9 @@ fn suggest_cloning_inner( // See `tests/ui/moves/needs-clone-through-deref.rs` return false; } + if self.in_move_closure(expr) { + return false; + } // Try to find predicates on *generic params* that would allow copying `ty` let suggestion = if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) { diff --git a/tests/ui/borrowck/borrowck-closures-slice-patterns.stderr b/tests/ui/borrowck/borrowck-closures-slice-patterns.stderr index 411d85b8e05..9066891d298 100644 --- a/tests/ui/borrowck/borrowck-closures-slice-patterns.stderr +++ b/tests/ui/borrowck/borrowck-closures-slice-patterns.stderr @@ -38,6 +38,11 @@ LL | let [y, z @ ..] = x; LL | }; LL | &x; | ^^ value borrowed here after move + | +help: consider cloning the value if the performance cost is acceptable + | +LL | let [y, z @ ..] = x.clone(); + | ++++++++ error[E0502]: cannot borrow `*x` as mutable because it is also borrowed as immutable --> $DIR/borrowck-closures-slice-patterns.rs:33:13 @@ -79,6 +84,12 @@ LL | let [y, z @ ..] = *x; LL | }; LL | &x; | ^^ value borrowed here after move + | +help: consider cloning the value if the performance cost is acceptable + | +LL - let [y, z @ ..] = *x; +LL + let [y, z @ ..] = x.clone(); + | error[E0502]: cannot borrow `*x` as mutable because it is also borrowed as immutable --> $DIR/borrowck-closures-slice-patterns.rs:59:13 diff --git a/tests/ui/borrowck/issue-101119.stderr b/tests/ui/borrowck/issue-101119.stderr index 1f32ece3d3d..b4775496f4f 100644 --- a/tests/ui/borrowck/issue-101119.stderr +++ b/tests/ui/borrowck/issue-101119.stderr @@ -4,11 +4,25 @@ error[E0382]: use of moved value: `state` LL | fn fill_memory_blocks_mt(state: &mut State) { | ----- move occurs because `state` has type `&mut State`, which does not implement the `Copy` trait LL | loop { + | ---- inside of this loop LL | once(move || { | ^^^^^^^ value moved into closure here, in previous iteration of loop LL | LL | fill_segment(state); | ----- use occurs due to use in closure + | +note: consider changing this parameter type in function `fill_segment` to borrow instead if owning the value isn't necessary + --> $DIR/issue-101119.rs:14:20 + | +LL | fn fill_segment(_: &mut State) {} + | ------------ ^^^^^^^^^^ this parameter takes ownership of the value + | | + | in this function +note: if `State` implemented `Clone`, you could clone the value + --> $DIR/issue-101119.rs:1:1 + | +LL | struct State; + | ^^^^^^^^^^^^ error: aborting due to 1 previous error diff --git a/tests/ui/issues/issue-24357.rs b/tests/ui/issues/issue-24357.rs index 152e69ebc87..d1a9e37251e 100644 --- a/tests/ui/issues/issue-24357.rs +++ b/tests/ui/issues/issue-24357.rs @@ -1,4 +1,4 @@ -struct NoCopy; +struct NoCopy; //~ NOTE if `NoCopy` implemented `Clone`, you could clone the value fn main() { let x = NoCopy; //~^ NOTE move occurs because `x` has type `NoCopy` diff --git a/tests/ui/issues/issue-24357.stderr b/tests/ui/issues/issue-24357.stderr index 08a5a8ac56e..6d50eea7e21 100644 --- a/tests/ui/issues/issue-24357.stderr +++ b/tests/ui/issues/issue-24357.stderr @@ -11,6 +11,12 @@ LL | let f = move || { let y = x; }; ... LL | let z = x; | ^ value used here after move + | +note: if `NoCopy` implemented `Clone`, you could clone the value + --> $DIR/issue-24357.rs:1:1 + | +LL | struct NoCopy; + | ^^^^^^^^^^^^^ error: aborting due to 1 previous error diff --git a/tests/ui/moves/issue-75904-move-closure-loop.stderr b/tests/ui/moves/issue-75904-move-closure-loop.stderr index 6f04105a35e..b6ad906bbdb 100644 --- a/tests/ui/moves/issue-75904-move-closure-loop.stderr +++ b/tests/ui/moves/issue-75904-move-closure-loop.stderr @@ -4,11 +4,18 @@ error[E0382]: use of moved value: `a` LL | let mut a = NotCopy; | ----- move occurs because `a` has type `NotCopy`, which does not implement the `Copy` trait LL | loop { + | ---- inside of this loop LL | || { | ^^ value moved into closure here, in previous iteration of loop LL | &mut a; LL | a; | - use occurs due to use in closure + | +note: if `NotCopy` implemented `Clone`, you could clone the value + --> $DIR/issue-75904-move-closure-loop.rs:5:1 + | +LL | struct NotCopy; + | ^^^^^^^^^^^^^^ error: aborting due to 1 previous error diff --git a/tests/ui/moves/moves-based-on-type-capture-clause-bad.rs b/tests/ui/moves/moves-based-on-type-capture-clause-bad.rs index b2f68352f89..9d7277c1c24 100644 --- a/tests/ui/moves/moves-based-on-type-capture-clause-bad.rs +++ b/tests/ui/moves/moves-based-on-type-capture-clause-bad.rs @@ -2,7 +2,7 @@ fn main() { let x = "Hello world!".to_string(); - thread::spawn(move|| { + thread::spawn(move || { println!("{}", x); }); println!("{}", x); //~ ERROR borrow of moved value diff --git a/tests/ui/moves/moves-based-on-type-capture-clause-bad.stderr b/tests/ui/moves/moves-based-on-type-capture-clause-bad.stderr index 5e527bf445e..c2b9aeab237 100644 --- a/tests/ui/moves/moves-based-on-type-capture-clause-bad.stderr +++ b/tests/ui/moves/moves-based-on-type-capture-clause-bad.stderr @@ -3,8 +3,8 @@ error[E0382]: borrow of moved value: `x` | LL | let x = "Hello world!".to_string(); | - move occurs because `x` has type `String`, which does not implement the `Copy` trait -LL | thread::spawn(move|| { - | ------ value moved into closure here +LL | thread::spawn(move || { + | ------- value moved into closure here LL | println!("{}", x); | - variable moved due to use in closure LL | }); diff --git a/tests/ui/nll/closure-move-spans.fixed b/tests/ui/nll/closure-move-spans.fixed new file mode 100644 index 00000000000..edd74e434e0 --- /dev/null +++ b/tests/ui/nll/closure-move-spans.fixed @@ -0,0 +1,23 @@ +// check that moves due to a closure capture give a special note +//@ run-rustfix +#![allow(unused_variables, unused_must_use, dead_code)] + +fn move_after_move(x: String) { + || x.clone(); + let y = x; //~ ERROR +} + +fn borrow_after_move(x: String) { + || x.clone(); + let y = &x; //~ ERROR +} + +fn borrow_mut_after_move(mut x: String) { + || x.clone(); + let y = &mut x; //~ ERROR +} + +fn fn_ref(f: F) -> F { f } +fn fn_mut(f: F) -> F { f } + +fn main() {} diff --git a/tests/ui/nll/closure-move-spans.rs b/tests/ui/nll/closure-move-spans.rs index bf2431870a9..bba5c3776e6 100644 --- a/tests/ui/nll/closure-move-spans.rs +++ b/tests/ui/nll/closure-move-spans.rs @@ -1,4 +1,6 @@ // check that moves due to a closure capture give a special note +//@ run-rustfix +#![allow(unused_variables, unused_must_use, dead_code)] fn move_after_move(x: String) { || x; diff --git a/tests/ui/nll/closure-move-spans.stderr b/tests/ui/nll/closure-move-spans.stderr index 0446ef7b066..0b1da57605c 100644 --- a/tests/ui/nll/closure-move-spans.stderr +++ b/tests/ui/nll/closure-move-spans.stderr @@ -1,5 +1,5 @@ error[E0382]: use of moved value: `x` - --> $DIR/closure-move-spans.rs:5:13 + --> $DIR/closure-move-spans.rs:7:13 | LL | fn move_after_move(x: String) { | - move occurs because `x` has type `String`, which does not implement the `Copy` trait @@ -9,9 +9,14 @@ LL | || x; | value moved into closure here LL | let y = x; | ^ value used here after move + | +help: consider cloning the value if the performance cost is acceptable + | +LL | || x.clone(); + | ++++++++ error[E0382]: borrow of moved value: `x` - --> $DIR/closure-move-spans.rs:10:13 + --> $DIR/closure-move-spans.rs:12:13 | LL | fn borrow_after_move(x: String) { | - move occurs because `x` has type `String`, which does not implement the `Copy` trait @@ -21,9 +26,14 @@ LL | || x; | value moved into closure here LL | let y = &x; | ^^ value borrowed here after move + | +help: consider cloning the value if the performance cost is acceptable + | +LL | || x.clone(); + | ++++++++ error[E0382]: borrow of moved value: `x` - --> $DIR/closure-move-spans.rs:15:13 + --> $DIR/closure-move-spans.rs:17:13 | LL | fn borrow_mut_after_move(mut x: String) { | ----- move occurs because `x` has type `String`, which does not implement the `Copy` trait @@ -33,6 +43,11 @@ LL | || x; | value moved into closure here LL | let y = &mut x; | ^^^^^^ value borrowed here after move + | +help: consider cloning the value if the performance cost is acceptable + | +LL | || x.clone(); + | ++++++++ error: aborting due to 3 previous errors diff --git a/tests/ui/nll/closures-in-loops.stderr b/tests/ui/nll/closures-in-loops.stderr index 2c1008c516c..050b220e626 100644 --- a/tests/ui/nll/closures-in-loops.stderr +++ b/tests/ui/nll/closures-in-loops.stderr @@ -4,10 +4,16 @@ error[E0382]: use of moved value: `x` LL | fn repreated_move(x: String) { | - move occurs because `x` has type `String`, which does not implement the `Copy` trait LL | for i in 0..10 { + | -------------- inside of this loop LL | || x; | ^^ - use occurs due to use in closure | | | value moved into closure here, in previous iteration of loop + | +help: consider cloning the value if the performance cost is acceptable + | +LL | || x.clone(); + | ++++++++ error[E0499]: cannot borrow `x` as mutable more than once at a time --> $DIR/closures-in-loops.rs:13:16 diff --git a/tests/ui/nll/issue-27282-move-match-input-into-guard.stderr b/tests/ui/nll/issue-27282-move-match-input-into-guard.stderr index ae797800457..39ec45b20ea 100644 --- a/tests/ui/nll/issue-27282-move-match-input-into-guard.stderr +++ b/tests/ui/nll/issue-27282-move-match-input-into-guard.stderr @@ -10,6 +10,11 @@ LL | _ if { (|| { let bar = b; *bar = false; })(); | -- - variable moved due to use in closure | | | value moved into closure here + | +help: consider cloning the value if the performance cost is acceptable + | +LL | _ if { (|| { let bar = b.clone(); *bar = false; })(); + | ++++++++ error[E0382]: use of moved value: `b` --> $DIR/issue-27282-move-match-input-into-guard.rs:24:5 @@ -23,6 +28,11 @@ LL | (|| { let bar = b; *bar = false; })(); | -- - variable moved due to use in closure | | | value moved into closure here + | +help: consider cloning the value if the performance cost is acceptable + | +LL | (|| { let bar = b.clone(); *bar = false; })(); + | ++++++++ error: aborting due to 2 previous errors diff --git a/tests/ui/unboxed-closures/unboxed-closure-illegal-move.stderr b/tests/ui/unboxed-closures/unboxed-closure-illegal-move.stderr index 5b995ff1585..cf4391311d0 100644 --- a/tests/ui/unboxed-closures/unboxed-closure-illegal-move.stderr +++ b/tests/ui/unboxed-closures/unboxed-closure-illegal-move.stderr @@ -37,11 +37,6 @@ LL | let f = to_fn(move || drop(x)); | ------- ^ move occurs because `x` has type `Box`, which does not implement the `Copy` trait | | | captured by this `Fn` closure - | -help: consider cloning the value if the performance cost is acceptable - | -LL | let f = to_fn(move || drop(x.clone())); - | ++++++++ error[E0507]: cannot move out of `x`, a captured variable in an `FnMut` closure --> $DIR/unboxed-closure-illegal-move.rs:32:40 @@ -52,11 +47,6 @@ LL | let f = to_fn_mut(move || drop(x)); | ------- ^ move occurs because `x` has type `Box`, which does not implement the `Copy` trait | | | captured by this `FnMut` closure - | -help: consider cloning the value if the performance cost is acceptable - | -LL | let f = to_fn_mut(move || drop(x.clone())); - | ++++++++ error: aborting due to 4 previous errors