Skip to content

Commit 0e39707

Browse files
committed
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: clone the value before moving it into the closure | LL ~ let value = x.clone(); LL ~ thread::spawn(move || { LL ~ println!("{}", value); | ``` Fix rust-lang#104232.
1 parent 20aa2d8 commit 0e39707

10 files changed

+89
-28
lines changed

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+18-4
Original file line numberDiff line numberDiff line change
@@ -492,11 +492,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
492492
..
493493
} = move_spans
494494
{
495-
self.suggest_cloning(err, ty, expr, None, Some(move_spans));
495+
self.suggest_cloning(err, place.as_ref(), ty, expr, None, Some(move_spans));
496496
} else if self.suggest_hoisting_call_outside_loop(err, expr) {
497497
// The place where the the type moves would be misleading to suggest clone.
498498
// #121466
499-
self.suggest_cloning(err, ty, expr, None, Some(move_spans));
499+
self.suggest_cloning(err, place.as_ref(), ty, expr, None, Some(move_spans));
500500
}
501501
}
502502
if let Some(pat) = finder.pat {
@@ -1083,6 +1083,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
10831083
pub(crate) fn suggest_cloning(
10841084
&self,
10851085
err: &mut Diag<'_>,
1086+
place: PlaceRef<'tcx>,
10861087
ty: Ty<'tcx>,
10871088
mut expr: &'cx hir::Expr<'cx>,
10881089
mut other_expr: Option<&'cx hir::Expr<'cx>>,
@@ -1189,7 +1190,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
11891190
}
11901191
let ty = ty.peel_refs();
11911192
if self.implements_clone(ty) {
1192-
self.suggest_cloning_inner(err, ty, expr);
1193+
if self.in_move_closure(expr) {
1194+
if let Some(name) = self.describe_place(place) {
1195+
self.suggest_clone_of_captured_var_in_move_closure(err, &name, use_spans);
1196+
}
1197+
} else {
1198+
self.suggest_cloning_inner(err, ty, expr);
1199+
}
11931200
} else if let ty::Adt(def, args) = ty.kind()
11941201
&& def.did().as_local().is_some()
11951202
&& def.variants().iter().all(|variant| {
@@ -1441,7 +1448,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
14411448
if let Some(expr) = self.find_expr(borrow_span)
14421449
&& let Some(ty) = typeck_results.node_type_opt(expr.hir_id)
14431450
{
1444-
self.suggest_cloning(&mut err, ty, expr, self.find_expr(span), Some(move_spans));
1451+
self.suggest_cloning(
1452+
&mut err,
1453+
place.as_ref(),
1454+
ty,
1455+
expr,
1456+
self.find_expr(span),
1457+
Some(move_spans),
1458+
);
14451459
}
14461460
self.buffer_error(err);
14471461
}

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

+15-22
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use rustc_errors::{Applicability, Diag};
55
use rustc_hir::intravisit::Visitor;
6-
use rustc_hir::{CaptureBy, ExprKind, HirId, Node};
6+
use rustc_hir::{CaptureBy, ExprKind, Node};
77
use rustc_middle::bug;
88
use rustc_middle::mir::*;
99
use rustc_middle::ty::{self, Ty};
@@ -307,25 +307,17 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
307307
self.cannot_move_out_of(span, &description)
308308
}
309309

310-
fn suggest_clone_of_captured_var_in_move_closure(
310+
pub(in crate::diagnostics) fn suggest_clone_of_captured_var_in_move_closure(
311311
&self,
312312
err: &mut Diag<'_>,
313-
upvar_hir_id: HirId,
314313
upvar_name: &str,
315314
use_spans: Option<UseSpans<'tcx>>,
316315
) {
317316
let tcx = self.infcx.tcx;
318-
let typeck_results = tcx.typeck(self.mir_def_id());
319317
let Some(use_spans) = use_spans else { return };
320318
// We only care about the case where a closure captured a binding.
321319
let UseSpans::ClosureUse { args_span, .. } = use_spans else { return };
322320
let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else { return };
323-
// Fetch the type of the expression corresponding to the closure-captured binding.
324-
let Some(captured_ty) = typeck_results.node_type_opt(upvar_hir_id) else { return };
325-
if !self.implements_clone(captured_ty) {
326-
// We only suggest cloning the captured binding if the type can actually be cloned.
327-
return;
328-
};
329321
// Find the closure that captured the binding.
330322
let mut expr_finder = FindExprBySpan::new(args_span, tcx);
331323
expr_finder.include_closures = true;
@@ -501,20 +493,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
501493
);
502494

503495
let closure_span = tcx.def_span(def_id);
504-
let mut err = self
505-
.cannot_move_out_of(span, &place_description)
496+
self.cannot_move_out_of(span, &place_description)
506497
.with_span_label(upvar_span, "captured outer variable")
507498
.with_span_label(
508499
closure_span,
509500
format!("captured by this `{closure_kind}` closure"),
510-
);
511-
self.suggest_clone_of_captured_var_in_move_closure(
512-
&mut err,
513-
upvar_hir_id,
514-
&upvar_name,
515-
use_spans,
516-
);
517-
err
501+
)
518502
}
519503
_ => {
520504
let source = self.borrowed_content_source(deref_base);
@@ -576,7 +560,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
576560
};
577561

578562
if let Some(expr) = self.find_expr(span) {
579-
self.suggest_cloning(err, place_ty, expr, self.find_expr(other_span), None);
563+
self.suggest_cloning(
564+
err,
565+
move_from.as_ref(),
566+
place_ty,
567+
expr,
568+
self.find_expr(other_span),
569+
None,
570+
);
580571
}
581572

582573
err.subdiagnostic(
@@ -613,6 +604,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
613604
if let Some(expr) = self.find_expr(use_span) {
614605
self.suggest_cloning(
615606
err,
607+
original_path.as_ref(),
616608
place_ty,
617609
expr,
618610
self.find_expr(span),
@@ -730,7 +722,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
730722
let place_desc = &format!("`{}`", self.local_names[*local].unwrap());
731723

732724
if let Some(expr) = self.find_expr(binding_span) {
733-
self.suggest_cloning(err, bind_to.ty, expr, None, None);
725+
let local_place: PlaceRef<'tcx> = (*local).into();
726+
self.suggest_cloning(err, local_place, bind_to.ty, expr, None, None);
734727
}
735728

736729
err.subdiagnostic(

tests/ui/borrowck/borrowck-move-moved-value-into-closure.stderr

+6
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ LL | call_f(move|| { *t + 1 });
1212
| ^^^^^^ -- use occurs due to use in closure
1313
| |
1414
| value used here after move
15+
|
16+
help: clone the value before moving it into the closure
17+
|
18+
LL ~ let value = t.clone();
19+
LL ~ call_f(move|| { value + 1 });
20+
|
1521

1622
error: aborting due to 1 previous error
1723

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//@ run-rustfix
2+
use std::thread;
3+
4+
fn main() {
5+
let x = "Hello world!".to_string();
6+
let value = x.clone();
7+
thread::spawn(move || {
8+
println!("{}", value);
9+
});
10+
println!("{}", x); //~ ERROR borrow of moved value
11+
}

tests/ui/moves/moves-based-on-type-capture-clause-bad.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//@ run-rustfix
12
use std::thread;
23

34
fn main() {

tests/ui/moves/moves-based-on-type-capture-clause-bad.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0382]: borrow of moved value: `x`
2-
--> $DIR/moves-based-on-type-capture-clause-bad.rs:8:20
2+
--> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
33
|
44
LL | let x = "Hello world!".to_string();
55
| - move occurs because `x` has type `String`, which does not implement the `Copy` trait
@@ -12,6 +12,12 @@ LL | println!("{}", x);
1212
| ^ value borrowed here after move
1313
|
1414
= 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)
15+
help: clone the value before moving it into the closure
16+
|
17+
LL ~ let value = x.clone();
18+
LL ~ thread::spawn(move || {
19+
LL ~ println!("{}", value);
20+
|
1521

1622
error: aborting due to 1 previous error
1723

tests/ui/no-capture-arc.stderr

+6
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ LL | assert_eq!((*arc_v)[2], 3);
1313
| ^^^^^^^^ value borrowed here after move
1414
|
1515
= note: borrow occurs due to deref coercion to `Vec<i32>`
16+
help: clone the value before moving it into the closure
17+
|
18+
LL ~ let value = arc_v.clone();
19+
LL ~ thread::spawn(move|| {
20+
LL ~ assert_eq!((*value)[3], 4);
21+
|
1622

1723
error: aborting due to 1 previous error
1824

tests/ui/no-reuse-move-arc.fixed

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

tests/ui/no-reuse-move-arc.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//@ run-rustfix
12
use std::sync::Arc;
23
use std::thread;
34

tests/ui/no-reuse-move-arc.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0382]: borrow of moved value: `arc_v`
2-
--> $DIR/no-reuse-move-arc.rs:12:16
2+
--> $DIR/no-reuse-move-arc.rs:13:16
33
|
44
LL | let arc_v = Arc::new(v);
55
| ----- 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);
1313
| ^^^^^^^^ value borrowed here after move
1414
|
1515
= note: borrow occurs due to deref coercion to `Vec<i32>`
16+
help: clone the value before moving it into the closure
17+
|
18+
LL ~ let value = arc_v.clone();
19+
LL ~ thread::spawn(move|| {
20+
LL ~ assert_eq!((*value)[3], 4);
21+
|
1622

1723
error: aborting due to 1 previous error
1824

0 commit comments

Comments
 (0)