Skip to content

Commit

Permalink
Auto merge of #52572 - davidtwco:issue-51027, r=nikomatsakis
Browse files Browse the repository at this point in the history 
NLL diagnostics replaced nice closure errors w/ indecipherable free region errors

Fixes #51027.

r? @nikomatsakis
  • Loading branch information
@bors
bors committed Jul 22, 2018
2 parents d3b3bc5 + c64db00 commit 32772fd
Show file tree
Hide file tree
Showing 19 changed files with 367 additions and 157 deletions.
33 changes: 33 additions & 0 deletions src/librustc/mir/tcx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,39 @@ impl<'tcx> Place<'tcx> {
proj.base.ty(local_decls, tcx).projection_ty(tcx, &proj.elem),
}
}

/// If this is a field projection, and the field is being projected from a closure type,
/// then returns the index of the field being projected. Note that this closure will always
/// be `self` in the current MIR, because that is the only time we directly access the fields
/// of a closure type.
pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>,
tcx: &TyCtxt<'cx, 'gcx, 'tcx>) -> Option<Field> {
let place = if let Place::Projection(ref proj) = self {
if let ProjectionElem::Deref = proj.elem {
&proj.base
} else {
self
}
} else {
self
};

match place {
Place::Projection(ref proj) => match proj.elem {
ProjectionElem::Field(field, _ty) => {
let base_ty = proj.base.ty(mir, *tcx).to_ty(*tcx);

if base_ty.is_closure() || base_ty.is_generator() {
Some(field)
} else {
None
}
},
_ => None,
},
_ => None,
}
}
}

