Skip to content

Commit

Permalink
Various refactorings to clean up nll diagnostics
Browse files Browse the repository at this point in the history 
- Create ErrorReportingCtx and ErrorConstraintInfo, vasting reducing the
  number of arguments passed around everywhere in the error reporting code
- Create RegionErrorNamingCtx, making a given lifetime have consistent
  numbering thoughout all error messages for that MIR def.
- Make the error reporting code return the DiagnosticBuilder rather than
  directly buffer the Diagnostic. This makes it easier to modify the
  diagnostic later, e.g. to add suggestions.
  • Loading branch information
@mark-i-m
mark-i-m committed Sep 12, 2019
1 parent eb48d6b commit 68b1a87
Show file tree
Hide file tree
Showing 3 changed files with 303 additions and 207 deletions.
241 changes: 126 additions & 115 deletions src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
View file
Expand Up @@ -13,7 +13,7 @@ use rustc::infer::NLLRegionVariableOrigin;
use rustc::mir::{ConstraintCategory, Location, Body};
use rustc::ty::{self, RegionVid};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_errors::{Diagnostic, DiagnosticBuilder};
use rustc_errors::DiagnosticBuilder;
use std::collections::VecDeque;
use syntax::errors::Applicability;
use syntax::symbol::kw;
Expand All @@ -22,7 +22,7 @@ use syntax_pos::Span;
mod region_name;
mod var_name;

crate use self::region_name::{RegionName, RegionNameSource};
crate use self::region_name::{RegionName, RegionNameSource, RegionErrorNamingCtx};

impl ConstraintDescription for ConstraintCategory {
fn description(&self) -> &'static str {
Expand Down Expand Up @@ -54,6 +54,30 @@ enum Trace {
NotVisited,
}

/// Various pieces of state used when reporting borrow checker errors.
pub struct ErrorReportingCtx<'a, 'b, 'tcx> {
rinfcx: &'b RegionInferenceContext<'tcx>,
infcx: &'b InferCtxt<'a, 'tcx>,

mir_def_id: DefId,
body: &'b Body<'tcx>,
upvars: &'b [Upvar],
}

/// Information about the various region constraints involved in a borrow checker error.
#[derive(Clone, Debug)]
pub struct ErrorConstraintInfo {
// fr: outlived_fr
fr: RegionVid,
fr_is_local: bool,
outlived_fr: RegionVid,
outlived_fr_is_local: bool,

// Category and span for best blame constraint
category: ConstraintCategory,
span: Span,
}

impl<'tcx> RegionInferenceContext<'tcx> {
/// Tries to find the best constraint to blame for the fact that
/// `R: from_region`, where `R` is some region that meets
Expand Down Expand Up @@ -257,16 +281,16 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// ```
///
/// Here we would be invoked with `fr = 'a` and `outlived_fr = `'b`.
pub(super) fn report_error(
&self,
pub(super) fn report_error<'a>(
&'a self,
body: &Body<'tcx>,
upvars: &[Upvar],
infcx: &InferCtxt<'_, 'tcx>,
infcx: &'a InferCtxt<'a, 'tcx>,
mir_def_id: DefId,
fr: RegionVid,
outlived_fr: RegionVid,
errors_buffer: &mut Vec<Diagnostic>,
) {
renctx: &mut RegionErrorNamingCtx,
) -> DiagnosticBuilder<'a> {
debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);

let (category, _, span) = self.best_blame_constraint(body, fr, |r| {
Expand All @@ -279,8 +303,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let tables = infcx.tcx.typeck_tables_of(mir_def_id);
let nice = NiceRegionError::new_from_span(infcx, span, o, f, Some(tables));
if let Some(diag) = nice.try_report_from_nll() {
diag.buffer(errors_buffer);
return;
return diag;
}
}

Expand All @@ -293,45 +316,35 @@ impl<'tcx> RegionInferenceContext<'tcx> {
"report_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}",
fr_is_local, outlived_fr_is_local, category
);

let errctx = ErrorReportingCtx {
rinfcx: self,
infcx,
mir_def_id,
body,
upvars,
};

let errci = ErrorConstraintInfo {
fr, outlived_fr, fr_is_local, outlived_fr_is_local, category, span
};

match (category, fr_is_local, outlived_fr_is_local) {
(ConstraintCategory::Return, true, false) if self.is_closure_fn_mut(infcx, fr) => {
self.report_fnmut_error(
body,
upvars,
infcx,
mir_def_id,
fr,
outlived_fr,
span,
errors_buffer,
)
self.report_fnmut_error(&errctx, &errci, renctx)
}
(ConstraintCategory::Assignment, true, false)
| (ConstraintCategory::CallArgument, true, false) => self.report_escaping_data_error(
body,
upvars,
infcx,
mir_def_id,
fr,
outlived_fr,
category,
span,
errors_buffer,
),
_ => self.report_general_error(
body,
upvars,
infcx,
mir_def_id,
fr,
fr_is_local,
outlived_fr,
outlived_fr_is_local,
category,
span,
errors_buffer,
),
};
| (ConstraintCategory::CallArgument, true, false) => {
let mut db = self.report_escaping_data_error(&errctx, &errci, renctx);

db
}
_ => {
let mut db = self.report_general_error(&errctx, &errci, renctx);

db
}
}
}

