From 3d3e0aa57190db4519d4e59ab606f685c21bfc7f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 18 Jul 2018 18:10:08 -0300 Subject: [PATCH] Buffer errors in MIR borrow check (pnkfelix updated to address tidy, and to change the buffer from `Vec>` to a `Vec` in order to avoid painful lifetime maintenance.) --- src/librustc_errors/diagnostic_builder.rs | 12 ++++++++ .../borrow_check/error_reporting.rs | 29 +++++++++---------- src/librustc_mir/borrow_check/mod.rs | 29 ++++++++++++++----- src/librustc_mir/borrow_check/move_errors.rs | 6 ++-- .../borrow_check/mutability_errors.rs | 2 +- src/librustc_mir/borrow_check/nll/mod.rs | 12 +++++--- .../nll/region_infer/error_reporting/mod.rs | 26 ++++++++++------- .../borrow_check/nll/region_infer/mod.rs | 14 +++++++-- 8 files changed, 87 insertions(+), 43 deletions(-) diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 24ece514a47..8f99ad87cb8 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -88,6 +88,18 @@ pub fn emit(&mut self) { self.cancel(); } + /// Buffers the diagnostic for later emission. + pub fn buffer(self, buffered_diagnostics: &mut Vec) { + // 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 | diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 5dca01f8842..9e822d28056 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -58,7 +58,7 @@ pub(super) fn report_use_of_moved_or_uninitialized( 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(), @@ -66,9 +66,9 @@ pub(super) fn report_use_of_moved_or_uninitialized( .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 @@ pub(super) fn report_use_of_moved_or_uninitialized( } } - err.emit(); + err.buffer(&mut self.errors_buffer); } } @@ -173,7 +173,7 @@ pub(super) fn report_move_out_while_borrowed( ); 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 @@ pub(super) fn report_use_while_mutably_borrowed( ); 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 @@ pub(super) fn report_conflicting_borrow( 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 @@ fn report_scoped_local_value_does_not_live_long_enough( 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 @@ fn report_scoped_temporary_value_does_not_live_long_enough( ); 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 @@ fn report_unscoped_local_value_does_not_live_long_enough( 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 @@ fn report_unscoped_temporary_value_does_not_live_long_enough( 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 @@ pub(super) fn report_illegal_mutation_of_borrowed( 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 @@ pub(super) fn report_illegal_reassignment( } } err.span_label(span, msg); - err.emit(); + err.buffer(&mut self.errors_buffer); } } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 28fd7272b78..08500efe8ef 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -23,6 +23,7 @@ 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>>) = 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>, + /// Errors to be reported buffer + errors_buffer: Vec, /// 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, @@ -1353,13 +1367,14 @@ fn check_for_local_borrow(&mut self, borrow: &BorrowData<'tcx>, yield_span: Span 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); } } diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index b7ba5c855b9..eacdbe8e945 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/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>) { + pub(crate) fn report_move_errors(&mut self, move_errors: Vec>) { let grouped_errors = self.group_move_errors(move_errors); for error in grouped_errors { self.report(error); @@ -218,7 +218,7 @@ fn append_binding_error( }; } - 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 @@ fn report(&self, error: GroupedMoveError<'tcx>) { }; self.add_move_hints(error, &mut err, err_span); - err.emit(); + err.buffer(&mut self.errors_buffer); } fn add_move_hints( diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 2a074a84e63..760d0a91b79 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -378,7 +378,7 @@ pub(super) fn report_mutability_error( } } - err.emit(); + err.buffer(&mut self.errors_buffer); } // Does this place refer to what the user sees as an upvar diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 76f8fa206be..b8b38b58319 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -23,6 +23,7 @@ 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>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, + errors_buffer: &mut Vec, ) -> ( RegionInferenceContext<'tcx>, Option>>, @@ -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, ®ioncx, &closure_region_requirements); + dump_annotation(infcx, &mir, def_id, ®ioncx, &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, + errors_buffer: &mut Vec, ) { 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); } } diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index c89dc889b5e..131e1defc1f 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -18,6 +18,7 @@ use rustc::ty::RegionVid; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::indexed_vec::IndexVec; +use rustc_errors::Diagnostic; use std::fmt; use syntax_pos::Span; @@ -199,6 +200,7 @@ pub(super) fn report_error( fr: RegionVid, outlived_fr: RegionVid, blame_span: Span, + errors_buffer: &mut Vec, ) { debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr); @@ -247,9 +249,11 @@ pub(super) fn report_error( match category { ConstraintCategory::AssignmentToUpvar | ConstraintCategory::CallArgumentToUpvar => - self.report_closure_error(mir, infcx, mir_def_id, fr, outlived_fr, category, span), + self.report_closure_error( + mir, infcx, mir_def_id, fr, outlived_fr, category, span, errors_buffer), _ => - self.report_general_error(mir, infcx, mir_def_id, fr, outlived_fr, category, span), + self.report_general_error( + mir, infcx, mir_def_id, fr, outlived_fr, category, span, errors_buffer), } } @@ -262,6 +266,7 @@ fn report_closure_error( outlived_fr: RegionVid, category: &ConstraintCategory, span: &Span, + errors_buffer: &mut Vec, ) { let fr_name_and_span = self.get_var_name_and_span_for_region( infcx.tcx, mir, fr); @@ -269,11 +274,11 @@ fn report_closure_error( infcx.tcx, mir,outlived_fr); if fr_name_and_span.is_none() && outlived_fr_name_and_span.is_none() { - return self.report_general_error(mir, infcx, mir_def_id, fr, outlived_fr, category, - span); + return self.report_general_error( + mir, infcx, mir_def_id, fr, outlived_fr, category, span, errors_buffer); } - let diag = &mut infcx.tcx.sess.struct_span_err( + let mut diag = infcx.tcx.sess.struct_span_err( *span, &format!("borrowed data escapes outside of closure"), ); @@ -297,7 +302,7 @@ fn report_closure_error( } } - diag.emit(); + diag.buffer(errors_buffer); } fn report_general_error( @@ -309,23 +314,24 @@ fn report_general_error( outlived_fr: RegionVid, category: &ConstraintCategory, span: &Span, + errors_buffer: &mut Vec, ) { - let diag = &mut infcx.tcx.sess.struct_span_err( + let mut diag = infcx.tcx.sess.struct_span_err( *span, &format!("unsatisfied lifetime constraints"), // FIXME ); let counter = &mut 1; let fr_name = self.give_region_a_name( - infcx.tcx, mir, mir_def_id, fr, counter, diag); + infcx.tcx, mir, mir_def_id, fr, counter, &mut diag); let outlived_fr_name = self.give_region_a_name( - infcx.tcx, mir, mir_def_id, outlived_fr, counter, diag); + infcx.tcx, mir, mir_def_id, outlived_fr, counter, &mut diag); diag.span_label(*span, format!( "{} requires that `{}` must outlive `{}`", category, fr_name, outlived_fr_name, )); - diag.emit(); + diag.buffer(errors_buffer); } // Find some constraint `X: Y` where: diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 5159fdc9fba..37df311805e 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -30,6 +30,7 @@ use rustc_data_structures::graph::scc::Sccs; use rustc_data_structures::indexed_set::{IdxSet, IdxSetBuf}; use rustc_data_structures::indexed_vec::IndexVec; +use rustc_errors::Diagnostic; use std::rc::Rc; @@ -360,11 +361,12 @@ pub(super) fn solve<'gcx>( infcx: &InferCtxt<'_, 'gcx, 'tcx>, mir: &Mir<'tcx>, mir_def_id: DefId, + errors_buffer: &mut Vec, ) -> Option> { common::time( infcx.tcx.sess, &format!("solve_nll_region_constraints({:?})", mir_def_id), - || self.solve_inner(infcx, mir, mir_def_id), + || self.solve_inner(infcx, mir, mir_def_id, errors_buffer), ) } @@ -373,6 +375,7 @@ fn solve_inner<'gcx>( infcx: &InferCtxt<'_, 'gcx, 'tcx>, mir: &Mir<'tcx>, mir_def_id: DefId, + errors_buffer: &mut Vec, ) -> Option> { self.propagate_constraints(mir); @@ -389,7 +392,8 @@ fn solve_inner<'gcx>( self.check_type_tests(infcx, mir, mir_def_id, outlives_requirements.as_mut()); - self.check_universal_regions(infcx, mir, mir_def_id, outlives_requirements.as_mut()); + self.check_universal_regions( + infcx, mir, mir_def_id, outlives_requirements.as_mut(), errors_buffer); let outlives_requirements = outlives_requirements.unwrap_or(vec![]); @@ -834,6 +838,7 @@ fn check_universal_regions<'gcx>( mir: &Mir<'tcx>, mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, + errors_buffer: &mut Vec, ) { // The universal regions are always found in a prefix of the // full list. @@ -851,6 +856,7 @@ fn check_universal_regions<'gcx>( mir_def_id, fr, &mut propagated_outlives_requirements, + errors_buffer, ); } } @@ -870,6 +876,7 @@ fn check_universal_region<'gcx>( mir_def_id: DefId, longer_fr: RegionVid, propagated_outlives_requirements: &mut Option<&mut Vec>>, + errors_buffer: &mut Vec, ) { debug!("check_universal_region(fr={:?})", longer_fr); @@ -924,7 +931,8 @@ fn check_universal_region<'gcx>( // Note: in this case, we use the unapproximated regions // to report the error. This gives better error messages // in some cases. - self.report_error(mir, infcx, mir_def_id, longer_fr, shorter_fr, blame_span); + self.report_error( + mir, infcx, mir_def_id, longer_fr, shorter_fr, blame_span, errors_buffer); } } }