pub enum RvalueInitializationState {
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_mir/borrow_check/error_reporting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Place::Projection(ref proj) => {
match proj.elem {
ProjectionElem::Deref => {
if let Some(field) = self.is_upvar_field_projection(&proj.base) {
let upvar_field_projection = place.is_upvar_field_projection(
self.mir, &self.tcx);
if let Some(field) = upvar_field_projection {
let var_index = field.index();
let name = self.mir.upvar_decls[var_index].debug_name.to_string();
if self.mir.upvar_decls[var_index].by_ref {
Expand Down Expand Up @@ -785,7 +787,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
ProjectionElem::Field(field, _ty) => {
autoderef = true;

if let Some(field) = self.is_upvar_field_projection(place) {
let upvar_field_projection = place.is_upvar_field_projection(
self.mir, &self.tcx);
if let Some(field) = upvar_field_projection {
let var_index = field.index();
let name = self.mir.upvar_decls[var_index].debug_name.to_string();
buf.push_str(&name);
Expand Down
34 changes: 8 additions & 26 deletions src/librustc_mir/borrow_check/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
Operand::Move(ref place @ Place::Projection(_))
| Operand::Copy(ref place @ Place::Projection(_)) => {
if let Some(field) = self.is_upvar_field_projection(place) {
if let Some(field) = place.is_upvar_field_projection(
self.mir, &self.tcx) {
self.used_mut_upvars.push(field);
}
}
Expand Down Expand Up @@ -1803,7 +1804,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
place: place @ Place::Projection(_),
is_local_mutation_allowed: _,
} => {
if let Some(field) = self.is_upvar_field_projection(&place) {
if let Some(field) = place.is_upvar_field_projection(self.mir, &self.tcx) {
self.used_mut_upvars.push(field);
}
}
Expand Down Expand Up @@ -1866,7 +1867,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// Mutably borrowed data is mutable, but only if we have a
// unique path to the `&mut`
hir::MutMutable => {
let mode = match self.is_upvar_field_projection(&proj.base)
let mode = match place.is_upvar_field_projection(
self.mir, &self.tcx)
{
Some(field)
if {
Expand Down Expand Up @@ -1911,7 +1913,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Downcast(..) => {
if let Some(field) = self.is_upvar_field_projection(place) {
let upvar_field_projection = place.is_upvar_field_projection(
self.mir, &self.tcx);
if let Some(field) = upvar_field_projection {
let decl = &self.mir.upvar_decls[field.index()];
debug!(
"decl.mutability={:?} local_mutation_is_allowed={:?} place={:?}",
Expand Down Expand Up @@ -1965,28 +1969,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}
}

/// If this is a field projection, and the field is being projected from a closure type,
/// then returns the index of the field being projected. Note that this closure will always
/// be `self` in the current MIR, because that is the only time we directly access the fields
/// of a closure type.
fn is_upvar_field_projection(&self, place: &Place<'tcx>) -> Option<Field> {
match *place {
Place::Projection(ref proj) => match proj.elem {
ProjectionElem::Field(field, _ty) => {
let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);

if base_ty.is_closure() || base_ty.is_generator() {
Some(field)
} else {
None
}
}
_ => None,
},
_ => None,
}
}
}

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
Expand Down
126 changes: 104 additions & 22 deletions src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use std::fmt;
use syntax_pos::Span;

mod region_name;
mod var_name;

/// Constraints that are considered interesting can be categorized to
/// determine why they are interesting. Order of variants indicates
Expand All @@ -30,7 +31,9 @@ mod region_name;
enum ConstraintCategory {
Cast,
Assignment,
AssignmentToUpvar,
Return,
CallArgumentToUpvar,
CallArgument,
Other,
Boring,
Expand All @@ -39,10 +42,12 @@ enum ConstraintCategory {
impl fmt::Display for ConstraintCategory {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
ConstraintCategory::Assignment => write!(f, "assignment"),
ConstraintCategory::Assignment |
ConstraintCategory::AssignmentToUpvar => write!(f, "assignment"),
ConstraintCategory::Return => write!(f, "return"),
ConstraintCategory::Cast => write!(f, "cast"),
ConstraintCategory::CallArgument => write!(f, "argument"),
ConstraintCategory::CallArgument |
ConstraintCategory::CallArgumentToUpvar => write!(f, "argument"),
_ => write!(f, "free region"),
}
}
Expand Down Expand Up @@ -130,8 +135,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
&self,
index: ConstraintIndex,
mir: &Mir<'tcx>,
_infcx: &InferCtxt<'_, '_, 'tcx>,
) -> (ConstraintCategory, Span) {
let constraint = self.constraints[index];
debug!("classify_constraint: constraint={:?}", constraint);
let span = constraint.locations.span(mir);
let location = constraint.locations.from_location().unwrap_or(Location::START);

Expand All @@ -140,8 +147,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}

let data = &mir[location.block];
debug!("classify_constraint: location={:?} data={:?}", location, data);
let category = if location.statement_index == data.statements.len() {
if let Some(ref terminator) = data.terminator {
debug!("classify_constraint: terminator.kind={:?}", terminator.kind);
match terminator.kind {
TerminatorKind::DropAndReplace { .. } => ConstraintCategory::Assignment,
TerminatorKind::Call { .. } => ConstraintCategory::CallArgument,
Expand All @@ -152,14 +161,17 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
} else {
let statement = &data.statements[location.statement_index];
debug!("classify_constraint: statement.kind={:?}", statement.kind);
match statement.kind {
StatementKind::Assign(ref place, ref rvalue) => {
debug!("classify_constraint: place={:?} rvalue={:?}", place, rvalue);
if *place == Place::Local(mir::RETURN_PLACE) {
ConstraintCategory::Return
} else {
match rvalue {
Rvalue::Cast(..) => ConstraintCategory::Cast,
Rvalue::Use(..) => ConstraintCategory::Assignment,
Rvalue::Use(..) |
Rvalue::Aggregate(..) => ConstraintCategory::Assignment,
_ => ConstraintCategory::Other,
}
}
Expand Down Expand Up @@ -208,7 +220,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {

// Classify each of the constraints along the path.
let mut categorized_path: Vec<(ConstraintCategory, Span)> = path.iter()
.map(|&index| self.classify_constraint(index, mir))
.map(|&index| self.classify_constraint(index, mir, infcx))
.collect();
debug!("report_error: categorized_path={:?}", categorized_path);

Expand All @@ -218,30 +230,100 @@ impl<'tcx> RegionInferenceContext<'tcx> {

// Get a span
let (category, span) = categorized_path.first().unwrap();

let category = match (
category,
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: category={:?}", category);
match category {
ConstraintCategory::AssignmentToUpvar |
ConstraintCategory::CallArgumentToUpvar =>
self.report_closure_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),
}
}

fn report_closure_error(
&self,
mir: &Mir<'tcx>,
infcx: &InferCtxt<'_, '_, 'tcx>,
mir_def_id: DefId,
fr: RegionVid,
outlived_fr: RegionVid,
category: &ConstraintCategory,
span: &Span,
) {
let fr_name_and_span = self.get_var_name_and_span_for_region(
infcx.tcx, mir, fr);
let outlived_fr_name_and_span = self.get_var_name_and_span_for_region(
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);
}

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

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),
);
}
}

if let Some((fr_name, fr_span)) = fr_name_and_span {
if let Some(name) = fr_name {
diag.span_label(
fr_span,
format!("`{}` is a reference that is only valid in the closure body", name),
);

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

diag.emit();
}

fn report_general_error(
&self,
mir: &Mir<'tcx>,
infcx: &InferCtxt<'_, '_, 'tcx>,
mir_def_id: DefId,
fr: RegionVid,
outlived_fr: RegionVid,
category: &ConstraintCategory,
span: &Span,
) {
let diag = &mut infcx.tcx.sess.struct_span_err(
*span,
&format!("unsatisfied lifetime constraints"), // FIXME
*span, &format!("unsatisfied lifetime constraints"), // FIXME
);

// Figure out how we can refer
let counter = &mut 1;
let fr_name = self.give_region_a_name(infcx.tcx, mir, mir_def_id, fr, counter, diag);
let fr_name = self.give_region_a_name(
infcx.tcx, mir, mir_def_id, fr, counter, 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, diag);

diag.span_label(
*span,
format!(
"{} requires that `{}` must outlive `{}`",
category, fr_name, outlived_fr_name,
),
);
diag.span_label(*span, format!(
"{} requires that `{}` must outlive `{}`",
category, fr_name, outlived_fr_name,
));

diag.emit();
}
Expand Down
Loading

0 comments on commit 32772fd

Please sign in to comment.