/// We have a constraint `fr1: fr2` that is not satisfied, where
Expand Down Expand Up @@ -379,19 +392,19 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// ```
fn report_fnmut_error(
&self,
body: &Body<'tcx>,
upvars: &[Upvar],
infcx: &InferCtxt<'_, 'tcx>,
mir_def_id: DefId,
_fr: RegionVid,
outlived_fr: RegionVid,
span: Span,
errors_buffer: &mut Vec<Diagnostic>,
) {
let mut diag = infcx
errctx: &ErrorReportingCtx<'_, '_, 'tcx>,
errci: &ErrorConstraintInfo,
renctx: &mut RegionErrorNamingCtx,
) -> DiagnosticBuilder<'_> {
let ErrorConstraintInfo {
outlived_fr, span, ..
} = errci;

let mut diag = errctx
.infcx
.tcx
.sess
.struct_span_err(span, "captured variable cannot escape `FnMut` closure body");
.struct_span_err(*span, "captured variable cannot escape `FnMut` closure body");

// We should check if the return type of this closure is in fact a closure - in that
// case, we can special case the error further.
Expand All @@ -403,11 +416,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
"returns a reference to a captured variable which escapes the closure body"
};

diag.span_label(span, message);
diag.span_label(*span, message);

match self.give_region_a_name(infcx, body, upvars, mir_def_id, outlived_fr, &mut 1)
.unwrap().source
{
match self.give_region_a_name(errctx, renctx, *outlived_fr).unwrap().source {
RegionNameSource::NamedEarlyBoundRegion(fr_span)
| RegionNameSource::NamedFreeRegion(fr_span)
| RegionNameSource::SynthesizedFreeEnvRegion(fr_span, _)
Expand All @@ -427,7 +438,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
);
diag.note("...therefore, they cannot allow references to captured variables to escape");

diag.buffer(errors_buffer);
diag
}

/// Reports a error specifically for when data is escaping a closure.
Expand All @@ -444,20 +455,22 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// ```
fn report_escaping_data_error(
&self,
body: &Body<'tcx>,
upvars: &[Upvar],
infcx: &InferCtxt<'_, 'tcx>,
mir_def_id: DefId,
fr: RegionVid,
outlived_fr: RegionVid,
category: ConstraintCategory,
span: Span,
errors_buffer: &mut Vec<Diagnostic>,
) {
errctx: &ErrorReportingCtx<'_, '_, 'tcx>,
errci: &ErrorConstraintInfo,
renctx: &mut RegionErrorNamingCtx,
) -> DiagnosticBuilder<'_> {
let ErrorReportingCtx {
infcx, body, upvars, ..
} = errctx;

let ErrorConstraintInfo {
span, category, ..
} = errci;

let fr_name_and_span =
self.get_var_name_and_span_for_region(infcx.tcx, body, upvars, fr);
self.get_var_name_and_span_for_region(infcx.tcx, body, upvars, errci.fr);
let outlived_fr_name_and_span =
self.get_var_name_and_span_for_region(infcx.tcx, body, upvars, outlived_fr);
self.get_var_name_and_span_for_region(infcx.tcx, body, upvars, errci.outlived_fr);

