Skip to content

Commit

Permalink
Buffer errors in MIR borrow check
Browse files Browse the repository at this point in the history
(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.)
  • Loading branch information
spastorino authored and pnkfelix committed Jul 23, 2018
1 parent da935e9 commit 3d3e0aa
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 43 deletions.
12 changes: 12 additions & 0 deletions src/librustc_errors/diagnostic_builder.rs
Expand Up @@ -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 |
Expand Down
29 changes: 14 additions & 15 deletions src/librustc_mir/borrow_check/error_reporting.rs
Expand Up @@ -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 "

Expand Down Expand Up @@ -143,7 +143,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}

err.emit();
err.buffer(&mut self.errors_buffer);
}
}

Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down
29 changes: 22 additions & 7 deletions src/librustc_mir/borrow_check/mod.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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];
Expand All @@ -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,
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/borrow_check/move_errors.rs
Expand Up @@ -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);
Expand Down Expand Up @@ -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, .. }
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/mutability_errors.rs
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions src/librustc_mir/borrow_check/nll/mod.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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>>>,
Expand Down Expand Up @@ -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.
Expand All @@ -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)
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 3d3e0aa

Please sign in to comment.