Skip to content

Commit

Permalink
fix #104232, suggest clone for Arc/Rc
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Nov 21, 2022
1 parent 9cdfe03 commit d5d3e30
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 7 deletions.
14 changes: 8 additions & 6 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let partial_str = if is_partial_move { "partial " } else { "" };
let partially_str = if is_partial_move { "partially " } else { "" };

let mpi = self.move_data.moves[move_out_indices[0]].path;
let place = &self.move_data.move_paths[mpi].place;
let ty = place.ty(self.body, self.infcx.tcx).ty;

let mut err = self.cannot_act_on_moved_value(
span,
desired_action.as_noun(),
Expand Down Expand Up @@ -186,6 +190,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} else {
""
};
let suggest_clone = self.suggest_using_clone(ty) && !move_spans.for_closure();

if location == move_out.source {
is_loop_move = true;
Expand All @@ -202,6 +207,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
move_msg,
is_loop_move,
maybe_reinitialized_locations.is_empty(),
suggest_clone,
);

if let (UseSpans::PatUse(span), []) =
Expand Down Expand Up @@ -237,8 +243,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
}

let ty = used_place.ty(self.body, self.infcx.tcx).ty;
let needs_note = match ty.kind() {
let used_ty = used_place.ty(self.body, self.infcx.tcx).ty;
let needs_note = match used_ty.kind() {
ty::Closure(id, _) => {
let tables = self.infcx.tcx.typeck(id.expect_local());
let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(id.expect_local());
Expand All @@ -248,10 +254,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
_ => true,
};

let mpi = self.move_data.moves[move_out_indices[0]].path;
let place = &self.move_data.move_paths[mpi].place;
let ty = place.ty(self.body, self.infcx.tcx).ty;

// If we're in pattern, we do nothing in favor of the previous suggestion (#80913).
// Same for if we're in a loop, see #101119.
if is_loop_move & !in_pattern && !matches!(use_spans, UseSpans::ClosureUse { .. }) {
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

/// We only suggest clone for `std::sync::Arc` and `std::rc::Rc` types.
fn suggest_using_clone(&self, ty: Ty<'tcx>) -> bool {
if let ty::Adt(adt, _) = ty.kind() &&
(self.infcx.tcx.is_diagnostic_item(sym::Arc, adt.did()) ||
self.infcx.tcx.is_diagnostic_item(sym::Rc, adt.did())) {
return true;
}
false
}

/// End-user visible description of the `field`nth field of `base`
fn describe_field(
&self,
Expand Down Expand Up @@ -1029,6 +1039,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
move_msg: &str,
is_loop_move: bool,
maybe_reinitialized_locations_is_empty: bool,
suggest_clone: bool,
) {
if let UseSpans::FnSelfUse { var_span, fn_call_span, fn_span, kind } = move_spans {
let place_name = self
Expand Down Expand Up @@ -1166,6 +1177,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
move_span,
format!("value {}moved{} here{}", partially_str, move_msg, loop_message),
);
if suggest_clone {
err.span_suggestion_verbose(
move_span.shrink_to_hi(),
"consider cloning here",
".clone()",
Applicability::MaybeIncorrect,
);
}
}
// If the move error occurs due to a loop, don't show
// another message for the same span
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};
if let Some(use_spans) = use_spans {
self.explain_captures(
&mut err, span, span, use_spans, move_place, "", "", "", false, true,
&mut err, span, span, use_spans, move_place, "", "", "", false, true, false,
);
}
err
Expand Down
36 changes: 36 additions & 0 deletions src/test/ui/borrowck/issue-104232-suggest-clone.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use std::sync::Arc;
use std::rc::Rc;

fn foo1(_: Arc<usize>) {}
fn bar1(_: Arc<usize>) {}
fn test_arc() {
let x = Arc::new(1);
foo1(x);
foo1(x); //~ ERROR use of moved value
bar1(x); //~ ERROR use of moved value
}

fn foo2(_: Rc<usize>) {}
fn bar2(_: Rc<usize>) {}
fn test_rc() {
let x = Rc::new(1);
foo2(x);
foo2(x); //~ ERROR use of moved value
bar2(x); //~ ERROR use of moved value
}

fn test_closure() {
let x = Arc::new(1);
for _ in 0..4 {
// Ideally we should suggest `let x = x.clone();` here.
std::thread::spawn(move || { //~ ERROR use of moved value
println!("{}", x);
});
}
}

fn main() {
test_rc();
test_arc();
test_closure();
}
76 changes: 76 additions & 0 deletions src/test/ui/borrowck/issue-104232-suggest-clone.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
error[E0382]: use of moved value: `x`
--> $DIR/issue-104232-suggest-clone.rs:9:10
|
LL | let x = Arc::new(1);
| - move occurs because `x` has type `Arc<usize>`, which does not implement the `Copy` trait
LL | foo1(x);
| - value moved here
LL | foo1(x);
| ^ value used here after move
|
help: consider cloning here
|
LL | foo1(x.clone());
| ++++++++

error[E0382]: use of moved value: `x`
--> $DIR/issue-104232-suggest-clone.rs:10:10
|
LL | let x = Arc::new(1);
| - move occurs because `x` has type `Arc<usize>`, which does not implement the `Copy` trait
LL | foo1(x);
LL | foo1(x);
| - value moved here
LL | bar1(x);
| ^ value used here after move
|
help: consider cloning here
|
LL | foo1(x.clone());
| ++++++++

error[E0382]: use of moved value: `x`
--> $DIR/issue-104232-suggest-clone.rs:18:10
|
LL | let x = Rc::new(1);
| - move occurs because `x` has type `Rc<usize>`, which does not implement the `Copy` trait
LL | foo2(x);
| - value moved here
LL | foo2(x);
| ^ value used here after move
|
help: consider cloning here
|
LL | foo2(x.clone());
| ++++++++

error[E0382]: use of moved value: `x`
--> $DIR/issue-104232-suggest-clone.rs:19:10
|
LL | let x = Rc::new(1);
| - move occurs because `x` has type `Rc<usize>`, which does not implement the `Copy` trait
LL | foo2(x);
LL | foo2(x);
| - value moved here
LL | bar2(x);
| ^ value used here after move
|
help: consider cloning here
|
LL | foo2(x.clone());
| ++++++++

error[E0382]: use of moved value: `x`
--> $DIR/issue-104232-suggest-clone.rs:26:28
|
LL | let x = Arc::new(1);
| - move occurs because `x` has type `Arc<i32>`, which does not implement the `Copy` trait
...
LL | std::thread::spawn(move || {
| ^^^^^^^ value moved into closure here, in previous iteration of loop
LL | println!("{}", x);
| - use occurs due to use in closure

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0382`.
5 changes: 5 additions & 0 deletions src/test/ui/moves/use_of_moved_value_clone_suggestions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider cloning here
|
LL | (t.clone(), t)
| ++++++++

error: aborting due to previous error

Expand Down

0 comments on commit d5d3e30

Please sign in to comment.