Skip to content

Commit

Permalink
remove vestiges of the old suggestion machinery
Browse files Browse the repository at this point in the history
  • Loading branch information
@nikomatsakis @brson
nikomatsakis authored and brson committed Feb 23, 2017
1 parent 6f405bc commit 3787d33
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 260 deletions.
246 changes: 25 additions & 221 deletions src/librustc/infer/error_reporting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ use super::region_inference::ConcreteFailure;
use super::region_inference::SubSupConflict;
use super::region_inference::GenericBoundFailure;
use super::region_inference::GenericKind;
use super::region_inference::ProcessedErrors;
use super::region_inference::ProcessedErrorOrigin;
use super::region_inference::SameRegions;

use hir::map as hir_map;
use hir;
Expand All @@ -78,12 +75,12 @@ use infer;
use middle::region;
use traits::{ObligationCause, ObligationCauseCode};
use ty::{self, TyCtxt, TypeFoldable};
use ty::{Region, ReFree};
use ty::Region;
use ty::error::TypeError;

use std::fmt;
use syntax::ast;
use syntax_pos::{Pos, Span};
use syntax::ast;
use errors::DiagnosticBuilder;

impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -256,8 +253,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {

// try to pre-process the errors, which will group some of them
// together into a `ProcessedErrors` group:
let processed_errors = self.process_errors(errors);
let errors = processed_errors.as_ref().unwrap_or(errors);
let errors = self.process_errors(errors);

debug!("report_region_errors: {} errors after preprocessing", errors.len());

Expand All @@ -279,13 +275,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
sub_origin, sub_r,
sup_origin, sup_r);
}

ProcessedErrors(ref origins,
ref same_regions) => {
if !same_regions.is_empty() {
self.report_processed_errors(origins);
}
}
}
}
}
Expand All @@ -301,202 +290,31 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
// duplicates that will be unhelpful to the end-user. But
// obviously it never weeds out ALL errors.
fn process_errors(&self, errors: &Vec<RegionResolutionError<'tcx>>)
-> Option<Vec<RegionResolutionError<'tcx>>> {
-> Vec<RegionResolutionError<'tcx>> {
debug!("process_errors()");
let mut origins = Vec::new();

// we collect up ConcreteFailures and SubSupConflicts that are
// relating free-regions bound on the fn-header and group them
// together into this vector
let mut same_regions = Vec::new();

// here we put errors that we will not be able to process nicely
let mut other_errors = Vec::new();

// we collect up GenericBoundFailures in here.
let mut bound_failures = Vec::new();

for error in errors {
// Check whether we can process this error into some other
// form; if not, fall through.
match *error {
ConcreteFailure(ref origin, sub, sup) => {
debug!("processing ConcreteFailure");
if let SubregionOrigin::CompareImplMethodObligation { .. } = *origin {
// When comparing an impl method against a
// trait method, it is not helpful to suggest
// changes to the impl method. This is
// because the impl method signature is being
// checked using the trait's environment, so
// usually the changes we suggest would
// actually have to be applied to the *trait*
// method (and it's not clear that the trait
// method is even under the user's control).
} else if let Some(same_frs) = free_regions_from_same_fn(self.tcx, sub, sup) {
origins.push(
ProcessedErrorOrigin::ConcreteFailure(
origin.clone(),
sub,
sup));
append_to_same_regions(&mut same_regions, &same_frs);
continue;
}
}
SubSupConflict(ref var_origin, ref sub_origin, sub, ref sup_origin, sup) => {
debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub, sup);
match (sub_origin, sup_origin) {
(&SubregionOrigin::CompareImplMethodObligation { .. }, _) => {
// As above, when comparing an impl method
// against a trait method, it is not helpful
// to suggest changes to the impl method.
}
(_, &SubregionOrigin::CompareImplMethodObligation { .. }) => {
// See above.
}
_ => {
if let Some(same_frs) = free_regions_from_same_fn(self.tcx, sub, sup) {
origins.push(
ProcessedErrorOrigin::VariableFailure(
var_origin.clone()));
append_to_same_regions(&mut same_regions, &same_frs);
continue;
}
}
}
}
GenericBoundFailure(ref origin, ref kind, region) => {
bound_failures.push((origin.clone(), kind.clone(), region));
continue;
}
ProcessedErrors(..) => {
bug!("should not encounter a `ProcessedErrors` yet: {:?}", error)
}
}

// No changes to this error.
other_errors.push(error.clone());
}

