Auto merge of #44882 - mikhail-m1:master, r=pnkfelix

add notes to report_conflicting_borrow MIR borrowck

part of #44596

4 files changed, 150 insertions, 136 deletions

diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs
index 834bde660a8..fea662e21fa 100644
--- a/src/librustc_borrowck/borrowck/check_loans.rs
+++ b/src/librustc_borrowck/borrowck/check_loans.rs
@@ -470,102 +470,29 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
                 old_loan.kill_scope.span(self.tcx(), &self.bccx.region_scope_tree).end_point();
 
             let mut err = match (new_loan.kind, old_loan.kind) {
-                (ty::MutBorrow, ty::MutBorrow) => {
-                    let mut err = self.bccx.cannot_mutably_borrow_multiply(
-                        new_loan.span, &nl, &new_loan_msg, Origin::Ast);
-
-                    if new_loan.span == old_loan.span {
-                        // Both borrows are happening in the same place
-                        // Meaning the borrow is occurring in a loop
-                        err.span_label(
-                                new_loan.span,
-                                format!("mutable borrow starts here in previous \
-                                        iteration of loop{}", new_loan_msg));
-                        err.span_label(
-                                previous_end_span,
-                                "mutable borrow ends here");
-                        err
-                    } else {
-                       err.span_label(
-                                old_loan.span,
-                                format!("first mutable borrow occurs here{}", old_loan_msg));
-                        err.span_label(
-                                new_loan.span,
-                                format!("second mutable borrow occurs here{}", new_loan_msg));
-                        err.span_label(
-                                previous_end_span,
-                                "first borrow ends here");
-                        err
-                    }
-                }
-
-                (ty::UniqueImmBorrow, ty::UniqueImmBorrow) => {
-                    let mut err = self.bccx.cannot_uniquely_borrow_by_two_closures(
-                        new_loan.span, &nl, Origin::Ast);
-                    err.span_label(
-                            old_loan.span,
-                            "first closure is constructed here");
-                    err.span_label(
-                            new_loan.span,
-                            "second closure is constructed here");
-                    err.span_label(
-                            previous_end_span,
-                            "borrow from first closure ends here");
-                    err
-                }
-
-                (ty::UniqueImmBorrow, _) => {
-                    let mut err = self.bccx.cannot_uniquely_borrow_by_one_closure(
-                        new_loan.span, &nl, &ol_pronoun, &old_loan_msg, Origin::Ast);
-                    err.span_label(
-                            new_loan.span,
-                            format!("closure construction occurs here{}", new_loan_msg));
-                    err.span_label(
-                            old_loan.span,
-                            format!("borrow occurs here{}", old_loan_msg));
-                    err.span_label(
-                            previous_end_span,
-                            "borrow ends here");
-                    err
-                }
-
+                (ty::MutBorrow, ty::MutBorrow) =>
+                    self.bccx.cannot_mutably_borrow_multiply(
+                        new_loan.span, &nl, &new_loan_msg, old_loan.span, &old_loan_msg,
+                        previous_end_span, Origin::Ast),
+                (ty::UniqueImmBorrow, ty::UniqueImmBorrow) =>
+                    self.bccx.cannot_uniquely_borrow_by_two_closures(
+                        new_loan.span, &nl, old_loan.span, previous_end_span, Origin::Ast),
+                (ty::UniqueImmBorrow, _) =>
+                    self.bccx.cannot_uniquely_borrow_by_one_closure(
+                        new_loan.span, &nl, &new_loan_msg,
+                        old_loan.span, &ol_pronoun, &old_loan_msg, previous_end_span, Origin::Ast),
                 (_, ty::UniqueImmBorrow) => {
                     let new_loan_str = &new_loan.kind.to_user_str();
-                    let mut err = self.bccx.cannot_reborrow_already_uniquely_borrowed(
-                        new_loan.span, &nl, &new_loan_msg, new_loan_str, Origin::Ast);
-                    err.span_label(
-                            new_loan.span,
-                            format!("borrow occurs here{}", new_loan_msg));
-                    err.span_label(
-                            old_loan.span,
-                            format!("closure construction occurs here{}", old_loan_msg));
-                    err.span_label(
-                            previous_end_span,
-                            "borrow from closure ends here");
-                    err
+                    self.bccx.cannot_reborrow_already_uniquely_borrowed(
+                        new_loan.span, &nl, &new_loan_msg, new_loan_str,
+                        old_loan.span, &old_loan_msg, previous_end_span, Origin::Ast)
                 }
-
-                (..) => {
-                    let mut err = self.bccx.cannot_reborrow_already_borrowed(
+                (..) =>
+                    self.bccx.cannot_reborrow_already_borrowed(
                         new_loan.span,
                         &nl, &new_loan_msg, &new_loan.kind.to_user_str(),
-                        &ol_pronoun, &old_loan.kind.to_user_str(), &old_loan_msg, Origin::Ast);
-                    err.span_label(
-                            new_loan.span,
-                            format!("{} borrow occurs here{}",
-                                     new_loan.kind.to_user_str(),
-                                     new_loan_msg));
-                    err.span_label(
-                            old_loan.span,
-                            format!("{} borrow occurs here{}",
-                                     old_loan.kind.to_user_str(),
-                                     old_loan_msg));
-                    err.span_label(
-                            previous_end_span,
-                            format!("{} borrow ends here",
-                                     old_loan.kind.to_user_str()));
-                    err
-                }
+                        old_loan.span, &ol_pronoun, &old_loan.kind.to_user_str(), &old_loan_msg,
+                        previous_end_span, Origin::Ast)
             };
 
             match new_loan.cause {
diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs
index 0ad22f91855..14b6e319132 100644
--- a/src/librustc_mir/borrow_check.rs
+++ b/src/librustc_mir/borrow_check.rs
@@ -396,28 +396,34 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
                             ReadKind::Copy =>
                                 this.report_use_while_mutably_borrowed(
                                     context, lvalue_span, borrow),
-                            ReadKind::Borrow(bk) =>
+                            ReadKind::Borrow(bk) => {
+                                let end_issued_loan_span =
+                                    flow_state.borrows.base_results.operator().region_span(
+                                        &borrow.region).end_point();
                                 this.report_conflicting_borrow(
-                                    context, lvalue_span,
-                                    common_prefix,
-                                    (lvalue_span.0, bk), (&borrow.lvalue, borrow.kind)),
+                                    context, common_prefix, lvalue_span, bk,
+                                    &borrow, end_issued_loan_span)
+                            }
                         }
                         Control::Break
                     }
                     (Write(kind), _) => {
                         match kind {
-                            WriteKind::MutableBorrow(bk) =>
+                            WriteKind::MutableBorrow(bk) => {
+                                let end_issued_loan_span =
+                                    flow_state.borrows.base_results.operator().region_span(
+                                        &borrow.region).end_point();
                                 this.report_conflicting_borrow(
-                                    context, lvalue_span,
-                                    common_prefix,
-                                    (lvalue_span.0, bk), (&borrow.lvalue, borrow.kind)),
+                                    context, common_prefix, lvalue_span, bk,
+                                    &borrow, end_issued_loan_span)
+                            }
                             WriteKind::StorageDead |
                             WriteKind::Mutate =>
                                 this.report_illegal_mutation_of_borrowed(
                                     context, lvalue_span, borrow),
                             WriteKind::Move =>
                                 this.report_move_out_while_borrowed(
-                                    context, lvalue_span, borrow),
+                                    context, lvalue_span, &borrow),
                         }
                         Control::Break
                     }
