Suggest cloning Arc moved into closure

```
error[E0382]: borrow of moved value: `x`
  --> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
   |
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 |         println!("{}", x);
   |                        - variable moved due to use in closure
LL |     });
LL |     println!("{}", x);
   |                    ^ value borrowed here after move
   |
   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider cloning the value before moving it into the closure
   |
LL ~     let value = x.clone();
LL ~     thread::spawn(move || {
LL ~         println!("{}", value);
   |
```
This commit is contained in:
Esteban Küber
2024-05-01 20:46:06 +00:00
parent 36b21637e9
commit 904652b2d0
13 changed files with 85 additions and 33 deletions

View File

@@ -518,11 +518,11 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
} = move_spans } = move_spans
&& can_suggest_clone && can_suggest_clone
{ {
self.suggest_cloning(err, ty, expr, Some(move_spans)); self.suggest_cloning(err, place.as_ref(), ty, expr, Some(move_spans));
} else if self.suggest_hoisting_call_outside_loop(err, expr) && can_suggest_clone { } else if self.suggest_hoisting_call_outside_loop(err, expr) && can_suggest_clone {
// The place where the type moves would be misleading to suggest clone. // The place where the type moves would be misleading to suggest clone.
// #121466 // #121466
self.suggest_cloning(err, ty, expr, Some(move_spans)); self.suggest_cloning(err, place.as_ref(), ty, expr, Some(move_spans));
} }
} }
@@ -1224,6 +1224,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
pub(crate) fn suggest_cloning( pub(crate) fn suggest_cloning(
&self, &self,
err: &mut Diag<'_>, err: &mut Diag<'_>,
place: PlaceRef<'tcx>,
ty: Ty<'tcx>, ty: Ty<'tcx>,
expr: &'tcx hir::Expr<'tcx>, expr: &'tcx hir::Expr<'tcx>,
use_spans: Option<UseSpans<'tcx>>, use_spans: Option<UseSpans<'tcx>>,
@@ -1238,7 +1239,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
} }
if self.implements_clone(ty) { if self.implements_clone(ty) {
self.suggest_cloning_inner(err, ty, expr); if self.in_move_closure(expr) {
if let Some(name) = self.describe_place(place) {
self.suggest_clone_of_captured_var_in_move_closure(err, &name, use_spans);
}
} else {
self.suggest_cloning_inner(err, ty, expr);
}
} else if let ty::Adt(def, args) = ty.kind() } else if let ty::Adt(def, args) = ty.kind()
&& def.did().as_local().is_some() && def.did().as_local().is_some()
&& def.variants().iter().all(|variant| { && def.variants().iter().all(|variant| {
@@ -1505,7 +1512,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
if let hir::ExprKind::AddrOf(_, _, borrowed_expr) = expr.kind if let hir::ExprKind::AddrOf(_, _, borrowed_expr) = expr.kind
&& let Some(ty) = typeck_results.expr_ty_opt(borrowed_expr) && let Some(ty) = typeck_results.expr_ty_opt(borrowed_expr)
{ {
self.suggest_cloning(&mut err, ty, borrowed_expr, Some(move_spans)); self.suggest_cloning(&mut err, place.as_ref(), ty, borrowed_expr, Some(move_spans));
} else if typeck_results.expr_adjustments(expr).first().is_some_and(|adj| { } else if typeck_results.expr_adjustments(expr).first().is_some_and(|adj| {
matches!( matches!(
adj.kind, adj.kind,
@@ -1518,7 +1525,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
) )
}) && let Some(ty) = typeck_results.expr_ty_opt(expr) }) && let Some(ty) = typeck_results.expr_ty_opt(expr)
{ {
self.suggest_cloning(&mut err, ty, expr, Some(move_spans)); self.suggest_cloning(&mut err, place.as_ref(), ty, expr, Some(move_spans));
} }
} }
self.buffer_error(err); self.buffer_error(err);

View File

@@ -325,25 +325,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
self.cannot_move_out_of(span, &description) self.cannot_move_out_of(span, &description)
} }
fn suggest_clone_of_captured_var_in_move_closure( pub(in crate::diagnostics) fn suggest_clone_of_captured_var_in_move_closure(
&self, &self,
err: &mut Diag<'_>, err: &mut Diag<'_>,
upvar_hir_id: HirId,
upvar_name: &str, upvar_name: &str,
use_spans: Option<UseSpans<'tcx>>, use_spans: Option<UseSpans<'tcx>>,
) { ) {
let tcx = self.infcx.tcx; let tcx = self.infcx.tcx;
let typeck_results = tcx.typeck(self.mir_def_id());
let Some(use_spans) = use_spans else { return }; let Some(use_spans) = use_spans else { return };
// We only care about the case where a closure captured a binding. // We only care about the case where a closure captured a binding.
let UseSpans::ClosureUse { args_span, .. } = use_spans else { return }; let UseSpans::ClosureUse { args_span, .. } = use_spans else { return };
let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else { return }; let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else { return };
// Fetch the type of the expression corresponding to the closure-captured binding.
let Some(captured_ty) = typeck_results.node_type_opt(upvar_hir_id) else { return };
if !self.implements_clone(captured_ty) {
// We only suggest cloning the captured binding if the type can actually be cloned.
return;
};
// Find the closure that captured the binding. // Find the closure that captured the binding.
let mut expr_finder = FindExprBySpan::new(args_span, tcx); let mut expr_finder = FindExprBySpan::new(args_span, tcx);
expr_finder.include_closures = true; expr_finder.include_closures = true;
@@ -396,7 +388,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
.indentation_before(stmt.span) .indentation_before(stmt.span)
.unwrap_or_else(|| " ".to_string()); .unwrap_or_else(|| " ".to_string());
err.multipart_suggestion_verbose( err.multipart_suggestion_verbose(
"clone the value before moving it into the closure", "consider cloning the value before moving it into the closure",
vec![ vec![
( (
stmt.span.shrink_to_lo(), stmt.span.shrink_to_lo(),
@@ -426,7 +418,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
.indentation_before(closure_expr.span) .indentation_before(closure_expr.span)
.unwrap_or_else(|| " ".to_string()); .unwrap_or_else(|| " ".to_string());
err.multipart_suggestion_verbose( err.multipart_suggestion_verbose(
"clone the value before moving it into the closure", "consider cloning the value before moving it into the closure",
vec![ vec![
( (
closure_expr.span.shrink_to_lo(), closure_expr.span.shrink_to_lo(),
@@ -519,20 +511,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
); );
let closure_span = tcx.def_span(def_id); let closure_span = tcx.def_span(def_id);
let mut err = self self.cannot_move_out_of(span, &place_description)
.cannot_move_out_of(span, &place_description)
.with_span_label(upvar_span, "captured outer variable") .with_span_label(upvar_span, "captured outer variable")
.with_span_label( .with_span_label(
closure_span, closure_span,
format!("captured by this `{closure_kind}` closure"), format!("captured by this `{closure_kind}` closure"),
); )
self.suggest_clone_of_captured_var_in_move_closure(
&mut err,
upvar_hir_id,
&upvar_name,
use_spans,
);
err
} }
_ => { _ => {
let source = self.borrowed_content_source(deref_base); let source = self.borrowed_content_source(deref_base);
@@ -593,7 +577,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}; };
if let Some(expr) = self.find_expr(span) { if let Some(expr) = self.find_expr(span) {
self.suggest_cloning(err, place_ty, expr, None); self.suggest_cloning(err, move_from.as_ref(), place_ty, expr, None);
} }
err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label { err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
@@ -625,7 +609,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}; };
if let Some(expr) = self.find_expr(use_span) { if let Some(expr) = self.find_expr(use_span) {
self.suggest_cloning(err, place_ty, expr, Some(use_spans)); self.suggest_cloning(
err,
original_path.as_ref(),
place_ty,
expr,
Some(use_spans),
);
} }
err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label { err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
@@ -828,7 +818,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
let place_desc = &format!("`{}`", self.local_names[*local].unwrap()); let place_desc = &format!("`{}`", self.local_names[*local].unwrap());
if let Some(expr) = self.find_expr(binding_span) { if let Some(expr) = self.find_expr(binding_span) {
self.suggest_cloning(err, bind_to.ty, expr, None); let local_place: PlaceRef<'tcx> = (*local).into();
self.suggest_cloning(err, local_place, bind_to.ty, expr, None);
} }
err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label { err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {

View File

@@ -12,7 +12,7 @@ LL | let _h = to_fn_once(move || -> isize { *bar });
| | move occurs because `bar` has type `Box<isize>`, which does not implement the `Copy` trait | | move occurs because `bar` has type `Box<isize>`, which does not implement the `Copy` trait
| `bar` is moved here | `bar` is moved here
| |
help: clone the value before moving it into the closure help: consider cloning the value before moving it into the closure
| |
LL ~ let value = bar.clone(); LL ~ let value = bar.clone();
LL ~ let _h = to_fn_once(move || -> isize { value }); LL ~ let _h = to_fn_once(move || -> isize { value });

View File

@@ -12,6 +12,12 @@ LL | call_f(move|| { *t + 1 });
| ^^^^^^ -- use occurs due to use in closure | ^^^^^^ -- use occurs due to use in closure
| | | |
| value used here after move | value used here after move
|
help: consider cloning the value before moving it into the closure
|
LL ~ let value = t.clone();
LL ~ call_f(move|| { value + 1 });
|
error: aborting due to 1 previous error error: aborting due to 1 previous error

View File

@@ -0,0 +1,11 @@
//@ run-rustfix
use std::thread;
fn main() {
let x = "Hello world!".to_string();
let value = x.clone();
thread::spawn(move || {
println!("{}", value);
});
println!("{}", x); //~ ERROR borrow of moved value
}

View File

@@ -1,3 +1,4 @@
//@ run-rustfix
use std::thread; use std::thread;
fn main() { fn main() {

View File

@@ -1,5 +1,5 @@
error[E0382]: borrow of moved value: `x` error[E0382]: borrow of moved value: `x`
--> $DIR/moves-based-on-type-capture-clause-bad.rs:8:20 --> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
| |
LL | let x = "Hello world!".to_string(); LL | let x = "Hello world!".to_string();
| - move occurs because `x` has type `String`, which does not implement the `Copy` trait | - move occurs because `x` has type `String`, which does not implement the `Copy` trait
@@ -12,6 +12,12 @@ LL | println!("{}", x);
| ^ value borrowed here after move | ^ value borrowed here after move
| |
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider cloning the value before moving it into the closure
|
LL ~ let value = x.clone();
LL ~ thread::spawn(move || {
LL ~ println!("{}", value);
|
error: aborting due to 1 previous error error: aborting due to 1 previous error

View File

@@ -13,6 +13,12 @@ LL | assert_eq!((*arc_v)[2], 3);
| ^^^^^ value borrowed here after move | ^^^^^ value borrowed here after move
| |
= note: borrow occurs due to deref coercion to `Vec<i32>` = note: borrow occurs due to deref coercion to `Vec<i32>`
help: consider cloning the value before moving it into the closure
|
LL ~ let value = arc_v.clone();
LL ~ thread::spawn(move|| {
LL ~ assert_eq!((*value)[3], 4);
|
error: aborting due to 1 previous error error: aborting due to 1 previous error

View File

@@ -0,0 +1,17 @@
//@ run-rustfix
use std::sync::Arc;
use std::thread;
fn main() {
let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
let arc_v = Arc::new(v);
let value = arc_v.clone();
thread::spawn(move|| {
assert_eq!((*value)[3], 4);
});
assert_eq!((*arc_v)[2], 3); //~ ERROR borrow of moved value: `arc_v`
println!("{:?}", *arc_v);
}

View File

@@ -1,3 +1,4 @@
//@ run-rustfix
use std::sync::Arc; use std::sync::Arc;
use std::thread; use std::thread;

View File

@@ -1,5 +1,5 @@
error[E0382]: borrow of moved value: `arc_v` error[E0382]: borrow of moved value: `arc_v`
--> $DIR/no-reuse-move-arc.rs:12:18 --> $DIR/no-reuse-move-arc.rs:13:18
| |
LL | let arc_v = Arc::new(v); LL | let arc_v = Arc::new(v);
| ----- move occurs because `arc_v` has type `Arc<Vec<i32>>`, which does not implement the `Copy` trait | ----- move occurs because `arc_v` has type `Arc<Vec<i32>>`, which does not implement the `Copy` trait
@@ -13,6 +13,12 @@ LL | assert_eq!((*arc_v)[2], 3);
| ^^^^^ value borrowed here after move | ^^^^^ value borrowed here after move
| |
= note: borrow occurs due to deref coercion to `Vec<i32>` = note: borrow occurs due to deref coercion to `Vec<i32>`
help: consider cloning the value before moving it into the closure
|
LL ~ let value = arc_v.clone();
LL ~ thread::spawn(move|| {
LL ~ assert_eq!((*value)[3], 4);
|
error: aborting due to 1 previous error error: aborting due to 1 previous error

View File

@@ -79,7 +79,7 @@ LL | let x = var;
| variable moved due to use in closure | variable moved due to use in closure
| move occurs because `var` has type `NotCopyableButCloneable`, which does not implement the `Copy` trait | move occurs because `var` has type `NotCopyableButCloneable`, which does not implement the `Copy` trait
| |
help: clone the value before moving it into the closure help: consider cloning the value before moving it into the closure
| |
LL ~ { LL ~ {
LL + let value = var.clone(); LL + let value = var.clone();