remove vestiges of the old suggestion machinery

Author: nikomatsakis

src/librustc/infer/error_reporting.rs

@@ -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;
@@ -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> {
@@ -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());
 
@@ -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);
-                    }
-                }
             }
         }
     }
@@ -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()
         }
     }
 
@@ -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 {

src/librustc/infer/region_inference/mod.rs

@@ -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};
 
@@ -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)]
@@ -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> {

src/test/compile-fail/issue-17728.rs

@@ -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,
@@ -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
     }
 }