@@ -966,48 +972,49 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
 
     fn report_conflicting_borrow(&mut self,
                                  _context: Context,
-                                 (lvalue, span): (&Lvalue, Span),
                                  common_prefix: &Lvalue,
-                                 loan1: (&Lvalue, BorrowKind),
-                                 loan2: (&Lvalue, BorrowKind)) {
+                                 (lvalue, span): (&Lvalue, Span),
+                                 gen_borrow_kind: BorrowKind,
+                                 issued_borrow: &BorrowData,
+                                 end_issued_loan_span: Span) {
         use self::prefixes::IsPrefixOf;
 
-        let (loan1_lvalue, loan1_kind) = loan1;
-        let (loan2_lvalue, loan2_kind) = loan2;
+        assert!(common_prefix.is_prefix_of(lvalue));
+        assert!(common_prefix.is_prefix_of(&issued_borrow.lvalue));
 
-        assert!(common_prefix.is_prefix_of(loan1_lvalue));
-        assert!(common_prefix.is_prefix_of(loan2_lvalue));
+        let issued_span = self.retrieve_borrow_span(issued_borrow);
 
         // FIXME: supply non-"" `opt_via` when appropriate
-        let mut err = match (loan1_kind, "immutable", "mutable",
-                             loan2_kind, "immutable", "mutable") {
+        let mut err = match (gen_borrow_kind, "immutable", "mutable",
+                             issued_borrow.kind, "immutable", "mutable") {
             (BorrowKind::Shared, lft, _, BorrowKind::Mut, _, rgt) |
             (BorrowKind::Mut, _, lft, BorrowKind::Shared, rgt, _) =>
                 self.tcx.cannot_reborrow_already_borrowed(
-                    span, &self.describe_lvalue(lvalue),
-                    "", lft, "it", rgt, "", Origin::Mir),
+                    span, &self.describe_lvalue(lvalue), "", lft, issued_span,
+                    "it", rgt, "", end_issued_loan_span, Origin::Mir),
 
             (BorrowKind::Mut, _, _, BorrowKind::Mut, _, _) =>
                 self.tcx.cannot_mutably_borrow_multiply(
-                    span, &self.describe_lvalue(lvalue), "", Origin::Mir),
+                    span, &self.describe_lvalue(lvalue), "", issued_span,
+                    "", end_issued_loan_span, Origin::Mir),
 
             (BorrowKind::Unique, _, _, BorrowKind::Unique, _, _) =>
                 self.tcx.cannot_uniquely_borrow_by_two_closures(
-                    span, &self.describe_lvalue(lvalue), Origin::Mir),
+                    span, &self.describe_lvalue(lvalue), issued_span,
+                    end_issued_loan_span, Origin::Mir),
 
             (BorrowKind::Unique, _, _, _, _, _) =>
                 self.tcx.cannot_uniquely_borrow_by_one_closure(
-                    span, &self.describe_lvalue(lvalue), "it", "", Origin::Mir),
+                    span, &self.describe_lvalue(lvalue), "",
+                    issued_span, "it", "", end_issued_loan_span, Origin::Mir),
 
             (_, _, _, BorrowKind::Unique, _, _) =>
                 self.tcx.cannot_reborrow_already_uniquely_borrowed(
-                    span, &self.describe_lvalue(lvalue), "it", "", Origin::Mir),
+                    span, &self.describe_lvalue(lvalue), "it", "",
+                    issued_span, "", end_issued_loan_span, Origin::Mir),
 
             (BorrowKind::Shared, _, _, BorrowKind::Shared, _, _) =>
                 unreachable!(),
