From 4bad56e54c1a870377b04b0b7f17e3051f246248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 25 Mar 2019 19:11:41 -0700 Subject: [PATCH] When moving out of a for loop head, suggest borrowing it in nll mode --- .../borrow_check/error_reporting.rs | 47 +++++++++++++++---- .../borrow_check/nll/explain_borrow/mod.rs | 18 ++++--- src/test/ui/augmented-assignments.nll.stderr | 17 ++----- ...-bound-on-closure-outlives-call.nll.stderr | 9 ++-- ...owck-call-is-borrow-issue-12224.nll.stderr | 1 - .../borrow-for-loop-head.nll.stderr | 9 ++-- 6 files changed, 63 insertions(+), 38 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 14289381aef45..88ff231202085 100644 --- 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 index dfa5af444d37e..67b77605f3c92 100644 --- 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, ) { 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 index 08f06e90162b4..e205e2a87810f 100644 --- 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 index bce310bafaa92..d455902ee8c07 100644 --- 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 index 80dc3ef2f80f2..1a5ab7a7d56a0 100644 --- 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 index 6450f7f52a47b..96dbdec7074a9 100644 --- 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