Skip to content

[nll] improve the "fully elaborated type" case in region errors #52648

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.

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

Merged
merged 8 commits into from
Jul 27, 2018
2 changes: 1 addition & 1 deletion src/librustc/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindLocalByTypeVisitor<'a, 'gcx, 'tcx> {


impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
fn extract_type_name(&self, ty: &'a Ty<'tcx>) -> String {
pub fn extract_type_name(&self, ty: &'a Ty<'tcx>) -> String {
if let ty::TyInfer(ty::TyVar(ty_vid)) = (*ty).sty {
let ty_vars = self.type_variables.borrow();
if let TypeVariableOrigin::TypeParameterDefinition(_, name) =
Expand Down
40 changes: 35 additions & 5 deletions src/librustc/util/ppaux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use ty::{TyError, TyStr, TyArray, TySlice, TyFloat, TyFnDef, TyFnPtr};
use ty::{TyParam, TyRawPtr, TyRef, TyNever, TyTuple};
use ty::{TyClosure, TyGenerator, TyGeneratorWitness, TyForeign, TyProjection, TyAnon};
use ty::{TyDynamic, TyInt, TyUint, TyInfer};
use ty::{self, Ty, TyCtxt, TypeFoldable, GenericParamCount, GenericParamDefKind};
use ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable, GenericParamCount, GenericParamDefKind};
use util::nodemap::FxHashSet;

use std::cell::Cell;
Expand All @@ -32,6 +32,12 @@ use syntax::ast::CRATE_NODE_ID;
use syntax::symbol::{Symbol, InternedString};
use hir;

thread_local! {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment please =)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated most recent commit to address this.

/// Mechanism for highlighting of specific regions for display in NLL region inference errors.
/// Contains region to highlight and counter for number to use when highlighting.
static HIGHLIGHT_REGION: Cell<Option<(RegionVid, usize)>> = Cell::new(None)
}

macro_rules! gen_display_debug_body {
( $with:path ) => {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
Expand Down Expand Up @@ -562,6 +568,19 @@ pub fn parameterized<F: fmt::Write>(f: &mut F,
PrintContext::new().parameterized(f, substs, did, projections)
}

fn get_highlight_region() -> Option<(RegionVid, usize)> {
HIGHLIGHT_REGION.with(|hr| hr.get())
}

pub fn with_highlight_region<R>(r: RegionVid, counter: usize, op: impl FnOnce() -> R) -> R {
HIGHLIGHT_REGION.with(|hr| {
assert_eq!(hr.get(), None);
hr.set(Some((r, counter)));
let r = op();
hr.set(None);
r
})
}

impl<'a, T: Print> Print for &'a T {
fn print<F: fmt::Write>(&self, f: &mut F, cx: &mut PrintContext) -> fmt::Result {
Expand Down Expand Up @@ -733,7 +752,7 @@ define_print! {
define_print! {
() ty::RegionKind, (self, f, cx) {
display {
if cx.is_verbose {
if cx.is_verbose || get_highlight_region().is_some() {
return self.print_debug(f, cx);
}

Expand Down Expand Up @@ -905,6 +924,15 @@ impl fmt::Debug for ty::FloatVid {

impl fmt::Debug for ty::RegionVid {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
if let Some((region, counter)) = get_highlight_region() {
debug!("RegionVid.fmt: region={:?} self={:?} counter={:?}", region, self, counter);
return if *self == region {
write!(f, "'{:?}", counter)
} else {
write!(f, "'_")
}
}

write!(f, "'_#{}r", self.index())
}
}
Expand Down Expand Up @@ -1022,9 +1050,11 @@ define_print! {
TyRef(r, ty, mutbl) => {
write!(f, "&")?;
let s = r.print_to_string(cx);
write!(f, "{}", s)?;
if !s.is_empty() {
write!(f, " ")?;
if s != "'_" {
write!(f, "{}", s)?;
if !s.is_empty() {
write!(f, " ")?;
}
}
ty::TypeAndMut { ty, mutbl }.print(f, cx)
}
Expand Down
142 changes: 62 additions & 80 deletions src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,21 @@ mod var_name;
enum ConstraintCategory {
Cast,
Assignment,
AssignmentToUpvar,
Return,
CallArgumentToUpvar,
CallArgument,
Other,
Boring,
}

impl fmt::Display for ConstraintCategory {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
// Must end with a space. Allows for empty names to be provided.
match self {
ConstraintCategory::Assignment | ConstraintCategory::AssignmentToUpvar => {
write!(f, "assignment")
}
ConstraintCategory::Return => write!(f, "return"),
ConstraintCategory::Cast => write!(f, "cast"),
ConstraintCategory::CallArgument | ConstraintCategory::CallArgumentToUpvar => {
write!(f, "argument")
}
_ => write!(f, "free region"),
ConstraintCategory::Assignment => write!(f, "assignment "),
ConstraintCategory::Return => write!(f, "return "),
ConstraintCategory::Cast => write!(f, "cast "),
ConstraintCategory::CallArgument => write!(f, "argument "),
_ => write!(f, ""),
}
}
}
Expand Down Expand Up @@ -224,10 +219,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
"constraint_is_interesting: locations={:?} constraint={:?}",
constraint.locations, constraint
);
if let Locations::Interesting(_) = constraint.locations {
true
} else {
false

match constraint.locations {
Locations::Interesting(_) | Locations::All => true,
_ => false,
}
}

Expand Down Expand Up @@ -320,45 +315,26 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
}

let category = match (
category,
let (fr_is_local, outlived_fr_is_local): (bool, bool) = (
self.universal_regions.is_local_free_region(fr),
self.universal_regions.is_local_free_region(outlived_fr),
) {
(ConstraintCategory::Assignment, true, false) => ConstraintCategory::AssignmentToUpvar,
(ConstraintCategory::CallArgument, true, false) => {
ConstraintCategory::CallArgumentToUpvar
}
(category, _, _) => category,
);
debug!("report_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}",
fr_is_local, outlived_fr_is_local, category);

match (category, fr_is_local, outlived_fr_is_local) {
(ConstraintCategory::Assignment, true, false) |
(ConstraintCategory::CallArgument, true, false) =>
self.report_escaping_data_error(mir, infcx, mir_def_id, fr, outlived_fr,
category, span, errors_buffer),
_ =>
self.report_general_error(mir, infcx, mir_def_id, fr, fr_is_local,
outlived_fr, outlived_fr_is_local,
category, span, errors_buffer),
};

debug!("report_error: category={:?}", category);
match category {
ConstraintCategory::AssignmentToUpvar | ConstraintCategory::CallArgumentToUpvar => 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,
errors_buffer,
),
}
}

fn report_closure_error(
fn report_escaping_data_error(
&self,
mir: &Mir<'tcx>,
infcx: &InferCtxt<'_, '_, 'tcx>,
Expand All @@ -373,29 +349,23 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let outlived_fr_name_and_span =
self.get_var_name_and_span_for_region(infcx.tcx, mir, outlived_fr);

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

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,
errors_buffer,
);
return self.report_general_error(mir, infcx, mir_def_id,
fr, true, outlived_fr, false,
category, span, errors_buffer);
}

let mut diag = infcx
.tcx
.sess
.struct_span_err(span, &format!("borrowed data escapes outside of closure"));
let mut diag = infcx.tcx.sess.struct_span_err(
span, &format!("borrowed data escapes outside of {}", escapes_from),
);

if let Some((outlived_fr_name, outlived_fr_span)) = outlived_fr_name_and_span {
if let Some(name) = outlived_fr_name {
diag.span_label(
outlived_fr_span,
format!("`{}` is declared here, outside of the closure body", name),
format!("`{}` is declared here, outside of the {} body", name, escapes_from),
);
}
}
Expand All @@ -404,13 +374,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
if let Some(name) = fr_name {
diag.span_label(
fr_span,
format!(
"`{}` is a reference that is only valid in the closure body",
name
),
format!("`{}` is a reference that is only valid in the {} body",
name, escapes_from),
);

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

Expand All @@ -423,7 +392,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
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>,
Expand All @@ -434,17 +405,28 @@ impl<'tcx> RegionInferenceContext<'tcx> {
);

let counter = &mut 1;
let fr_name = self.give_region_a_name(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, &mut diag);

diag.span_label(
span,
format!(
"{} requires that `{}` must outlive `{}`",
category, fr_name, outlived_fr_name,
),
);
let fr_name = self.give_region_a_name(
infcx, mir, mir_def_id, fr, counter, &mut diag);
let outlived_fr_name = self.give_region_a_name(
infcx, mir, mir_def_id, outlived_fr, counter, &mut diag);

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

match (category, outlived_fr_is_local, fr_is_local) {
(ConstraintCategory::Return, true, _) => {
diag.span_label(span, format!(
"{} was supposed to return data with lifetime `{}` but it is returning \
data with lifetime `{}`",
mir_def_name, fr_name, outlived_fr_name,
));
},
_ => {
diag.span_label(span, format!(
"{}requires that `{}` must outlive `{}`",
category, fr_name, outlived_fr_name,
));
},
}

diag.buffer(errors_buffer);
}
Expand Down
Loading