-
-            // FIXME: add span labels for first and second mutable borrows, as well as
-            // end point for first.
         };
         err.emit();
     }
diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs
index 3f815ec83e3..22ba7479ead 100644
--- a/src/librustc_mir/dataflow/impls/borrows.rs
+++ b/src/librustc_mir/dataflow/impls/borrows.rs
@@ -11,6 +11,7 @@
 use rustc::mir::{self, Location, Mir};
 use rustc::mir::visit::Visitor;
 use rustc::ty::{Region, TyCtxt};
+use rustc::ty::RegionKind;
 use rustc::ty::RegionKind::ReScope;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
 
@@ -21,6 +22,8 @@ use rustc_data_structures::indexed_vec::{IndexVec};
 use dataflow::{BitDenotation, BlockSets, DataflowOperator};
 pub use dataflow::indexes::BorrowIndex;
 
+use syntax_pos::Span;
+
 use std::fmt;
 
 // `Borrows` maps each dataflow bit to an `Rvalue::Ref`, which can be
@@ -32,6 +35,7 @@ pub struct Borrows<'a, 'tcx: 'a> {
     borrows: IndexVec<BorrowIndex, BorrowData<'tcx>>,
     location_map: FxHashMap<Location, BorrowIndex>,
     region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
+    region_span_map: FxHashMap<RegionKind, Span>,
 }
 
 // temporarily allow some dead fields: `kind` and `region` will be
