From 3787d33889cf16887cf791c0cc14466950ac14b1 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 17 Feb 2017 10:33:06 -0500 Subject: [PATCH] remove vestiges of the old suggestion machinery --- src/librustc/infer/error_reporting.rs | 246 ++---------------- src/librustc/infer/region_inference/mod.rs | 36 +-- src/test/compile-fail/issue-17728.rs | 5 +- .../ex2c-push-inference-variable.stderr | 29 +++ .../ex2d-push-inference-variable-2.stderr | 31 +++ .../ex2e-push-inference-variable-3.stderr | 31 +++ 6 files changed, 118 insertions(+), 260 deletions(-) diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs index 2489a6a6c7a63..1fb5a5d69719c 100644 --- a/src/librustc/infer/error_reporting.rs +++ b/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>) - -> Option>> { + -> Vec> { 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 { - 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, - 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 { diff --git a/src/librustc/infer/region_inference/mod.rs b/src/librustc/infer/region_inference/mod.rs index af6f2c50e72fc..0bb9e2c7fa15c 100644 --- a/src/librustc/infer/region_inference/mod.rs +++ b/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>, - Vec), } #[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, -} - -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, RegionVid>; pub struct RegionVarBindings<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { diff --git a/src/test/compile-fail/issue-17728.rs b/src/test/compile-fail/issue-17728.rs index f508d7123d88f..9724d17bef1ea 100644 --- a/src/test/compile-fail/issue-17728.rs +++ b/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 } } diff --git a/src/test/ui/lifetime-errors/ex2c-push-inference-variable.stderr b/src/test/ui/lifetime-errors/ex2c-push-inference-variable.stderr index e34d5f0a2f3a1..755b71d4a1d9e 100644 --- a/src/test/ui/lifetime-errors/ex2c-push-inference-variable.stderr +++ b/src/test/ui/lifetime-errors/ex2c-push-inference-variable.stderr @@ -3,6 +3,35 @@ error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` d | 16 | let z = Ref { data: y.data }; | ^^^ + | +note: first, the lifetime cannot outlive the lifetime 'c as defined on the body at 15:66... + --> $DIR/ex2c-push-inference-variable.rs:15:67 + | +15 | fn foo<'a, 'b, 'c>(x: &'a mut Vec>, y: Ref<'c, i32>) { + | ___________________________________________________________________^ starting here... +16 | | let z = Ref { data: y.data }; +17 | | x.push(z); +18 | | } + | |_^ ...ending here +note: ...so that reference does not outlive borrowed content + --> $DIR/ex2c-push-inference-variable.rs:16:25 + | +16 | let z = Ref { data: y.data }; + | ^^^^^^ +note: but, the lifetime must be valid for the lifetime 'b as defined on the body at 15:66... + --> $DIR/ex2c-push-inference-variable.rs:15:67 + | +15 | fn foo<'a, 'b, 'c>(x: &'a mut Vec>, y: Ref<'c, i32>) { + | ___________________________________________________________________^ starting here... +16 | | let z = Ref { data: y.data }; +17 | | x.push(z); +18 | | } + | |_^ ...ending here +note: ...so that expression is assignable (expected Ref<'b, i32>, found Ref<'_, i32>) + --> $DIR/ex2c-push-inference-variable.rs:17:12 + | +17 | x.push(z); + | ^ error: aborting due to previous error diff --git a/src/test/ui/lifetime-errors/ex2d-push-inference-variable-2.stderr b/src/test/ui/lifetime-errors/ex2d-push-inference-variable-2.stderr index f0799a17a468d..daa6ea2d91aa3 100644 --- a/src/test/ui/lifetime-errors/ex2d-push-inference-variable-2.stderr +++ b/src/test/ui/lifetime-errors/ex2d-push-inference-variable-2.stderr @@ -3,6 +3,37 @@ error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` d | 17 | let b = Ref { data: y.data }; | ^^^ + | +note: first, the lifetime cannot outlive the lifetime 'c as defined on the body at 15:66... + --> $DIR/ex2d-push-inference-variable-2.rs:15:67 + | +15 | fn foo<'a, 'b, 'c>(x: &'a mut Vec>, y: Ref<'c, i32>) { + | ___________________________________________________________________^ starting here... +16 | | let a: &mut Vec> = x; +17 | | let b = Ref { data: y.data }; +18 | | a.push(b); +19 | | } + | |_^ ...ending here +note: ...so that reference does not outlive borrowed content + --> $DIR/ex2d-push-inference-variable-2.rs:17:25 + | +17 | let b = Ref { data: y.data }; + | ^^^^^^ +note: but, the lifetime must be valid for the lifetime 'b as defined on the body at 15:66... + --> $DIR/ex2d-push-inference-variable-2.rs:15:67 + | +15 | fn foo<'a, 'b, 'c>(x: &'a mut Vec>, y: Ref<'c, i32>) { + | ___________________________________________________________________^ starting here... +16 | | let a: &mut Vec> = x; +17 | | let b = Ref { data: y.data }; +18 | | a.push(b); +19 | | } + | |_^ ...ending here +note: ...so that expression is assignable (expected &mut std::vec::Vec>, found &mut std::vec::Vec>) + --> $DIR/ex2d-push-inference-variable-2.rs:16:33 + | +16 | let a: &mut Vec> = x; + | ^ error: aborting due to previous error diff --git a/src/test/ui/lifetime-errors/ex2e-push-inference-variable-3.stderr b/src/test/ui/lifetime-errors/ex2e-push-inference-variable-3.stderr index c479be88b139c..b679532a4d910 100644 --- a/src/test/ui/lifetime-errors/ex2e-push-inference-variable-3.stderr +++ b/src/test/ui/lifetime-errors/ex2e-push-inference-variable-3.stderr @@ -3,6 +3,37 @@ error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` d | 17 | let b = Ref { data: y.data }; | ^^^ + | +note: first, the lifetime cannot outlive the lifetime 'c as defined on the body at 15:66... + --> $DIR/ex2e-push-inference-variable-3.rs:15:67 + | +15 | fn foo<'a, 'b, 'c>(x: &'a mut Vec>, y: Ref<'c, i32>) { + | ___________________________________________________________________^ starting here... +16 | | let a: &mut Vec> = x; +17 | | let b = Ref { data: y.data }; +18 | | Vec::push(a, b); +19 | | } + | |_^ ...ending here +note: ...so that reference does not outlive borrowed content + --> $DIR/ex2e-push-inference-variable-3.rs:17:25 + | +17 | let b = Ref { data: y.data }; + | ^^^^^^ +note: but, the lifetime must be valid for the lifetime 'b as defined on the body at 15:66... + --> $DIR/ex2e-push-inference-variable-3.rs:15:67 + | +15 | fn foo<'a, 'b, 'c>(x: &'a mut Vec>, y: Ref<'c, i32>) { + | ___________________________________________________________________^ starting here... +16 | | let a: &mut Vec> = x; +17 | | let b = Ref { data: y.data }; +18 | | Vec::push(a, b); +19 | | } + | |_^ ...ending here +note: ...so that expression is assignable (expected &mut std::vec::Vec>, found &mut std::vec::Vec>) + --> $DIR/ex2e-push-inference-variable-3.rs:16:33 + | +16 | let a: &mut Vec> = x; + | ^ error: aborting due to previous error