Skip to content

Commit

Permalink
Auto merge of #59195 - estebank:for-loop-move, r=petrochenkov
Browse files Browse the repository at this point in the history
When moving out of a for loop head, suggest borrowing it

When encountering code like the following, suggest borrowing the for loop
head to avoid moving it into the for loop pattern:

```
fn main() {
    let a = vec![1, 2, 3];
    for i in &a {
        for j in a {
            println!("{} * {} = {}", i, j, i * j);
        }
    }
}
```

Fix #25534.
  • Loading branch information
bors committed Mar 25, 2019
2 parents 3752b3d + 66202c1 commit 4691471
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/librustc/hir/lowering.rs
Expand Up @@ -4326,13 +4326,14 @@ impl<'a> LoweringContext<'a> {
// }

// expand <head>
let head = self.lower_expr(head);
let mut head = self.lower_expr(head);
let head_sp = head.span;
let desugared_span = self.mark_span_with_reason(
CompilerDesugaringKind::ForLoop,
head_sp,
None,
);
head.span = desugared_span;

let iter = self.str_to_ident("iter");

Expand Down
14 changes: 14 additions & 0 deletions src/librustc_borrowck/borrowck/mod.rs
Expand Up @@ -34,6 +34,7 @@ use std::fmt;
use std::rc::Rc;
use rustc_data_structures::sync::Lrc;
use std::hash::{Hash, Hasher};
use syntax::source_map::CompilerDesugaringKind;
use syntax_pos::{MultiSpan, Span};
use errors::{Applicability, DiagnosticBuilder, DiagnosticId};
use log::debug;
Expand Down Expand Up @@ -743,6 +744,19 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
},
moved_lp.ty));
}
if let (Some(CompilerDesugaringKind::ForLoop), Ok(snippet)) = (
move_span.compiler_desugaring_kind(),
self.tcx.sess.source_map().span_to_snippet(move_span),
) {
if !snippet.starts_with("&") {
err.span_suggestion(
move_span,
"consider borrowing this to avoid moving it into the for loop",
format!("&{}", snippet),
Applicability::MaybeIncorrect,
);
}
}

// Note: we used to suggest adding a `ref binding` or calling
// `clone` but those suggestions have been removed because
Expand Down
24 changes: 24 additions & 0 deletions src/test/ui/suggestions/borrow-for-loop-head.nll.stderr
@@ -0,0 +1,24 @@
error[E0505]: cannot move out of `a` because it is borrowed
--> $DIR/borrow-for-loop-head.rs:4:18
|
LL | for i in &a {
| --
| |
| borrow of `a` occurs here
| borrow later used here
LL | for j in a {
| ^ move out of `a` occurs here

error[E0382]: use of moved value: `a`
--> $DIR/borrow-for-loop-head.rs:4:18
|
LL | let a = vec![1, 2, 3];
| - move occurs because `a` has type `std::vec::Vec<i32>`, which does not implement the `Copy` trait
LL | for i in &a {
LL | for j in a {
| ^ value moved here, in previous iteration of loop

error: aborting due to 2 previous errors

Some errors occurred: E0382, E0505.
For more information about an error, try `rustc --explain E0382`.
10 changes: 10 additions & 0 deletions src/test/ui/suggestions/borrow-for-loop-head.rs
@@ -0,0 +1,10 @@
fn main() {
let a = vec![1, 2, 3];
for i in &a {
for j in a {
//~^ ERROR cannot move out of `a` because it is borrowed
//~| ERROR use of moved value: `a`
println!("{} * {} = {}", i, j, i * j);
}
}
}
24 changes: 24 additions & 0 deletions src/test/ui/suggestions/borrow-for-loop-head.stderr
@@ -0,0 +1,24 @@
error[E0505]: cannot move out of `a` because it is borrowed
--> $DIR/borrow-for-loop-head.rs:4:18
|
LL | for i in &a {
| - borrow of `a` occurs here
LL | for j in a {
| ^ move out of `a` occurs here

error[E0382]: use of moved value: `a`
--> $DIR/borrow-for-loop-head.rs:4:18
|
LL | for j in a {
| ^ value moved here in previous iteration of loop
|
= note: move occurs because `a` has type `std::vec::Vec<i32>`, which does not implement the `Copy` trait
help: consider borrowing this to avoid moving it into the for loop
|
LL | for j in &a {
| ^^

error: aborting due to 2 previous errors

Some errors occurred: E0382, E0505.
For more information about an error, try `rustc --explain E0382`.

0 comments on commit 4691471

Please sign in to comment.