// ok, let's pull together the errors, sorted in an order that
// we think will help user the best
let mut processed_errors = vec![];

// first, put the processed errors, if any
if !same_regions.is_empty() {
let common_scope_id = same_regions[0].scope_id;
for sr in &same_regions {
// Since ProcessedErrors is used to reconstruct the function
// declaration, we want to make sure that they are, in fact,
// from the same scope
if sr.scope_id != common_scope_id {
debug!("returning empty result from process_errors because
{} != {}", sr.scope_id, common_scope_id);
return None;
}
}
assert!(origins.len() > 0);
let pe = ProcessedErrors(origins, same_regions);
debug!("errors processed: {:?}", pe);
processed_errors.push(pe);
}

// next, put the other misc errors
processed_errors.extend(other_errors);

// finally, put the `T: 'a` errors, but only if there were no
// other errors. otherwise, these have a very high rate of
// being unhelpful in practice. This is because they are
// basically secondary checks that test the state of the
// region graph after the rest of inference is done, and the
// other kinds of errors indicate that the region constraint
// graph is internally inconsistent, so these test results are
// likely to be meaningless.
if processed_errors.is_empty() {
for (origin, kind, region) in bound_failures {
processed_errors.push(GenericBoundFailure(origin, kind, region));
}
}

// we should always wind up with SOME errors, unless there were no
// errors to start
assert!(if errors.len() > 0 {processed_errors.len() > 0} else {true});

return Some(processed_errors);

#[derive(Debug)]
struct FreeRegionsFromSameFn {
sub_fr: ty::FreeRegion,
sup_fr: ty::FreeRegion,
scope_id: ast::NodeId
}

impl FreeRegionsFromSameFn {
fn new(sub_fr: ty::FreeRegion,
sup_fr: ty::FreeRegion,
scope_id: ast::NodeId)
-> FreeRegionsFromSameFn {
FreeRegionsFromSameFn {
sub_fr: sub_fr,
sup_fr: sup_fr,
scope_id: scope_id
}
}
}

fn free_regions_from_same_fn<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
sub: &'tcx Region,
sup: &'tcx Region)
-> Option<FreeRegionsFromSameFn> {
debug!("free_regions_from_same_fn(sub={:?}, sup={:?})", sub, sup);
let (scope_id, fr1, fr2) = match (sub, sup) {
(&ReFree(fr1), &ReFree(fr2)) => {
if fr1.scope != fr2.scope {
return None
}
assert!(fr1.scope == fr2.scope);
(fr1.scope.node_id(&tcx.region_maps), fr1, fr2)
},
_ => return None
};
let parent = tcx.hir.get_parent(scope_id);
let parent_node = tcx.hir.find(parent);
match parent_node {
Some(node) => match node {
hir_map::NodeItem(item) => match item.node {
hir::ItemFn(..) => {
Some(FreeRegionsFromSameFn::new(fr1, fr2, scope_id))
},
_ => None
},
hir_map::NodeImplItem(..) |
hir_map::NodeTraitItem(..) => {
Some(FreeRegionsFromSameFn::new(fr1, fr2, scope_id))
},
_ => None
},
None => {
debug!("no parent node of scope_id {}", scope_id);
None
}
}
}
// We want to avoid reporting generic-bound failures if we can
// avoid it: these have a very high rate of being unhelpful in
// practice. This is because they are basically secondary
// checks that test the state of the region graph after the
// rest of inference is done, and the other kinds of errors
// indicate that the region constraint graph is internally
// inconsistent, so these test results are likely to be
// meaningless.
//
// Therefore, we filter them out of the list unless they are
// the only thing in the list.

let is_bound_failure = |e: &RegionResolutionError<'tcx>| match *e {
ConcreteFailure(..) => false,
SubSupConflict(..) => false,
GenericBoundFailure(..) => true,
};

fn append_to_same_regions(same_regions: &mut Vec<SameRegions>,
same_frs: &FreeRegionsFromSameFn) {
debug!("append_to_same_regions(same_regions={:?}, same_frs={:?})",
same_regions, same_frs);
let scope_id = same_frs.scope_id;
let (sub_fr, sup_fr) = (same_frs.sub_fr, same_frs.sup_fr);
for sr in same_regions.iter_mut() {
if sr.contains(&sup_fr.bound_region) && scope_id == sr.scope_id {
sr.push(sub_fr.bound_region);
return
}
}
same_regions.push(SameRegions {
scope_id: scope_id,
regions: vec![sub_fr.bound_region, sup_fr.bound_region]
})
if errors.iter().all(|e| is_bound_failure(e)) {
errors.clone()
} else {
errors.iter().filter(|&e| !is_bound_failure(e)).cloned().collect()
}
}