@@ -63,18 +67,21 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
     pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>, mir: &'a Mir<'tcx>) -> Self {
         let mut visitor = GatherBorrows { idx_vec: IndexVec::new(),
                                           location_map: FxHashMap(),
-                                          region_map: FxHashMap(), };
+                                          region_map: FxHashMap(),
+                                          region_span_map: FxHashMap()};
         visitor.visit_mir(mir);
         return Borrows { tcx: tcx,
                          mir: mir,
                          borrows: visitor.idx_vec,
                          location_map: visitor.location_map,
-                         region_map: visitor.region_map, };
+                         region_map: visitor.region_map,
+                         region_span_map: visitor.region_span_map};
 
         struct GatherBorrows<'tcx> {
             idx_vec: IndexVec<BorrowIndex, BorrowData<'tcx>>,
             location_map: FxHashMap<Location, BorrowIndex>,
             region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
+            region_span_map: FxHashMap<RegionKind, Span>,
         }
         impl<'tcx> Visitor<'tcx> for GatherBorrows<'tcx> {
             fn visit_rvalue(&mut self,
@@ -90,6 +97,16 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
                     borrows.insert(idx);
                 }
             }
+
+            fn visit_statement(&mut self,
+                               block: mir::BasicBlock,
+                               statement: &mir::Statement<'tcx>,
+                               location: Location) {
+                if let mir::StatementKind::EndRegion(region_scope) = statement.kind {
+                    self.region_span_map.insert(ReScope(region_scope), statement.source_info.span);
+                }
+                self.super_statement(block, statement, location);
+            }
         }
     }
 
@@ -98,6 +115,12 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
     pub fn location(&self, idx: BorrowIndex) -> &Location {
         &self.borrows[idx].location
     }
