Rollup merge of rust-lang#59429 - estebank:for-loop-move-nll, r=petrochenkov

Centril · web-flow · commit f59ee136bcf7 · 2019-03-27T10:14:04.000+01:00
When moving out of a for loop head, suggest borrowing it in nll mode Follow up to rust-lang#59195 for NLL.
diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs
@@ -19,6 +19,7 @@ use rustc_data_structures::indexed_vec::Idx;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::{Applicability, DiagnosticBuilder};
 use syntax_pos::Span;
+use syntax::source_map::CompilerDesugaringKind;
 
 use super::borrow_set::BorrowData;
 use super::{Context, MirBorrowckCtxt};
@@ -154,6 +155,18 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                         span,
                         format!("value moved{} here, in previous iteration of loop", move_msg),
                     );
+                    if Some(CompilerDesugaringKind::ForLoop) == span.compiler_desugaring_kind() {
+                        if let Ok(snippet) = self.infcx.tcx.sess.source_map()
+                            .span_to_snippet(span)
+                        {
+                            err.span_suggestion(
+                                move_span,
+                                "consider borrowing this to avoid moving it into the for loop",
+                                format!("&{}", snippet),
+                                Applicability::MaybeIncorrect,
+                            );
+                        }
+                    }
                     is_loop_move = true;
                 } else if move_site.traversed_back_edge {
                     err.span_label(
@@ -291,8 +304,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             format!("move occurs due to use{}", move_spans.describe())
         );
 
-        self.explain_why_borrow_contains_point(context, borrow, None)
-            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
+        self.explain_why_borrow_contains_point(
+            context,
+            borrow,
+            None,
+        ).add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", Some(borrow_span));
         err.buffer(&mut self.errors_buffer);
     }
 
@@ -329,7 +345,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         });
 
         self.explain_why_borrow_contains_point(context, borrow, None)
-            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
+            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);
         err.buffer(&mut self.errors_buffer);
     }
 
@@ -542,8 +558,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             ));
         }
 
-        explanation
-            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc);
+        explanation.add_explanation_to_diagnostic(
+            self.infcx.tcx,
+            self.mir,
+            &mut err,
+            first_borrow_desc,
+            None,
+        );
 
         err.buffer(&mut self.errors_buffer);
     }
@@ -866,7 +887,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
             if let BorrowExplanation::MustBeValidFor { .. } = explanation {
             } else {
-                explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
+                explanation.add_explanation_to_diagnostic(
+                    self.infcx.tcx,
+                    self.mir,
+                    &mut err,
+                    "",
+                    None,
+                );
             }
         } else {
             err.span_label(borrow_span, "borrowed value does not live long enough");
@@ -886,7 +913,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 format!("value captured here{}", within),
             );
 
-            explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
+            explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);
         }
 
         err
@@ -946,7 +973,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             _ => {}
         }
 
-        explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
+        explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);
 
         err.buffer(&mut self.errors_buffer);
     }
@@ -1027,7 +1054,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             }
             _ => {}
         }
-        explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
+        explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);
 
         let within = if borrow_spans.for_generator() {
             " by generator"
@@ -1367,7 +1394,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         );
 
         self.explain_why_borrow_contains_point(context, loan, None)
-            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
+            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);
 
         err.buffer(&mut self.errors_buffer);
     }
diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
@@ -56,17 +56,23 @@ impl BorrowExplanation {
         mir: &Mir<'tcx>,
         err: &mut DiagnosticBuilder<'_>,
         borrow_desc: &str,
+        borrow_span: Option<Span>,
     ) {
         match *self {
             BorrowExplanation::UsedLater(later_use_kind, var_or_use_span) => {
                 let message = match later_use_kind {
-                    LaterUseKind::TraitCapture => "borrow later captured here by trait object",
-                    LaterUseKind::ClosureCapture => "borrow later captured here by closure",
-                    LaterUseKind::Call => "borrow later used by call",
-                    LaterUseKind::FakeLetRead => "borrow later stored here",
-                    LaterUseKind::Other => "borrow later used here",
+                    LaterUseKind::TraitCapture => "captured here by trait object",
+                    LaterUseKind::ClosureCapture => "captured here by closure",
+                    LaterUseKind::Call => "used by call",
+                    LaterUseKind::FakeLetRead => "stored here",
+                    LaterUseKind::Other => "used here",
                 };
-                err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message));
+                if !borrow_span.map(|sp| sp.overlaps(var_or_use_span)).unwrap_or(false) {
+                    err.span_label(
+                        var_or_use_span,
+                        format!("{}borrow later {}", borrow_desc, message),
+                    );
+                }
             }
             BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => {
                 let message = match later_use_kind {
diff --git a/src/test/ui/augmented-assignments.nll.stderr b/src/test/ui/augmented-assignments.nll.stderr
@@ -1,18 +1,11 @@
 error[E0505]: cannot move out of `x` because it is borrowed
   --> $DIR/augmented-assignments.rs:16:5
    |
-LL |       x
-   |       -
-   |       |
-   |  _____borrow of `x` occurs here
-   | |
-LL | |
-LL | |     +=
-LL | |     x;
-   | |     ^
-   | |     |
-   | |_____move out of `x` occurs here
-   |       borrow later used here
+LL |     x
+   |     - borrow of `x` occurs here
+...
+LL |     x;
+   |     ^ move out of `x` occurs here
 
 error[E0596]: cannot borrow `y` as mutable, as it is not declared as mutable
   --> $DIR/augmented-assignments.rs:21:5
diff --git a/src/test/ui/regions/region-bound-on-closure-outlives-call.nll.stderr b/src/test/ui/regions/region-bound-on-closure-outlives-call.nll.stderr
@@ -14,11 +14,10 @@ error[E0505]: cannot move out of `f` because it is borrowed
   --> $DIR/region-bound-on-closure-outlives-call.rs:3:25
    |
 LL |     (|x| f(x))(call_rec(f))
-   |     ----------          ^ move out of `f` occurs here
-   |     ||   |
-   |     ||   borrow occurs due to use in closure
-   |     |borrow of `f` occurs here
-   |     borrow later used by call
+   |      --- -              ^ move out of `f` occurs here
+   |      |   |
+   |      |   borrow occurs due to use in closure
+   |      borrow of `f` occurs here
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr b/src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr
@@ -42,7 +42,6 @@ LL |     f(Box::new(|a| {
    |     -          ^^^ move out of `f` occurs here
    |     |
    |     borrow of `f` occurs here
-   |     borrow later used by call
 LL |         foo(f);
    |             - move occurs due to use in closure
 
diff --git a/src/test/ui/suggestions/borrow-for-loop-head.nll.stderr b/src/test/ui/suggestions/borrow-for-loop-head.nll.stderr
@@ -2,10 +2,7 @@ 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
+   |              -- borrow of `a` occurs here
 LL |         for j in a {
    |                  ^ move out of `a` occurs here
 
@@ -17,6 +14,10 @@ LL |     let a = vec![1, 2, 3];
 LL |     for i in &a {
 LL |         for j in a {
    |                  ^ value moved here, in previous iteration of loop
+help: consider borrowing this to avoid moving it into the for loop
+   |
+LL |         for j in &a {
+   |                  ^^
 
 error: aborting due to 2 previous errors
 
