Skip to content

Commit 98cb7c8

Browse files
estebankestebank
committed
Suggest .clone() or ref binding on E0382
1 parent b7bc90f commit 98cb7c8

File tree

86 files changed

+1092
-49
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+1092
-49
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1376,7 +1376,7 @@ pub enum ExprKind {
13761376
/// Conditionless loop (can be exited with `break`, `continue`, or `return`).
13771377
///
13781378
/// `'label: loop { block }`
1379-
Loop(P<Block>, Option<Label>),
1379+
Loop(P<Block>, Option<Label>, Span),
13801380
/// A `match` block.
13811381
Match(P<Expr>, Vec<Arm>),
13821382
/// A closure (e.g., `move |a, b, c| a + b + c`).

‎compiler/rustc_ast/src/mut_visit.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1351,9 +1351,10 @@ pub fn noop_visit_expr<T: MutVisitor>(
13511351
vis.visit_block(body);
13521352
visit_opt(label, |label| vis.visit_label(label));
13531353
}
1354-
ExprKind::Loop(body, label) => {
1354+
ExprKind::Loop(body, label, span) => {
13551355
vis.visit_block(body);
13561356
visit_opt(label, |label| vis.visit_label(label));
1357+
vis.visit_span(span);
13571358
}
13581359
ExprKind::Match(expr, arms) => {
13591360
vis.visit_expr(expr);

‎compiler/rustc_ast/src/visit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
824824
visitor.visit_expr(subexpression);
825825
visitor.visit_block(block);
826826
}
827-
ExprKind::Loop(block, opt_label) => {
827+
ExprKind::Loop(block, opt_label, _) => {
828828
walk_list!(visitor, visit_label, opt_label);
829829
visitor.visit_block(block);
830830
}

‎compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
134134
this.lower_expr_while_in_loop_scope(span, cond, body, opt_label)
135135
})
136136
}
137-
ExprKind::Loop(ref body, opt_label) => self.with_loop_scope(e.id, |this| {
137+
ExprKind::Loop(ref body, opt_label, span) => self.with_loop_scope(e.id, |this| {
138138
hir::ExprKind::Loop(
139139
this.lower_block(body, false),
140140
this.lower_label(opt_label),
141141
hir::LoopSource::Loop,
142-
DUMMY_SP,
142+
this.lower_span(span),
143143
)
144144
}),
145145
ExprKind::TryBlock(ref body) => self.lower_expr_try_block(body),

‎compiler/rustc_ast_pretty/src/pprust/state/expr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ impl<'a> State<'a> {
377377
self.space();
378378
self.print_block_with_attrs(blk, attrs);
379379
}
380-
ast::ExprKind::Loop(ref blk, opt_label) => {
380+
ast::ExprKind::Loop(ref blk, opt_label, _) => {
381381
if let Some(label) = opt_label {
382382
self.print_ident(label.ident);
383383
self.word_space(":");

‎compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 173 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,146 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
191191
is_loop_move = true;
192192
}
193193

194+
struct ExpressionFinder<'hir> {
195+
expr_span: Span,
196+
expr: Option<&'hir hir::Expr<'hir>>,
197+
pat: Option<&'hir hir::Pat<'hir>>,
198+
}
199+
impl<'hir> Visitor<'hir> for ExpressionFinder<'hir> {
200+
fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) {
201+
if e.span == self.expr_span {
202+
self.expr = Some(e);
203+
}
204+
hir::intravisit::walk_expr(self, e);
205+
}
206+
fn visit_pat(&mut self, p: &'hir hir::Pat<'hir>) {
207+
if p.span == self.expr_span {
208+
self.pat = Some(p);
209+
}
210+
if let hir::PatKind::Binding(hir::BindingAnnotation::NONE, _, i, _) = p.kind
211+
&& i.span == self.expr_span
212+
{
213+
self.pat = Some(p);
214+
}
215+
hir::intravisit::walk_pat(self, p);
216+
}
217+
}
218+
219+
let hir = self.infcx.tcx.hir();
220+
if let Some(hir::Node::Item(hir::Item {
221+
kind: hir::ItemKind::Fn(_, _, body_id),
222+
..
223+
})) = hir.find(hir.local_def_id_to_hir_id(self.mir_def_id()))
224+
&& let Some(hir::Node::Expr(expr)) = hir.find(body_id.hir_id)
225+
{
226+
let place = &self.move_data.move_paths[mpi].place;
227+
let span = place.as_local()
228+
.map(|local| self.body.local_decls[local].source_info.span);
229+
let mut finder = ExpressionFinder {
230+
expr_span: move_span,
231+
expr: None,
232+
pat: None,
233+
};
234+
finder.visit_expr(expr);
235+
if let Some(span) = span && let Some(expr) = finder.expr {
236+
for (_, expr) in hir.parent_iter(expr.hir_id) {
237+
if let hir::Node::Expr(expr) = expr {
238+
if expr.span.contains(span) {
239+
// If the let binding occurs within the same loop, then that
240+
// loop isn't relevant, like in the following, the outermost `loop`
241+
// doesn't play into `x` being moved.
242+
// ```
243+
// loop {
244+
// let x = String::new();
245+
// loop {
246+
// foo(x);
247+
// }
248+
// }
249+
// ```
250+
break;
251+
}
252+
if let hir::ExprKind::Loop(.., loop_span) = expr.kind {
253+
err.span_label(loop_span, "inside of this loop");
254+
}
255+
}
256+
}
257+
let typeck = self.infcx.tcx.typeck(self.mir_def_id());
258+
let hir_id = hir.get_parent_node(expr.hir_id);
259+
if let Some(parent) = hir.find(hir_id) {
260+
if let hir::Node::Expr(parent_expr) = parent
261+
&& let hir::ExprKind::MethodCall(_, _, args, _) = parent_expr.kind
262+
&& let Some(def_id) = typeck.type_dependent_def_id(parent_expr.hir_id)
263+
&& let Some(def_id) = def_id.as_local()
264+
&& let Some(node) = hir.find(hir.local_def_id_to_hir_id(def_id))
265+
&& let Some(fn_sig) = node.fn_sig()
266+
&& let Some(ident) = node.ident()
267+
&& let Some(pos) = args.iter()
268+
.position(|arg| arg.hir_id == expr.hir_id)
269+
&& let Some(arg) = fn_sig.decl.inputs.get(pos + 1)
270+
{
271+
let mut span: MultiSpan = arg.span.into();
272+
span.push_span_label(
273+
arg.span,
274+
"this type parameter takes ownership of the value".to_string(),
275+
);
276+
span.push_span_label(
277+
ident.span,
278+
"in this method".to_string(),
279+
);
280+
err.span_note(
281+
span,
282+
format!(
283+
"consider changing this parameter type in `{}` to borrow \
284+
instead if ownering the value isn't necessary",
285+
ident,
286+
),
287+
);
288+
}
289+
if let hir::Node::Expr(parent_expr) = parent
290+
&& let hir::ExprKind::Call(call, args) = parent_expr.kind
291+
&& let ty::FnDef(def_id, _) = typeck.node_type(call.hir_id).kind()
292+
&& let Some(def_id) = def_id.as_local()
293+
&& let Some(node) = hir.find(hir.local_def_id_to_hir_id(def_id))
294+
&& let Some(fn_sig) = node.fn_sig()
295+
&& let Some(ident) = node.ident()
296+
&& let Some(pos) = args.iter()
297+
.position(|arg| arg.hir_id == expr.hir_id)
298+
&& let Some(arg) = fn_sig.decl.inputs.get(pos)
299+
{
300+
let mut span: MultiSpan = arg.span.into();
301+
span.push_span_label(
302+
arg.span,
303+
"this type parameter takes ownership of the value".to_string(),
304+
);
305+
span.push_span_label(
306+
ident.span,
307+
"in this function".to_string(),
308+
);
309+
err.span_note(
310+
span,
311+
format!(
312+
"consider changing this parameter type in `{}` to borrow \
313+
instead if ownering the value isn't necessary",
314+
ident,
315+
),
316+
);
317+
}
318+
let place = &self.move_data.move_paths[mpi].place;
319+
let ty = place.ty(self.body, self.infcx.tcx).ty;
320+
self.suggest_cloning(&mut err, ty, move_span);
321+
}
322+
}
323+
if let Some(pat) = finder.pat {
324+
in_pattern = true;
325+
err.span_suggestion_verbose(
326+
pat.span.shrink_to_lo(),
327+
"borrow this binding in the pattern to avoid moving the value",
328+
"ref ".to_string(),
329+
Applicability::MachineApplicable,
330+
);
331+
}
332+
}
333+
194334
self.explain_captures(
195335
&mut err,
196336
span,
@@ -203,25 +343,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
203343
is_loop_move,
204344
maybe_reinitialized_locations.is_empty(),
205345
);
206-
207-
if let (UseSpans::PatUse(span), []) =
208-
(move_spans, &maybe_reinitialized_locations[..])
209-
{
210-
if maybe_reinitialized_locations.is_empty() {
211-
err.span_suggestion_verbose(
212-
span.shrink_to_lo(),
213-
&format!(
214-
"borrow this field in the pattern to avoid moving {}",
215-
self.describe_place(moved_place.as_ref())
216-
.map(|n| format!("`{}`", n))
217-
.unwrap_or_else(|| "the value".to_string())
218-
),
219-
"ref ",
220-
Applicability::MachineApplicable,
221-
);
222-
in_pattern = true;
223-
}
224-
}
225346
}
226347

