Refactor explain_borrow to return explanation.

Previously, explain_borrow would emit an error with the explanation of
the a borrow. Now, it returns a enum with what the explanation for the
borrow is and any relevant spans or information such that the calling
code can choose to emit the same note/suggestion as before by calling
the emit method on the new enum.

src/librustc_mir/borrow_check/error_reporting.rs

@@ -10,11 +10,14 @@
 
 use borrow_check::{WriteKind, StorageDeadOrDrop};
 use borrow_check::prefixes::IsPrefixOf;
+use borrow_check::nll::explain_borrow::BorrowExplanation;
 use rustc::middle::region::ScopeTree;
-use rustc::mir::VarBindingForm;
-use rustc::mir::{BindingForm, BorrowKind, ClearCrossCrate, Field, Local};
-use rustc::mir::{FakeReadCause, LocalDecl, LocalKind, Location, Operand, Place};
-use rustc::mir::{ProjectionElem, Rvalue, Statement, StatementKind};
+use rustc::mir::{
+    BindingForm, BorrowKind, ClearCrossCrate, Field, FakeReadCause, Local,
+    LocalDecl, LocalKind, Location, Operand, Place,
+    ProjectionElem, Rvalue, Statement, StatementKind,
+    VarBindingForm,
+};
 use rustc::ty;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::sync::Lrc;
@@ -25,7 +28,6 @@
 use super::{Context, MirBorrowckCtxt};
 use super::{InitializationRequiringAction, PrefixSet};
 
-use borrow_check::nll::explain_borrow::BorrowContainsPointReason;
 use dataflow::drop_flag_effects;
 use dataflow::move_paths::indexes::MoveOutIndex;
 use dataflow::move_paths::MovePathIndex;
@@ -226,7 +228,7 @@ pub(super) fn report_move_out_while_borrowed(
 
         move_spans.var_span_label(&mut err, "move occurs due to use in closure");
 
-        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, None).emit(self.tcx, &mut err);
         err.buffer(&mut self.errors_buffer);
     }
 
@@ -263,7 +265,7 @@ pub(super) fn report_use_while_mutably_borrowed(
             format!("borrow occurs due to use of `{}` in closure", desc_place)
         });
 
-        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, None).emit(self.tcx, &mut err);
         err.buffer(&mut self.errors_buffer);
     }
 
@@ -390,7 +392,8 @@ pub(super) fn report_conflicting_borrow(
             );
         }
 
-        self.explain_why_borrow_contains_point(context, issued_borrow, None, &mut err);
+        self.explain_why_borrow_contains_point(context, issued_borrow, None)
+            .emit(self.tcx, &mut err);
 
         err.buffer(&mut self.errors_buffer);
     }
