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.

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

[NLL] Record more infomation on free region constraints in typeck #54262

Merged
merged 5 commits into from
Sep 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/librustc/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}

fn constrain_opaque_type<FRR: FreeRegionRelations<'tcx>>(
pub fn constrain_opaque_type<FRR: FreeRegionRelations<'tcx>>(
&self,
def_id: DefId,
opaque_defn: &OpaqueTypeDecl<'tcx>,
Expand Down
7 changes: 5 additions & 2 deletions src/librustc_mir/borrow_check/nll/constraints/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
// except according to those terms.

use borrow_check::nll::type_check::Locations;
use borrow_check::nll::constraints::{ConstraintIndex, ConstraintSet, OutlivesConstraint};
use borrow_check::nll::constraints::{ConstraintCategory, ConstraintIndex};
use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint};
use rustc::ty::RegionVid;
use rustc_data_structures::graph;
use rustc_data_structures::indexed_vec::IndexVec;
use syntax_pos::DUMMY_SP;

/// The construct graph organizes the constraints by their end-points.
/// It can be used to view a `R1: R2` constraint as either an edge `R1
Expand Down Expand Up @@ -174,7 +176,8 @@ impl<'s, D: ConstraintGraphDirecton> Iterator for Edges<'s, D> {
Some(OutlivesConstraint {
sup: self.static_region,
sub: next_static_idx.into(),
locations: Locations::All,
locations: Locations::All(DUMMY_SP),
category: ConstraintCategory::Internal,
})
} else {
None
Expand Down
39 changes: 39 additions & 0 deletions src/librustc_mir/borrow_check/nll/constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,42 @@ crate struct ConstraintSet {
constraints: IndexVec<ConstraintIndex, OutlivesConstraint>,
}

/// Constraints can be categorized to determine whether and why they are
/// interesting. Order of variants indicates sort order of the category,
/// thereby influencing diagnostic output.
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
pub enum ConstraintCategory {
Return,
TypeAnnotation,
Cast,
CallArgument,

/// A constraint that came from checking the body of a closure.
///
/// Ideally we would give an explanation that points to the relevant part
/// of the closure's body.
ClosureBounds,
CopyBound,
SizedBound,
Assignment,
OpaqueType,

/// A "boring" constraint (caused by the given location) is one that
/// the user probably doesn't want to see described in diagnostics,
/// because it is kind of an artifact of the type system setup.
/// Example: `x = Foo { field: y }` technically creates
/// intermediate regions representing the "type of `Foo { field: y
/// }`", and data flows from `y` into those variables, but they
/// are not very interesting. The assignment into `x` on the other
/// hand might be.
Boring,
// Boring and applicable everywhere.
BoringNoLocation,

/// A constraint that doesn't correspond to anything the user sees.
Internal,
}

impl ConstraintSet {
crate fn push(&mut self, constraint: OutlivesConstraint) {
debug!(
Expand Down Expand Up @@ -87,6 +123,9 @@ pub struct OutlivesConstraint {

/// Where did this constraint arise?
pub locations: Locations,

/// What caused this constraint?
pub category: ConstraintCategory,
}

impl fmt::Debug for OutlivesConstraint {
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
sup,
sub,
locations,
category,
} = constraint;
with_msg(&format!(
"{:?}: {:?} due to {:?}",
"{:?}: {:?} due to {:?} at {:?}",
sup,
sub,
category,
locations,
))?;
}
Expand Down
152 changes: 22 additions & 130 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 @@ -8,14 +8,13 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use borrow_check::nll::constraints::OutlivesConstraint;
use borrow_check::nll::constraints::{OutlivesConstraint, ConstraintCategory};
use borrow_check::nll::region_infer::RegionInferenceContext;
use borrow_check::nll::type_check::Locations;
use rustc::hir::def_id::DefId;
use rustc::infer::error_reporting::nice_region_error::NiceRegionError;
use rustc::infer::InferCtxt;
use rustc::mir::{self, Location, Mir, Place, Rvalue, StatementKind, TerminatorKind};
use rustc::ty::{self, TyCtxt, RegionVid};
use rustc::mir::{Location, Mir};
use rustc::ty::{self, RegionVid};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_errors::{Diagnostic, DiagnosticBuilder};
use std::collections::VecDeque;
Expand All @@ -28,19 +27,6 @@ mod var_name;

use self::region_name::RegionName;

/// Constraints that are considered interesting can be categorized to
/// determine why they are interesting. Order of variants indicates
/// sort order of the category, thereby influencing diagnostic output.
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
enum ConstraintCategory {
Cast,
Assignment,
Return,
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.
Expand All @@ -49,7 +35,14 @@ impl fmt::Display for ConstraintCategory {
ConstraintCategory::Return => write!(f, "returning this value "),
ConstraintCategory::Cast => write!(f, "cast "),
ConstraintCategory::CallArgument => write!(f, "argument "),
_ => write!(f, ""),
ConstraintCategory::TypeAnnotation => write!(f, "type annotation "),
ConstraintCategory::ClosureBounds => write!(f, "closure body "),
ConstraintCategory::SizedBound => write!(f, "proving this value is `Sized` "),
ConstraintCategory::CopyBound => write!(f, "copying this value "),
ConstraintCategory::OpaqueType => write!(f, "opaque type "),
ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal => write!(f, ""),
}
}
}
Expand All @@ -71,7 +64,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
fn best_blame_constraint(
&self,
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, '_, 'tcx>,
from_region: RegionVid,
target_test: impl Fn(RegionVid) -> bool,
) -> (ConstraintCategory, Span, RegionVid) {
Expand All @@ -96,7 +88,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, tcx))
.map(|constraint| (constraint.category, constraint.locations.span(mir)))
.collect();
debug!(
"best_blame_constraint: categorized_path={:#?}",
Expand Down Expand Up @@ -129,12 +121,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup);

match categorized_path[i].0 {
ConstraintCategory::Boring => false,
ConstraintCategory::Other => {
// other isn't interesting when the two lifetimes
// are unified.
constraint_sup_scc != self.constraint_sccs.scc(constraint.sub)
}
ConstraintCategory::OpaqueType
| ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal => false,
_ => constraint_sup_scc != target_scc,
}
});
Expand Down Expand Up @@ -220,106 +210,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
None
}

