Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Various refactorings to clean up nll diagnostics #64416

Merged
merged 4 commits into from
Sep 17, 2019
Merged

Various refactorings to clean up nll diagnostics #64416

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
68b1a87
Various refactorings to clean up nll diagnostics
mark-i-m Sep 12, 2019
23db450
minor fixes
mark-i-m Sep 12, 2019
5b09358
update tests
mark-i-m Sep 12, 2019
2a774b1
address Centril's comments
mark-i-m Sep 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
242 changes: 127 additions & 115 deletions src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
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,31 @@ enum Trace {
NotVisited,
}

/// Various pieces of state used when reporting borrow checker errors.
pub struct ErrorReportingCtx<'a, 'b, 'tcx> {
#[allow(dead_code)] // FIXME(mark-i-m): used by outlives suggestions
rinfcx: &'b RegionInferenceContext<'tcx>,
mark-i-m marked this conversation as resolved.
Show resolved Hide resolved
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 {
mark-i-m marked this conversation as resolved.
Show resolved Hide resolved
// 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 +282,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 +304,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 +317,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 db = self.report_escaping_data_error(&errctx, &errci, renctx);
mark-i-m marked this conversation as resolved.
Show resolved Hide resolved

db
}
_ => {
let 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 +393,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 +417,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 +439,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 +456,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 +483,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 +523,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 +548,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 +588,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
_ => {
diag.span_label(
span,
*span,
format!(
"{}requires that `{}` must outlive `{}`",
category.description(),
Expand All @@ -593,9 +599,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 +710,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
Loading