@@ -445,17 +448,13 @@ pub(super) fn report_borrowed_value_does_not_live_long_enough(
         self.access_place_error_reported
             .insert((root_place.clone(), borrow_span));
 
-        let borrow_reason = self.find_why_borrow_contains_point(context, borrow);
-
-        if let Some(WriteKind::StorageDeadOrDrop(StorageDeadOrDrop::Destructor)) = kind
-        {
+        if let Some(WriteKind::StorageDeadOrDrop(StorageDeadOrDrop::Destructor)) = kind {
             // If a borrow of path `B` conflicts with drop of `D` (and
             // we're not in the uninteresting case where `B` is a
             // prefix of `D`), then report this as a more interesting
             // destructor conflict.
             if !borrow.borrowed_place.is_prefix_of(place_span.0) {
-                self.report_borrow_conflicts_with_destructor(
-                    context, borrow, borrow_reason, place_span, kind);
+                self.report_borrow_conflicts_with_destructor(context, borrow, place_span, kind);
                 return;
             }
         }
@@ -469,7 +468,6 @@ pub(super) fn report_borrowed_value_does_not_live_long_enough(
                 name,
                 &scope_tree,
                 &borrow,
-                borrow_reason,
                 drop_span,
                 borrow_span,
                 kind.map(|k| (k, place_span.0)),
@@ -478,7 +476,6 @@ pub(super) fn report_borrowed_value_does_not_live_long_enough(
                 context,
                 &scope_tree,
                 &borrow,
-                borrow_reason,
                 drop_span,
                 proper_span,
             ),
@@ -495,16 +492,15 @@ fn report_local_value_does_not_live_long_enough(
         name: &String,
         scope_tree: &Lrc<ScopeTree>,
         borrow: &BorrowData<'tcx>,
-        reason: BorrowContainsPointReason<'tcx>,
         drop_span: Span,
         borrow_span: Span,
         kind_place: Option<(WriteKind, &Place<'tcx>)>,
     ) -> DiagnosticBuilder<'cx> {
         debug!(
             "report_local_value_does_not_live_long_enough(\
-             {:?}, {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\
+             {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\
              )",
-            context, name, scope_tree, borrow, reason, drop_span, borrow_span
+            context, name, scope_tree, borrow, drop_span, borrow_span
         );
 
         let mut err = self.tcx.path_does_not_live_long_enough(
@@ -519,7 +515,9 @@ fn report_local_value_does_not_live_long_enough(
             format!("`{}` dropped here while still borrowed", name),
         );
 
-        self.report_why_borrow_contains_point(&mut err, reason, kind_place);
+        self.explain_why_borrow_contains_point(context, borrow, kind_place)
+            .emit(self.tcx, &mut err);
+
         err
     }
 
@@ -527,15 +525,14 @@ pub(super) fn report_borrow_conflicts_with_destructor(
         &mut self,
         context: Context,
         borrow: &BorrowData<'tcx>,
-        borrow_reason: BorrowContainsPointReason<'tcx>,
-        place_span: (&Place<'tcx>, Span),
+        (place, drop_span): (&Place<'tcx>, Span),
         kind: Option<WriteKind>,
     ) {
         debug!(
             "report_borrow_conflicts_with_destructor(\
-             {:?}, {:?}, {:?}, {:?} {:?}\
+             {:?}, {:?}, ({:?}, {:?}), {:?}\
              )",
-            context, borrow, borrow_reason, place_span, kind,
+            context, borrow, place, drop_span, kind,
         );
 
         let borrow_spans = self.retrieve_borrow_spans(borrow);
@@ -543,10 +540,7 @@ pub(super) fn report_borrow_conflicts_with_destructor(
 
         let mut err = self.tcx.cannot_borrow_across_destructor(borrow_span, Origin::Mir);
 
-        let drop_span = place_span.1;
-
         let (what_was_dropped, dropped_ty) = {
-            let place = place_span.0;
             let desc = match self.describe_place(place) {
                 Some(name) => format!("`{}`", name.as_str()),
                 None => format!("temporary value"),
@@ -571,17 +565,19 @@ pub(super) fn report_borrow_conflicts_with_destructor(
         };
         err.span_label(drop_span, label);
 
-        // Only give this note and suggestion if they could be relevant
-        match borrow_reason {
-            BorrowContainsPointReason::Liveness {..}
-            | BorrowContainsPointReason::DropLiveness {..} => {
+        // Only give this note and suggestion if they could be relevant.
+        let explanation = self.explain_why_borrow_contains_point(
+            context, borrow, kind.map(|k| (k, place)),
+        );
+        match explanation {
+            BorrowExplanation::UsedLater {..} |
+            BorrowExplanation::UsedLaterWhenDropped {..} => {
                 err.note("consider using a `let` binding to create a longer lived value");
-            }
-            BorrowContainsPointReason::OutlivesFreeRegion {..} => (),
+            },
+            _ => {},
         }
 
-        self.report_why_borrow_contains_point(
-            &mut err, borrow_reason, kind.map(|k| (k, place_span.0)));
+        explanation.emit(self.tcx, &mut err);
 
         err.buffer(&mut self.errors_buffer);
     }
@@ -607,6 +603,7 @@ fn report_thread_local_value_does_not_live_long_enough(
             "thread-local variables cannot be borrowed beyond the end of the function",
         );
         err.span_label(drop_span, "end of enclosing function is here");
+
         err
     }
 
@@ -615,15 +612,14 @@ fn report_temporary_value_does_not_live_long_enough(
         context: Context,
         scope_tree: &Lrc<ScopeTree>,
         borrow: &BorrowData<'tcx>,
-        reason: BorrowContainsPointReason<'tcx>,
         drop_span: Span,
         proper_span: Span,
     ) -> DiagnosticBuilder<'cx> {
         debug!(
             "report_temporary_value_does_not_live_long_enough(\
-             {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\
+             {:?}, {:?}, {:?}, {:?}, {:?}\
              )",
-            context, scope_tree, borrow, reason, drop_span, proper_span
+            context, scope_tree, borrow, drop_span, proper_span
         );
 
         let tcx = self.tcx;
@@ -632,16 +628,18 @@ fn report_temporary_value_does_not_live_long_enough(
         err.span_label(proper_span, "temporary value does not live long enough");
         err.span_label(drop_span, "temporary value only lives until here");
 
-        // Only give this note and suggestion if they could be relevant
-        match reason {
-            BorrowContainsPointReason::Liveness {..}
-            | BorrowContainsPointReason::DropLiveness {..} => {
+        let explanation = self.explain_why_borrow_contains_point(context, borrow, None);
+        match explanation {
+            BorrowExplanation::UsedLater(..) |
+            BorrowExplanation::UsedLaterInLoop(..) |
+            BorrowExplanation::UsedLaterWhenDropped(..) => {
+                // Only give this note and suggestion if it could be relevant.
                 err.note("consider using a `let` binding to create a longer lived value");
-            }
-            BorrowContainsPointReason::OutlivesFreeRegion {..} => (),
+            },
+            _ => {},
         }
+        explanation.emit(self.tcx, &mut err);
 
-        self.report_why_borrow_contains_point(&mut err, reason, None);
         err
     }
 
@@ -749,7 +747,7 @@ pub(super) fn report_illegal_mutation_of_borrowed(
 
         loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure");
 
-        self.explain_why_borrow_contains_point(context, loan, None, &mut err);
+        self.explain_why_borrow_contains_point(context, loan, None).emit(self.tcx, &mut err);
 
         err.buffer(&mut self.errors_buffer);
     }

src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs

@@ -121,9 +121,7 @@ struct DefUseVisitor<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
 
 enum DefUseResult {
     Def,
-
     UseLive { local: Local },
-
     UseDrop { local: Local },
 }
 

src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs

@@ -11,26 +11,71 @@
 use borrow_check::borrow_set::BorrowData;
 use borrow_check::nll::region_infer::Cause;
 use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
-use rustc::mir::{FakeReadCause, Local, Location, Place, TerminatorKind};
+use rustc::ty::{Region, TyCtxt};
+use rustc::mir::{FakeReadCause, Location, Place, TerminatorKind};
 use rustc_errors::DiagnosticBuilder;
-use rustc::ty::Region;
+use syntax_pos::Span;
+use syntax_pos::symbol::Symbol;
 
 mod find_use;
 
-#[derive(Copy, Clone, Debug)]
-pub enum BorrowContainsPointReason<'tcx> {
-    Liveness {
-        local: Local,
-        location: Location,
-        in_loop: bool,
-    },
-    DropLiveness {
-        local: Local,
-        location: Location,
-    },
-    OutlivesFreeRegion {
-        outlived_region: Option<Region<'tcx>>,
-    },
+pub(in borrow_check) enum BorrowExplanation<'tcx> {
+    UsedLater(bool, Option<FakeReadCause>, Span),
+    UsedLaterInLoop(bool, Span),
+    UsedLaterWhenDropped(Span, Symbol, bool),
+    MustBeValidFor(Region<'tcx>),
+    Unexplained,
+}
+
+impl<'tcx> BorrowExplanation<'tcx> {
+    pub(in borrow_check) fn emit<'cx, 'gcx>(
+        &self,
+        tcx: TyCtxt<'cx, 'gcx, 'tcx>,
+        err: &mut DiagnosticBuilder<'_>
+    ) {
+        match *self {
+            BorrowExplanation::UsedLater(is_in_closure, fake_read_cause, var_or_use_span) => {
+                let message = if is_in_closure {
+                    "borrow later captured here by closure"
+                } else if let Some(FakeReadCause::ForLet) = fake_read_cause {
+                    "borrow later stored here"
+                } else {
+                    "borrow later used here"
+                };
+                err.span_label(var_or_use_span, message);
+            },
+            BorrowExplanation::UsedLaterInLoop(is_in_closure, var_or_use_span) => {
+                let message = if is_in_closure {
+                    "borrow captured here by closure in later iteration of loop"
+                } else {
+                    "borrow used here in later iteration of loop"
+                };
+                err.span_label(var_or_use_span, message);
+            },
+            BorrowExplanation::UsedLaterWhenDropped(span, local_name, should_note_order) => {
+                err.span_label(
+                    span,
+                    format!("borrow later used here, when `{}` is dropped", local_name),
+                );
+
+                if should_note_order {
+                    err.note(
+                        "values in a scope are dropped \
+                         in the opposite order they are defined",
+                    );
+                }
+            },
+            BorrowExplanation::MustBeValidFor(region) => {
+                tcx.note_and_explain_free_region(
+                    err,
+                    "borrowed value must be valid for ",
+                    region,
+                    "...",
+                );
+            },
+            _ => {},
+        }
+    }
 }
 
 impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
@@ -53,23 +98,7 @@ pub(in borrow_check) fn explain_why_borrow_contains_point(
         context: Context,
         borrow: &BorrowData<'tcx>,
         kind_place: Option<(WriteKind, &Place<'tcx>)>,
-        err: &mut DiagnosticBuilder<'_>,
-    ) {
-        let reason = self.find_why_borrow_contains_point(context, borrow);
-        self.report_why_borrow_contains_point(err, reason, kind_place);
-    }
-
-    /// Finds the reason that [explain_why_borrow_contains_point] will report
-    /// but doesn't add it to any message. This is a separate function in case
-    /// the caller wants to change the error they report based on the reason
-    /// that will be reported.
-    pub(in borrow_check) fn find_why_borrow_contains_point(
-        &self,
-        context: Context,
-        borrow: &BorrowData<'tcx>
-    ) -> BorrowContainsPointReason<'tcx> {
-        use self::BorrowContainsPointReason::*;
-
+    ) -> BorrowExplanation<'tcx> {
         debug!(
             "find_why_borrow_contains_point(context={:?}, borrow={:?})",
             context, borrow,
@@ -80,113 +109,67 @@ pub(in borrow_check) fn find_why_borrow_contains_point(
         let tcx = self.tcx;
 
         let borrow_region_vid = regioncx.to_region_vid(borrow.region);
-
         debug!(
             "explain_why_borrow_contains_point: borrow_region_vid={:?}",
             borrow_region_vid
         );
 
         let region_sub = regioncx.find_sub_region_live_at(borrow_region_vid, context.loc);
-
         debug!(
             "explain_why_borrow_contains_point: region_sub={:?}",
             region_sub
         );
 
-        match find_use::find(mir, regioncx, tcx, region_sub, context.loc) {
-            Some(Cause::LiveVar(local, location)) => Liveness {
-                local,
-                location,
-                in_loop: self.is_borrow_location_in_loop(context.loc),
-            },
-            Some(Cause::DropVar(local, location)) => DropLiveness {
-                local,
-                location,
-            },
-            None => OutlivesFreeRegion {
-                outlived_region: regioncx.to_error_region(region_sub),
-            },
-        }
-    }
-
-    /// Adds annotations to `err` for the explanation `reason`. This is a
-    /// separate method so that the caller can change their error message based
-    /// on the reason that is going to be reported.
-    pub (in borrow_check) fn report_why_borrow_contains_point(
-        &self,
-        err: &mut DiagnosticBuilder,
-        reason: BorrowContainsPointReason<'tcx>,
-        kind_place: Option<(WriteKind, &Place<'tcx>)>,
-    ) {
-        use self::BorrowContainsPointReason::*;
-
-        debug!(
-            "find_why_borrow_contains_point(reason={:?}, kind_place={:?})",
-            reason, kind_place,
-        );
-
-        let mir = self.mir;
-
-        match reason {
-            Liveness { local, location, in_loop } => {
+         match find_use::find(mir, regioncx, tcx, region_sub, context.loc) {
+            Some(Cause::LiveVar(local, location)) => {
                 let span = mir.source_info(location).span;
                 let spans = self.move_spans(&Place::Local(local), location)
                     .or_else(|| self.borrow_spans(span, location));
-                let message = if in_loop {
-                    if spans.for_closure() {
-                        "borrow captured here by closure in later iteration of loop"
-                    } else {
-                        "borrow used here in later iteration of loop"
-                    }
+
+                if self.is_borrow_location_in_loop(context.loc) {
+                    BorrowExplanation::UsedLaterInLoop(spans.for_closure(), spans.var_or_use())
                 } else {
-                    if spans.for_closure() {
-                        "borrow later captured here by closure"
-                    } else {
-                        // Check if the location represents a `FakeRead`, and adapt the error
-                        // message to the `FakeReadCause` it is from: in particular,
-                        // the ones inserted in optimized `let var = <expr>` patterns.
-                        match self.retrieve_fake_read_cause_for_location(&location) {
-                            Some(FakeReadCause::ForLet) => "borrow later stored here",
-                            _ => "borrow later used here"
-                        }
-                    }
-                };
-                err.span_label(spans.var_or_use(), message);
+                    // Check if the location represents a `FakeRead`, and adapt the error
+                    // message to the `FakeReadCause` it is from: in particular,
+                    // the ones inserted in optimized `let var = <expr>` patterns.
+                    BorrowExplanation::UsedLater(
+                        spans.for_closure(),
+                        self.retrieve_fake_read_cause_for_location(&location),
+                        spans.var_or_use()
+                    )
+                }
             }
-            DropLiveness { local, location } => match &mir.local_decls[local].name {
-                Some(local_name) => {
-                    err.span_label(
-                        mir.source_info(location).span,
-                        format!("borrow later used here, when `{}` is dropped", local_name),
-                    );
 
+            Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name {
+                Some(local_name) => {
+                    let mut should_note_order = false;
                     if let Some((WriteKind::StorageDeadOrDrop(_), place)) = kind_place {
                         if let Place::Local(borrowed_local) = place {
                             let dropped_local_scope = mir.local_decls[local].visibility_scope;
                             let borrowed_local_scope =
-                            mir.local_decls[*borrowed_local].visibility_scope;
+                                mir.local_decls[*borrowed_local].visibility_scope;
 
                             if mir.is_sub_scope(borrowed_local_scope, dropped_local_scope) {
-                                err.note(
-                                "values in a scope are dropped \
-                                                     in the opposite order they are defined",
-                                );
+                                should_note_order = true;
                             }
                         }
                     }
-                }
 
-                None => {}
-            }
-            OutlivesFreeRegion { outlived_region: Some(region) } => {
-                self.tcx.note_and_explain_free_region(
-                    err,
-                    "borrowed value must be valid for ",
-                    region,
-                    "...",
-                );
-            }
-            OutlivesFreeRegion { outlived_region: None } => (),
+                    BorrowExplanation::UsedLaterWhenDropped(
+                        mir.source_info(location).span,
+                        *local_name,
+                        should_note_order
+                    )
+                },
+
+                None => BorrowExplanation::Unexplained,
+            },
+
+            None => if let Some(region) = regioncx.to_error_region(region_sub) {
+                BorrowExplanation::MustBeValidFor(region)
+            } else {
+                BorrowExplanation::Unexplained
+            },
         }
     }
 
@@ -262,4 +245,3 @@ fn is_borrow_location_in_loop(
         false
     }
 }
-