Buffer errors in MIR borrow check

(pnkfelix updated to address tidy, and to change the buffer from
`Vec<DiagnosticBuilder<'errs>>` to a `Vec<Diagnostic>` in order to
avoid painful lifetime maintenance.)

Author: spastorino

src/librustc_errors/diagnostic_builder.rs

@@ -88,6 +88,18 @@ impl<'a> DiagnosticBuilder<'a> {
         self.cancel();
     }
 
+    /// Buffers the diagnostic for later emission.
+    pub fn buffer(self, buffered_diagnostics: &mut Vec<Diagnostic>) {
+        // We need to use `ptr::read` because `DiagnosticBuilder`
+        // implements `Drop`.
+        let diagnostic;
+        unsafe {
+            diagnostic = ::std::ptr::read(&self.diagnostic);
+            ::std::mem::forget(self);
+        };
+        buffered_diagnostics.push(diagnostic);
+    }
+
     pub fn is_error(&self) -> bool {
         match self.level {
             Level::Bug |

src/librustc_mir/borrow_check/error_reporting.rs

@@ -58,17 +58,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 Some(name) => format!("`{}`", name),
                 None => "value".to_owned(),
             };
-            self.tcx
+            let mut err = self.tcx
                 .cannot_act_on_uninitialized_variable(
                     span,
                     desired_action.as_noun(),
                     &self
                         .describe_place_with_options(place, IncludingDowncast(true))
                         .unwrap_or("_".to_owned()),
                     Origin::Mir,
-                )
-                .span_label(span, format!("use of possibly uninitialized {}", item_msg))
-                .emit();
+                );
+            err.span_label(span, format!("use of possibly uninitialized {}", item_msg));
+            err.buffer(&mut self.errors_buffer);
         } else {
             let msg = ""; //FIXME: add "partially " or "collaterally "
 
@@ -143,7 +143,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 }
             }
 
-            err.emit();
+            err.buffer(&mut self.errors_buffer);
         }
     }
 
@@ -173,7 +173,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         );
         err.span_label(span, format!("move out of {} occurs here", value_msg));
         self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     pub(super) fn report_use_while_mutably_borrowed(
@@ -194,8 +194,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         );
 
         self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
-
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     /// Finds the span of arguments of a closure (within `maybe_closure_span`) and its usage of
@@ -391,7 +390,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
         self.explain_why_borrow_contains_point(context, issued_borrow, None, &mut err);
 
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     pub(super) fn report_borrowed_value_does_not_live_long_enough(
@@ -513,7 +512,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             format!("`{}` dropped here while still borrowed", name),
         );
         self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     fn report_scoped_temporary_value_does_not_live_long_enough(
@@ -535,7 +534,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         );
         err.note("consider using a `let` binding to increase its lifetime");
         self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     fn report_unscoped_local_value_does_not_live_long_enough(
@@ -563,7 +562,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         err.span_label(drop_span, "borrowed value only lives until here");
 
         self.explain_why_borrow_contains_point(context, borrow, kind_place, &mut err);
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     fn report_unscoped_temporary_value_does_not_live_long_enough(
@@ -589,7 +588,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         err.span_label(drop_span, "temporary value only lives until here");
 
         self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     pub(super) fn report_illegal_mutation_of_borrowed(
@@ -608,7 +607,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
         self.explain_why_borrow_contains_point(context, loan, None, &mut err);
 
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     /// Reports an illegal reassignment; for example, an assignment to
@@ -679,7 +678,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             }
         }
         err.span_label(span, msg);
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 }
 

src/librustc_mir/borrow_check/mod.rs

@@ -23,6 +23,7 @@ use rustc::mir::{Terminator, TerminatorKind};
 use rustc::ty::query::Providers;
 use rustc::ty::{self, ParamEnv, TyCtxt};
 
+use rustc_errors::{Diagnostic, DiagnosticBuilder};
 use rustc_data_structures::graph::dominators::Dominators;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::indexed_set::IdxSetBuf;
@@ -148,6 +149,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
     let mir = &mir; // no further changes
     let location_table = &LocationTable::new(mir);
 
+    let mut errors_buffer = Vec::new();
     let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<MoveError<'tcx>>>) =
         match MoveData::gather_moves(mir, tcx) {
             Ok(move_data) => (move_data, None),
@@ -214,6 +216,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
         &mut flow_inits,
         &mdpe.move_data,
         &borrow_set,
+        &mut errors_buffer,
     );
     let regioncx = Rc::new(regioncx);
 
@@ -252,6 +255,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
         access_place_error_reported: FxHashSet(),
         reservation_error_reported: FxHashSet(),
         moved_error_reported: FxHashSet(),
+        errors_buffer,
         nonlexical_regioncx: regioncx,
         used_mut: FxHashSet(),
         used_mut_upvars: SmallVec::new(),
@@ -287,10 +291,12 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
 
     debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
 
+    let used_mut = mbcx.used_mut;
+
     for local in mbcx
         .mir
         .mut_vars_and_args_iter()