/// This function will return true if a constraint is interesting and false if a constraint
/// is not. It is useful in filtering constraint paths to only interesting points.
fn constraint_is_interesting(&self, constraint: OutlivesConstraint) -> bool {
debug!(
"constraint_is_interesting: locations={:?} constraint={:?}",
constraint.locations, constraint
);

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

/// This function classifies a constraint from a location.
fn classify_constraint(
&self,
constraint: OutlivesConstraint,
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, '_, 'tcx>,
) -> (ConstraintCategory, Span) {
debug!("classify_constraint: constraint={:?}", constraint);
let span = constraint.locations.span(mir);
let location = constraint
.locations
.from_location()
.unwrap_or(Location::START);

if !self.constraint_is_interesting(constraint) {
return (ConstraintCategory::Boring, span);
}

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,
// Classify calls differently depending on whether or not
// the sub region appears in the destination type (so the
// sup region is in the return type). If the return type
// contains the sub-region, then this is either an
// assignment or a return, depending on whether we are
// writing to the RETURN_PLACE or not.
//
// The idea here is that the region is being propagated
// from an input into the output place, so it's a kind of
// assignment. Otherwise, if the sub-region only appears in
// the argument types, then use the CallArgument
// classification.
TerminatorKind::Call { destination: Some((ref place, _)), .. } => {
if tcx.any_free_region_meets(
&place.ty(mir, tcx).to_ty(tcx),
|region| self.to_region_vid(region) == constraint.sub,
) {
match place {
Place::Local(mir::RETURN_PLACE) => ConstraintCategory::Return,
_ => ConstraintCategory::Assignment,
}
} else {
ConstraintCategory::CallArgument
}
}
TerminatorKind::Call { destination: None, .. } => {
ConstraintCategory::CallArgument
}
_ => ConstraintCategory::Other,
}
} else {
ConstraintCategory::Other
}
} 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(..) | Rvalue::Aggregate(..) => {
ConstraintCategory::Assignment
}
_ => ConstraintCategory::Other,
}
}
}
_ => ConstraintCategory::Other,
}
};

(category, span)
}

/// Report an error because the universal region `fr` was required to outlive
/// `outlived_fr` but it is not known to do so. For example:
///
Expand All @@ -341,7 +231,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {

let (category, span, _) = self.best_blame_constraint(
mir,
infcx.tcx,
fr,
|r| r == outlived_fr
);
Expand Down Expand Up @@ -391,7 +280,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {

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() {
// 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")
{
return self.report_general_error(mir, infcx, mir_def_id,
fr, true, outlived_fr, false,
category, span, errors_buffer);
Expand Down Expand Up @@ -570,11 +463,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
crate fn find_outlives_blame_span(
&self,
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, '_, 'tcx>,
fr1: RegionVid,
fr2: RegionVid,
) -> Span {
let (_, span, _) = self.best_blame_constraint(mir, tcx, fr1, |r| r == fr2);
let (_, span, _) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
span
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -452,16 +452,23 @@ impl<'tcx> RegionInferenceContext<'tcx> {
ty::Adt(_adt_def, substs),
hir::TyKind::Path(hir::QPath::Resolved(None, path)),
) => {
if let Some(last_segment) = path.segments.last() {
if let Some(name) = self.match_adt_and_segment(
substs,
needle_fr,
last_segment,
counter,
diag,
search_stack,
) {
return Some(name);
match path.def {
// Type parameters of the type alias have no reason to
// be the same as those of the ADT.
// FIXME: We should be able to do something similar to
// match_adt_and_segment in this case.
hir::def::Def::TyAlias(_) => (),
_ => if let Some(last_segment) = path.segments.last() {
if let Some(name) = self.match_adt_and_segment(
substs,
needle_fr,
last_segment,
counter,
diag,
search_stack,
) {
return Some(name);
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/nll/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
longer_fr, shorter_fr,
);

let blame_span = self.find_outlives_blame_span(mir, infcx.tcx, longer_fr, shorter_fr);
let blame_span = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);

if let Some(propagated_outlives_requirements) = propagated_outlives_requirements {
// Shrink `fr` until we find a non-local region (if we do).
Expand Down Expand Up @@ -1147,7 +1147,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
};

// Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
let span = self.find_outlives_blame_span(mir, infcx.tcx, longer_fr, error_region);
let span = self.find_outlives_blame_span(mir, longer_fr, error_region);

// Obviously, this error message is far from satisfactory.
// At present, though, it only appears in unit tests --
Expand Down
Loading