let escapes_from = match self.universal_regions.defining_ty {
DefiningTy::Closure(..) => "closure",
Expand All @@ -469,27 +482,23 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// Revert to the normal error in these cases.
// Assignments aren't "escapes" in function items.
if (fr_name_and_span.is_none() && outlived_fr_name_and_span.is_none())
|| (category == ConstraintCategory::Assignment && escapes_from == "function")
|| (*category == ConstraintCategory::Assignment && escapes_from == "function")
|| escapes_from == "const"
{
return self.report_general_error(
body,
upvars,
infcx,
mir_def_id,
fr,
true,
outlived_fr,
false,
category,
span,
errors_buffer,
errctx,
&ErrorConstraintInfo {
fr_is_local: true,
outlived_fr_is_local: false,
.. *errci
},
renctx,
);
}

let mut diag = borrowck_errors::borrowed_data_escapes_closure(
infcx.tcx,
span,
*span,
escapes_from,
);

Expand All @@ -513,12 +522,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
);

diag.span_label(
span,
*span,
format!("`{}` escapes the {} body here", fr_name, escapes_from),
);
}

diag.buffer(errors_buffer);
diag
}

/// Reports a region inference error for the general case with named/synthesized lifetimes to
Expand All @@ -538,41 +547,37 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// ```
fn report_general_error(
&self,
body: &Body<'tcx>,
upvars: &[Upvar],
infcx: &InferCtxt<'_, 'tcx>,
mir_def_id: DefId,
fr: RegionVid,
fr_is_local: bool,
outlived_fr: RegionVid,
outlived_fr_is_local: bool,
category: ConstraintCategory,
span: Span,
errors_buffer: &mut Vec<Diagnostic>,
) {
errctx: &ErrorReportingCtx<'_, '_, 'tcx>,
errci: &ErrorConstraintInfo,
renctx: &mut RegionErrorNamingCtx,
) -> DiagnosticBuilder<'_> {
let ErrorReportingCtx {
infcx, mir_def_id, ..
} = errctx;
let ErrorConstraintInfo {
fr, fr_is_local, outlived_fr, outlived_fr_is_local, span, category, ..
} = errci;

let mut diag = infcx.tcx.sess.struct_span_err(
span,
*span,
"lifetime may not live long enough"
);

let counter = &mut 1;
let fr_name = self.give_region_a_name(
infcx, body, upvars, mir_def_id, fr, counter).unwrap();
fr_name.highlight_region_name(&mut diag);
let outlived_fr_name =
self.give_region_a_name(infcx, body, upvars, mir_def_id, outlived_fr, counter).unwrap();
outlived_fr_name.highlight_region_name(&mut diag);

let mir_def_name = if infcx.tcx.is_closure(mir_def_id) {
let mir_def_name = if infcx.tcx.is_closure(*mir_def_id) {
"closure"
} else {
"function"
};

let fr_name = self.give_region_a_name(errctx, renctx, *fr).unwrap();
fr_name.highlight_region_name(&mut diag);
let outlived_fr_name = self.give_region_a_name(errctx, renctx, *outlived_fr).unwrap();
outlived_fr_name.highlight_region_name(&mut diag);

match (category, outlived_fr_is_local, fr_is_local) {
(ConstraintCategory::Return, true, _) => {
diag.span_label(
span,
*span,
format!(
"{} was supposed to return data with lifetime `{}` but it is returning \
data with lifetime `{}`",
Expand All @@ -582,7 +587,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
_ => {
diag.span_label(
span,
*span,
format!(
"{}requires that `{}` must outlive `{}`",
category.description(),
Expand All @@ -593,9 +598,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
}

self.add_static_impl_trait_suggestion(infcx, &mut diag, fr, fr_name, outlived_fr);
self.add_static_impl_trait_suggestion(infcx, &mut diag, *fr, fr_name, *outlived_fr);

diag.buffer(errors_buffer);
diag
}

/// Adds a suggestion to errors where a `impl Trait` is returned.
Expand Down Expand Up @@ -704,8 +709,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
borrow_region,
|r| self.provides_universal_region(r, borrow_region, outlived_region)
);
let outlived_fr_name =
self.give_region_a_name(infcx, body, upvars, mir_def_id, outlived_region, &mut 1);

let mut renctx = RegionErrorNamingCtx::new();
let errctx = ErrorReportingCtx {
infcx, body, upvars, mir_def_id,
rinfcx: self,
};
let outlived_fr_name = self.give_region_a_name(&errctx, &mut renctx, outlived_region);

(category, from_closure, span, outlived_fr_name)
}

Expand Down

0 comments on commit 68b1a87

Please sign in to comment.