+
+    pub fn region_span(&self, region: &Region) -> Span {
+        let opt_span = self.region_span_map.get(region);
+        assert!(opt_span.is_some(), "end region not found for {:?}", region);
+        *opt_span.unwrap()
+    }
 }
 
 impl<'a, 'tcx> BitDenotation for Borrows<'a, 'tcx> {
diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs
index eddcb89c344..81e3cd7f61f 100644
--- a/src/librustc_mir/util/borrowck_errors.rs
+++ b/src/librustc_mir/util/borrowck_errors.rs
@@ -88,50 +88,101 @@ pub trait BorrowckErrors {
     }
 
     fn cannot_mutably_borrow_multiply(&self,
-                                      span: Span,
+                                      new_loan_span: Span,
                                       desc: &str,
                                       opt_via: &str,
+                                      old_loan_span: Span,
+                                      old_opt_via: &str,
+                                      old_load_end_span:Span,
                                       o: Origin)
                                       -> DiagnosticBuilder
     {
-        struct_span_err!(self, span, E0499,
+        let mut err = struct_span_err!(self, new_loan_span, E0499,
                          "cannot borrow `{}`{} as mutable more than once at a time{OGN}",
-                         desc, opt_via, OGN=o)
+                         desc, opt_via, OGN=o);
+        if old_loan_span == new_loan_span {
+            // Both borrows are happening in the same place
+            // Meaning the borrow is occurring in a loop
+            err.span_label(new_loan_span,
+                           format!("mutable borrow starts here in previous \
+                                    iteration of loop{}", opt_via));
+            err.span_label(old_load_end_span, "mutable borrow ends here");
+        } else {
+            err.span_label(old_loan_span,
+                           format!("first mutable borrow occurs here{}", old_opt_via));
+            err.span_label(new_loan_span,
+                           format!("second mutable borrow occurs here{}", opt_via));
+            err.span_label(old_load_end_span, "first borrow ends here");
+        }
+        err
     }
 
-    fn cannot_uniquely_borrow_by_two_closures(&self, span: Span, desc: &str, o: Origin)
+    fn cannot_uniquely_borrow_by_two_closures(&self,
+                                              new_loan_span: Span,
+                                              desc: &str,
+                                              old_loan_span: Span,
+                                              old_load_end_span: Span,
+                                              o: Origin)
                                               -> DiagnosticBuilder
     {
-        struct_span_err!(self, span, E0524,
+        let mut err = struct_span_err!(self, new_loan_span, E0524,
                          "two closures require unique access to `{}` at the same time{OGN}",
-                         desc, OGN=o)
+                         desc, OGN=o);
+        err.span_label(
+            old_loan_span,
+            "first closure is constructed here");
+        err.span_label(
+            new_loan_span,
+            "second closure is constructed here");
+        err.span_label(
+            old_load_end_span,
+            "borrow from first closure ends here");
+        err
     }
 
     fn cannot_uniquely_borrow_by_one_closure(&self,
-                                             span: Span,
+                                             new_loan_span: Span,
                                              desc_new: &str,
+                                             opt_via: &str,
+                                             old_loan_span: Span,
                                              noun_old: &str,
-                                             msg_old: &str,
+                                             old_opt_via: &str,
+                                             previous_end_span: Span,
                                              o: Origin)
                                              -> DiagnosticBuilder
     {
-        struct_span_err!(self, span, E0500,
+        let mut err = struct_span_err!(self, new_loan_span, E0500,
                          "closure requires unique access to `{}` but {} is already borrowed{}{OGN}",
-                         desc_new, noun_old, msg_old, OGN=o)
+                         desc_new, noun_old, old_opt_via, OGN=o);
+        err.span_label(new_loan_span,
+                       format!("closure construction occurs here{}", opt_via));
+        err.span_label(old_loan_span,
+                       format!("borrow occurs here{}", old_opt_via));
+        err.span_label(previous_end_span, "borrow ends here");
+        err
     }
 
     fn cannot_reborrow_already_uniquely_borrowed(&self,
-                                                 span: Span,
+                                                 new_loan_span: Span,
                                                  desc_new: &str,
-                                                 msg_new: &str,
+                                                 opt_via: &str,
                                                  kind_new: &str,
+                                                 old_loan_span: Span,
+                                                 old_opt_via: &str,
+                                                 previous_end_span: Span,
                                                  o: Origin)
                                                  -> DiagnosticBuilder
     {
-        struct_span_err!(self, span, E0501,
+        let mut err = struct_span_err!(self, new_loan_span, E0501,
                          "cannot borrow `{}`{} as {} because previous closure \
                           requires unique access{OGN}",
-                         desc_new, msg_new, kind_new, OGN=o)
+                         desc_new, opt_via, kind_new, OGN=o);
+        err.span_label(new_loan_span,
+                       format!("borrow occurs here{}", opt_via));
+        err.span_label(old_loan_span,
+                       format!("closure construction occurs here{}", old_opt_via));
+        err.span_label(previous_end_span, "borrow from closure ends here");
+        err
     }
 
     fn cannot_reborrow_already_borrowed(&self,
@@ -139,15 +190,21 @@ pub trait BorrowckErrors {
                                         desc_new: &str,
                                         msg_new: &str,
                                         kind_new: &str,
+                                        old_span: Span,
                                         noun_old: &str,
                                         kind_old: &str,
                                         msg_old: &str,
+                                        old_load_end_span: Span,
                                         o: Origin)
                                         -> DiagnosticBuilder
     {
-        struct_span_err!(self, span, E0502,
+        let mut err = struct_span_err!(self, span, E0502,
                          "cannot borrow `{}`{} as {} because {} is also borrowed as {}{}{OGN}",
-                         desc_new, msg_new, kind_new, noun_old, kind_old, msg_old, OGN=o)
+                         desc_new, msg_new, kind_new, noun_old, kind_old, msg_old, OGN=o);
+        err.span_label(span, format!("{} borrow occurs here{}", kind_new, msg_new));
+        err.span_label(old_span, format!("{} borrow occurs here{}", kind_old, msg_old));
+        err.span_label(old_load_end_span, format!("{} borrow ends here", kind_old));
+        err
     }
 
     fn cannot_assign_to_borrowed(&self, span: Span, borrow_span: Span, desc: &str, o: Origin)