-        .filter(|local| !mbcx.used_mut.contains(local))
+        .filter(|local| !used_mut.contains(local))
     {
         if let ClearCrossCrate::Set(ref vsi) = mbcx.mir.source_scope_local_data {
             let local_decl = &mbcx.mir.local_decls[local];
@@ -311,16 +317,22 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
             let span = local_decl.source_info.span;
             let mut_span = tcx.sess.codemap().span_until_non_whitespace(span);
 
-            tcx.struct_span_lint_node(
+            let mut err = tcx.struct_span_lint_node(
                 UNUSED_MUT,
                 vsi[local_decl.source_info.scope].lint_root,
                 span,
                 "variable does not need to be mutable",
-            ).span_suggestion_short(mut_span, "remove this `mut`", "".to_owned())
-                .emit();
+            );
+            err.span_suggestion_short(mut_span, "remove this `mut`", "".to_owned());
+
+            err.buffer(&mut mbcx.errors_buffer);
         }
     }
 
+    for diag in mbcx.errors_buffer.drain(..) {
+        DiagnosticBuilder::new_diagnostic(mbcx.tcx.sess.diagnostic(), diag).emit();
+    }
+
     let result = BorrowCheckResult {
         closure_requirements: opt_closure_req,
         used_mut_upvars: mbcx.used_mut_upvars,
@@ -367,6 +379,8 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
     /// This field keeps track of errors reported in the checking of moved variables,
     /// so that we don't report seemingly duplicate errors.
     moved_error_reported: FxHashSet<Place<'tcx>>,
+    /// Errors to be reported buffer
+    errors_buffer: Vec<Diagnostic>,
     /// This field keeps track of all the local variables that are declared mut and are mutated.
     /// Used for the warning issued by an unused mutable local variable.
     used_mut: FxHashSet<Local>,
@@ -1353,13 +1367,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         debug!("check_for_local_borrow({:?})", borrow);
 
         if borrow_of_local_data(&borrow.borrowed_place) {
-            self.tcx
+            let err = self.tcx
                 .cannot_borrow_across_generator_yield(
                     self.retrieve_borrow_span(borrow),
                     yield_span,
                     Origin::Mir,
-                )
-                .emit();
+                );
+
+            err.buffer(&mut self.errors_buffer);
         }
     }
 

src/librustc_mir/borrow_check/move_errors.rs

@@ -59,7 +59,7 @@ enum GroupedMoveError<'tcx> {
 }
 
 impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
-    pub(crate) fn report_move_errors(&self, move_errors: Vec<MoveError<'tcx>>) {
+    pub(crate) fn report_move_errors(&mut self, move_errors: Vec<MoveError<'tcx>>) {
         let grouped_errors = self.group_move_errors(move_errors);
         for error in grouped_errors {
             self.report(error);
@@ -218,7 +218,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
         };
     }
 
-    fn report(&self, error: GroupedMoveError<'tcx>) {
+    fn report(&mut self, error: GroupedMoveError<'tcx>) {
         let (mut err, err_span) = {
             let (span, kind): (Span, &IllegalMoveOriginKind) = match error {
                 GroupedMoveError::MovesFromMatchPlace { span, ref kind, .. }
@@ -286,7 +286,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
         };
 
         self.add_move_hints(error, &mut err, err_span);
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     fn add_move_hints(

src/librustc_mir/borrow_check/mutability_errors.rs

@@ -378,7 +378,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
             }
         }
 
-        err.emit();
+        err.buffer(&mut self.errors_buffer);
     }
 
     // Does this place refer to what the user sees as an upvar

src/librustc_mir/borrow_check/nll/mod.rs

@@ -23,6 +23,7 @@ use rustc::infer::InferCtxt;
 use rustc::mir::{ClosureOutlivesSubject, ClosureRegionRequirements, Mir};
 use rustc::ty::{self, RegionKind, RegionVid};
 use rustc::util::nodemap::FxHashMap;
+use rustc_errors::Diagnostic;
 use std::collections::BTreeSet;
 use std::fmt::Debug;
 use std::env;
@@ -91,6 +92,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
     flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'cx, 'gcx, 'tcx>>,
     move_data: &MoveData<'tcx>,
     borrow_set: &BorrowSet<'tcx>,
+    errors_buffer: &mut Vec<Diagnostic>,
 ) -> (
     RegionInferenceContext<'tcx>,
     Option<Rc<Output<RegionVid, BorrowIndex, LocationIndex>>>,
@@ -190,7 +192,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
     });
 
     // Solve the region constraints.
-    let closure_region_requirements = regioncx.solve(infcx, &mir, def_id);
+    let closure_region_requirements = regioncx.solve(infcx, &mir, def_id, errors_buffer);
 
     // Dump MIR results into a file, if that is enabled. This let us
     // write unit-tests, as well as helping with debugging.
@@ -205,7 +207,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
 
     // We also have a `#[rustc_nll]` annotation that causes us to dump
     // information
-    dump_annotation(infcx, &mir, def_id, &regioncx, &closure_region_requirements);
+    dump_annotation(infcx, &mir, def_id, &regioncx, &closure_region_requirements, errors_buffer);
 
     (regioncx, polonius_output, closure_region_requirements)
 }
@@ -323,6 +325,7 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
     mir_def_id: DefId,
     regioncx: &RegionInferenceContext,
     closure_region_requirements: &Option<ClosureRegionRequirements>,
+    errors_buffer: &mut Vec<Diagnostic>,
 ) {
     let tcx = infcx.tcx;
     let base_def_id = tcx.closure_base_def_id(mir_def_id);
@@ -357,14 +360,15 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
             Ok(())
         }).unwrap();
 
-        err.emit();
+        err.buffer(errors_buffer);
     } else {
         let mut err = tcx
             .sess
             .diagnostic()
             .span_note_diag(mir.span, "No external requirements");
         regioncx.annotate(&mut err);
-        err.emit();
+
+        err.buffer(errors_buffer);
     }
 }