Expand Down Expand Up @@ -1040,20 +858,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
err.emit();
}

fn report_processed_errors(&self,
origins: &[ProcessedErrorOrigin<'tcx>]) {
for origin in origins.iter() {
let mut err = match *origin {
ProcessedErrorOrigin::VariableFailure(ref var_origin) =>
self.report_inference_failure(var_origin.clone()),
ProcessedErrorOrigin::ConcreteFailure(ref sr_origin, sub, sup) =>
self.report_concrete_failure(sr_origin.clone(), sub, sup),
};

err.emit();
}
}

pub fn issue_32330_warnings(&self, span: Span, issue32330s: &[ty::Issue32330]) {
for issue32330 in issue32330s {
match *issue32330 {
Expand Down
36 changes: 1 addition & 35 deletions src/librustc/infer/region_inference/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use rustc_data_structures::graph::{self, Direction, NodeIndex, OUTGOING};
use rustc_data_structures::unify::{self, UnificationTable};
use middle::free_region::FreeRegionMap;
use ty::{self, Ty, TyCtxt};
use ty::{BoundRegion, Region, RegionVid};
use ty::{Region, RegionVid};
use ty::{ReEmpty, ReStatic, ReFree, ReEarlyBound, ReErased};
use ty::{ReLateBound, ReScope, ReVar, ReSkolemized, BrFresh};

Expand Down Expand Up @@ -171,13 +171,6 @@ pub enum RegionResolutionError<'tcx> {
&'tcx Region,
SubregionOrigin<'tcx>,
&'tcx Region),

/// For subsets of `ConcreteFailure` and `SubSupConflict`, we can derive
/// more specific errors message by suggesting to the user where they
/// should put a lifetime. In those cases we process and put those errors
/// into `ProcessedErrors` before we do any reporting.
ProcessedErrors(Vec<ProcessedErrorOrigin<'tcx>>,
Vec<SameRegions>),
}

#[derive(Clone, Debug)]
Expand All @@ -186,33 +179,6 @@ pub enum ProcessedErrorOrigin<'tcx> {
VariableFailure(RegionVariableOrigin),
}

/// SameRegions is used to group regions that we think are the same and would
/// like to indicate so to the user.
/// For example, the following function
/// ```
/// struct Foo { bar: i32 }
/// fn foo2<'a, 'b>(x: &'a Foo) -> &'b i32 {
/// &x.bar
/// }
/// ```
/// would report an error because we expect 'a and 'b to match, and so we group
/// 'a and 'b together inside a SameRegions struct
#[derive(Clone, Debug)]
pub struct SameRegions {
pub scope_id: ast::NodeId,
pub regions: Vec<BoundRegion>,
}

impl SameRegions {
pub fn contains(&self, other: &BoundRegion) -> bool {
self.regions.contains(other)
}

pub fn push(&mut self, other: BoundRegion) {
self.regions.push(other);
}
}

pub type CombineMap<'tcx> = FxHashMap<TwoRegions<'tcx>, RegionVid>;

pub struct RegionVarBindings<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
Expand Down
5 changes: 1 addition & 4 deletions src/test/compile-fail/issue-17728.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ impl Debug for Player {

fn str_to_direction(to_parse: &str) -> RoomDirection {
match to_parse { //~ ERROR match arms have incompatible types
//~^ expected enum `RoomDirection`, found enum `std::option::Option`
//~| expected type `RoomDirection`
//~| found type `std::option::Option<_>`
"w" | "west" => RoomDirection::West,
"e" | "east" => RoomDirection::East,
"n" | "north" => RoomDirection::North,
Expand All @@ -119,7 +116,7 @@ fn str_to_direction(to_parse: &str) -> RoomDirection {
"out" => RoomDirection::Out,
"up" => RoomDirection::Up,
"down" => RoomDirection::Down,
_ => None //~ NOTE match arm with an incompatible type
_ => None
}
}

Expand Down
Loading

0 comments on commit 3787d33

Please sign in to comment.