227348
use_spans.var_path_only_subdiag(&mut err, desired_action);
@@ -595,6 +716,39 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
595716
true
596717
}
597718

719+
fn suggest_cloning(&self, err: &mut Diagnostic, ty: Ty<'tcx>, span: Span) {
720+
let tcx = self.infcx.tcx;
721+
722+
// Try to find predicates on *generic params* that would allow copying `ty`
723+
let infcx = tcx.infer_ctxt().build();
724+
let mut fulfill_cx = <dyn rustc_infer::traits::TraitEngine<'_>>::new(infcx.tcx);
725+
726+
let clone_did = infcx.tcx.lang_items().clone_trait().unwrap();
727+
let cause = ObligationCause::new(
728+
span,
729+
self.mir_hir_id(),
730+
rustc_infer::traits::ObligationCauseCode::MiscObligation,
731+
);
732+
fulfill_cx.register_bound(
733+
&infcx,
734+
self.param_env,
735+
// Erase any region vids from the type, which may not be resolved
736+
infcx.tcx.erase_regions(ty),
737+
clone_did,
738+
cause,
739+
);
740+
// Select all, including ambiguous predicates
741+
let errors = fulfill_cx.select_all_or_error(&infcx);
742+
if errors.is_empty() {
743+
err.span_suggestion_verbose(
744+
span.shrink_to_hi(),
745+
"consider cloning the value if the performance cost is acceptable",
746+
".clone()".to_string(),
747+
Applicability::MachineApplicable,
748+
);
749+
}
750+
}
751+
598752
fn suggest_adding_copy_bounds(&self, err: &mut Diagnostic, ty: Ty<'tcx>, span: Span) {
599753
let tcx = self.infcx.tcx;
600754
let generics = tcx.generics_of(self.mir_def_id());

‎compiler/rustc_builtin_macros/src/assert/context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
307307
| ExprKind::InlineAsm(_)
308308
| ExprKind::Let(_, _, _)
309309
| ExprKind::Lit(_)
310-
| ExprKind::Loop(_, _)
310+
| ExprKind::Loop(_, _, _)
311311
| ExprKind::MacCall(_)
312312
| ExprKind::Match(_, _)
313313
| ExprKind::Path(_, _)

‎compiler/rustc_parse/src/parser/expr.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,7 +1734,7 @@ impl<'a> Parser<'a> {
17341734
expr.kind,
17351735
ExprKind::While(_, _, None)
17361736
| ExprKind::ForLoop(_, _, _, None)
1737-
| ExprKind::Loop(_, None)
1737+
| ExprKind::Loop(_, None, _)
17381738
| ExprKind::Block(_, None)
17391739
)
17401740
{
@@ -2444,10 +2444,11 @@ impl<'a> Parser<'a> {
24442444

24452445
/// Parses `loop { ... }` (`loop` token already eaten).
24462446
fn parse_loop_expr(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'a, P<Expr>> {
2447+
let loop_span = self.prev_token.span;
24472448
let (attrs, body) = self.parse_inner_attrs_and_block()?;
24482449
Ok(self.mk_expr_with_attrs(
24492450
lo.to(self.prev_token.span),
2450-
ExprKind::Loop(body, opt_label),
2451+
ExprKind::Loop(body, opt_label, loop_span),
24512452
attrs,
24522453
))
24532454
}

‎compiler/rustc_resolve/src/late.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3841,7 +3841,9 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
38413841
}
38423842
}
38433843

3844-
ExprKind::Loop(ref block, label) => self.resolve_labeled_block(label, expr.id, &block),
3844+
ExprKind::Loop(ref block, label, _) => {
3845+
self.resolve_labeled_block(label, expr.id, &block)
3846+
}
38453847

38463848
ExprKind::While(ref cond, ref block, label) => {
38473849
self.with_resolved_label(label, expr.id, |this| {

‎src/test/ui/binding/issue-53114-borrow-checks.stderr

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ LL | match mm { (_, _y) => { } }
1717
| ^^ value used here after partial move
1818
|
1919
= note: partial move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait
20+
help: borrow this binding in the pattern to avoid moving the value
21+
|
22+
LL | match mm { (ref _x, _) => { } }
23+
| +++
2024

2125
error[E0382]: use of partially moved value: `mm`
2226
--> $DIR/issue-53114-borrow-checks.rs:29:11
@@ -28,6 +32,10 @@ LL | match mm { (_, _) => { } }
2832
| ^^ value used here after partial move
2933
|
3034
= note: partial move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait
35+
help: borrow this binding in the pattern to avoid moving the value
36+
|
37+
LL | match mm { (_, ref _y) => { } }
38+
| +++
3139

3240
error[E0382]: use of moved value: `m`
3341
--> $DIR/issue-53114-borrow-checks.rs:36:16
@@ -48,6 +56,10 @@ LL | if let (_, _y) = mm { }
4856
| ^^ value used here after partial move
4957
|
5058
= note: partial move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait
59+
help: borrow this binding in the pattern to avoid moving the value
60+
|
61+
LL | if let (ref _x, _) = mm { }
62+
| +++
5163

5264
error[E0382]: use of partially moved value: `mm`
5365
--> $DIR/issue-53114-borrow-checks.rs:43:21
@@ -59,6 +71,10 @@ LL | if let (_, _) = mm { }
5971
| ^^ value used here after partial move
6072
|
6173
= note: partial move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait
74+
help: borrow this binding in the pattern to avoid moving the value
75+
|
76+
LL | if let (_, ref _y) = mm { }
77+
| +++
6278

6379
error: aborting due to 6 previous errors
6480

0 commit comments

Comments
 (0)