From cba4732b6555676ded9e945d1895580e2e328ee9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 12 Dec 2017 08:31:38 -0500 Subject: [PATCH] introduce a `NiceRegionError` type and define methods on that This is more convenient, and allows us to be more independent from infcx, particularly with respect to `in_progress_tables` field. No functional change. --- src/librustc/infer/error_reporting/mod.rs | 4 +- .../nice_region_error/different_lifetimes.rs | 14 +-- .../nice_region_error/find_anon_type.rs | 30 ++--- .../error_reporting/nice_region_error/mod.rs | 50 +++++++- .../nice_region_error/named_anon_conflict.rs | 113 ++++++++++-------- .../error_reporting/nice_region_error/util.rs | 8 +- 6 files changed, 137 insertions(+), 82 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 224f2a3f8c9..aabe4057685 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -291,9 +291,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { for error in errors { debug!("report_region_errors: error = {:?}", error); - if !self.try_report_named_anon_conflict(&error) && - !self.try_report_anon_anon_conflict(&error) - { + if !self.try_report_nice_region_error(&error) { match error.clone() { // These errors could indicate all manner of different // problems with many different solutions. Rather diff --git a/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs b/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs index 35c94853b9e..5061a2ba277 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs @@ -10,12 +10,10 @@ //! Error Reporting for Anonymous Region Lifetime Errors //! where both the regions are anonymous. -use infer::InferCtxt; -use infer::lexical_region_resolve::RegionResolutionError::*; -use infer::lexical_region_resolve::RegionResolutionError; +use infer::error_reporting::nice_region_error::NiceRegionError; use infer::error_reporting::nice_region_error::util::AnonymousArgInfo; -impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { +impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { /// Print the error message for lifetime errors when both the concerned regions are anonymous. /// /// Consider a case where we have @@ -52,12 +50,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { /// ```` /// /// It will later be extended to trait objects. - pub fn try_report_anon_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool { - let (span, sub, sup) = match *error { - ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup), - SubSupConflict(_, ref origin, sub, _, sup) => (origin.span(), sub, sup), - _ => return false, // inapplicable - }; + pub(super) fn try_report_anon_anon_conflict(&self) -> bool { + let NiceRegionError { span, sub, sup, .. } = *self; // Determine whether the sub and sup consist of both anonymous (elided) regions. let anon_reg_sup = or_false!(self.is_suitable_region(sup)); diff --git a/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs b/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs index 92fdb52e02f..dc53c1db06f 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs @@ -9,13 +9,13 @@ // except according to those terms. use hir; -use infer::InferCtxt; -use ty::{self, Region}; +use ty::{self, Region, TyCtxt}; use hir::map as hir_map; use middle::resolve_lifetime as rl; use hir::intravisit::{self, NestedVisitorMap, Visitor}; +use infer::error_reporting::nice_region_error::NiceRegionError; -impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { +impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { /// This function calls the `visit_ty` method for the parameters /// corresponding to the anonymous regions. The `nested_visitor.found_type` /// contains the anonymous type. @@ -74,8 +74,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { br: &ty::BoundRegion, ) -> Option<(&'gcx hir::Ty)> { let mut nested_visitor = FindNestedTypeVisitor { - infcx: &self, - hir_map: &self.tcx.hir, + tcx: self.tcx, bound_region: *br, found_type: None, depth: 1, @@ -93,8 +92,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // where that lifetime appears. This allows us to highlight the // specific part of the type in the error message. struct FindNestedTypeVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { - infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, - hir_map: &'a hir::map::Map<'gcx>, + tcx: TyCtxt<'a, 'gcx, 'tcx>, // The bound_region corresponding to the Refree(freeregion) // associated with the anonymous region we are looking for. bound_region: ty::BoundRegion, @@ -106,7 +104,7 @@ struct FindNestedTypeVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindNestedTypeVisitor<'a, 'gcx, 'tcx> { fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'gcx> { - NestedVisitorMap::OnlyBodies(&self.hir_map) + NestedVisitorMap::OnlyBodies(&self.tcx.hir) } fn visit_ty(&mut self, arg: &'gcx hir::Ty) { @@ -126,8 +124,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindNestedTypeVisitor<'a, 'gcx, 'tcx> { hir::TyRptr(ref lifetime, _) => { // the lifetime of the TyRptr - let hir_id = self.infcx.tcx.hir.node_to_hir_id(lifetime.id); - match (self.infcx.tcx.named_region(hir_id), self.bound_region) { + let hir_id = self.tcx.hir.node_to_hir_id(lifetime.id); + match (self.tcx.named_region(hir_id), self.bound_region) { // Find the index of the anonymous region that was part of the // error. We will then search the function parameters for a bound // region at the right depth with the same index @@ -195,10 +193,9 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindNestedTypeVisitor<'a, 'gcx, 'tcx> { // Checks if it is of type `hir::TyPath` which corresponds to a struct. hir::TyPath(_) => { let subvisitor = &mut TyPathVisitor { - infcx: self.infcx, + tcx: self.tcx, found_it: false, bound_region: self.bound_region, - hir_map: self.hir_map, depth: self.depth, }; intravisit::walk_ty(subvisitor, arg); // call walk_ty; as visit_ty is empty, @@ -222,8 +219,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindNestedTypeVisitor<'a, 'gcx, 'tcx> { // where that lifetime appears. This allows us to highlight the // specific part of the type in the error message. struct TyPathVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { - infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, - hir_map: &'a hir::map::Map<'gcx>, + tcx: TyCtxt<'a, 'gcx, 'tcx>, found_it: bool, bound_region: ty::BoundRegion, depth: u32, @@ -231,12 +227,12 @@ struct TyPathVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { impl<'a, 'gcx, 'tcx> Visitor<'gcx> for TyPathVisitor<'a, 'gcx, 'tcx> { fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'gcx> { - NestedVisitorMap::OnlyBodies(&self.hir_map) + NestedVisitorMap::OnlyBodies(&self.tcx.hir) } fn visit_lifetime(&mut self, lifetime: &hir::Lifetime) { - let hir_id = self.infcx.tcx.hir.node_to_hir_id(lifetime.id); - match (self.infcx.tcx.named_region(hir_id), self.bound_region) { + let hir_id = self.tcx.hir.node_to_hir_id(lifetime.id); + match (self.tcx.named_region(hir_id), self.bound_region) { // the lifetime of the TyPath! (Some(rl::Region::LateBoundAnon(debruijn_index, anon_index)), ty::BrAnon(br_index)) => { if debruijn_index.depth == self.depth && anon_index == br_index { diff --git a/src/librustc/infer/error_reporting/nice_region_error/mod.rs b/src/librustc/infer/error_reporting/nice_region_error/mod.rs index ca6247defe0..46283041fac 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/mod.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/mod.rs @@ -8,8 +8,56 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#[macro_use] mod util; +use infer::InferCtxt; +use infer::lexical_region_resolve::RegionResolutionError; +use infer::lexical_region_resolve::RegionResolutionError::*; +use syntax::codemap::Span; +use ty::{self, TyCtxt}; + +#[macro_use] +mod util; mod find_anon_type; mod different_lifetimes; mod named_anon_conflict; + +impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { + pub fn try_report_nice_region_error(&self, error: &RegionResolutionError<'tcx>) -> bool { + let (span, sub, sup) = match *error { + ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup), + SubSupConflict(_, ref origin, sub, _, sup) => (origin.span(), sub, sup), + _ => return false, // inapplicable + }; + + if let Some(tables) = self.in_progress_tables { + let tables = tables.borrow(); + NiceRegionError::new(self.tcx, span, sub, sup, Some(&tables)).try_report() + } else { + NiceRegionError::new(self.tcx, span, sub, sup, None).try_report() + } + } +} + +pub struct NiceRegionError<'cx, 'gcx: 'tcx, 'tcx: 'cx> { + tcx: TyCtxt<'cx, 'gcx, 'tcx>, + span: Span, + sub: ty::Region<'tcx>, + sup: ty::Region<'tcx>, + tables: Option<&'cx ty::TypeckTables<'tcx>>, +} + +impl<'cx, 'gcx, 'tcx> NiceRegionError<'cx, 'gcx, 'tcx> { + pub fn new( + tcx: TyCtxt<'cx, 'gcx, 'tcx>, + span: Span, + sub: ty::Region<'tcx>, + sup: ty::Region<'tcx>, + tables: Option<&'cx ty::TypeckTables<'tcx>>, + ) -> Self { + Self { tcx, span, sub, sup, tables } + } + + pub fn try_report(&self) -> bool { + self.try_report_anon_anon_conflict() || self.try_report_named_anon_conflict() + } +} diff --git a/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs b/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs index 6af7415ba53..87c3da2ee16 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs @@ -10,22 +10,20 @@ //! Error Reporting for Anonymous Region Lifetime Errors //! where one region is named and the other is anonymous. -use infer::InferCtxt; -use infer::lexical_region_resolve::RegionResolutionError::*; -use infer::lexical_region_resolve::RegionResolutionError; +use infer::error_reporting::nice_region_error::NiceRegionError; use ty; -impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { +impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { /// When given a `ConcreteFailure` for a function with arguments containing a named region and /// an anonymous region, emit an descriptive diagnostic error. - pub fn try_report_named_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool { - let (span, sub, sup) = match *error { - ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup), - SubSupConflict(_, ref origin, sub, _, sup) => (origin.span(), sub, sup), - _ => return false, // inapplicable - }; + pub(super) fn try_report_named_anon_conflict(&self) -> bool { + let NiceRegionError { span, sub, sup, .. } = *self; - debug!("try_report_named_anon_conflict(sub={:?}, sup={:?})", sub, sup); + debug!( + "try_report_named_anon_conflict(sub={:?}, sup={:?})", + sub, + sup + ); // Determine whether the sub and sup consist of one named region ('a) // and one anonymous (elided) region. If so, find the parameter arg @@ -33,33 +31,47 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // only introduced anonymous regions in parameters) as well as a // version new_ty of its type where the anonymous region is replaced // with the named one.//scope_def_id - let (named, anon, anon_arg_info, region_info) = - if self.is_named_region(sub) && self.is_suitable_region(sup).is_some() && - self.find_arg_with_region(sup, sub).is_some() { - (sub, - sup, - self.find_arg_with_region(sup, sub).unwrap(), - self.is_suitable_region(sup).unwrap()) - } else if self.is_named_region(sup) && self.is_suitable_region(sub).is_some() && - self.find_arg_with_region(sub, sup).is_some() { - (sup, - sub, - self.find_arg_with_region(sub, sup).unwrap(), - self.is_suitable_region(sub).unwrap()) - } else { - return false; // inapplicable - }; + let (named, anon, anon_arg_info, region_info) = if self.is_named_region(sub) + && self.is_suitable_region(sup).is_some() + && self.find_arg_with_region(sup, sub).is_some() + { + ( + sub, + sup, + self.find_arg_with_region(sup, sub).unwrap(), + self.is_suitable_region(sup).unwrap(), + ) + } else if self.is_named_region(sup) && self.is_suitable_region(sub).is_some() + && self.find_arg_with_region(sub, sup).is_some() + { + ( + sup, + sub, + self.find_arg_with_region(sub, sup).unwrap(), + self.is_suitable_region(sub).unwrap(), + ) + } else { + return false; // inapplicable + }; debug!("try_report_named_anon_conflict: named = {:?}", named); - debug!("try_report_named_anon_conflict: anon_arg_info = {:?}", anon_arg_info); - debug!("try_report_named_anon_conflict: region_info = {:?}", region_info); + debug!( + "try_report_named_anon_conflict: anon_arg_info = {:?}", + anon_arg_info + ); + debug!( + "try_report_named_anon_conflict: region_info = {:?}", + region_info + ); - let (arg, new_ty, br, is_first, scope_def_id, is_impl_item) = (anon_arg_info.arg, - anon_arg_info.arg_ty, - anon_arg_info.bound_region, - anon_arg_info.is_first, - region_info.def_id, - region_info.is_impl_item); + let (arg, new_ty, br, is_first, scope_def_id, is_impl_item) = ( + anon_arg_info.arg, + anon_arg_info.arg_ty, + anon_arg_info.bound_region, + anon_arg_info.is_first, + region_info.def_id, + region_info.is_impl_item, + ); match br { ty::BrAnon(_) => {} _ => { @@ -75,27 +87,34 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } if let Some((_, fndecl)) = self.find_anon_type(anon, &br) { - if self.is_return_type_anon(scope_def_id, br, fndecl).is_some() || - self.is_self_anon(is_first, scope_def_id) { + if self.is_return_type_anon(scope_def_id, br, fndecl).is_some() + || self.is_self_anon(is_first, scope_def_id) + { return false; } } let (error_var, span_label_var) = if let Some(simple_name) = arg.pat.simple_name() { - (format!("the type of `{}`", simple_name), format!("the type of `{}`", simple_name)) + ( + format!("the type of `{}`", simple_name), + format!("the type of `{}`", simple_name), + ) } else { ("parameter type".to_owned(), "type".to_owned()) }; - struct_span_err!(self.tcx.sess, - span, - E0621, - "explicit lifetime required in {}", - error_var) - .span_label(arg.pat.span, - format!("consider changing {} to `{}`", span_label_var, new_ty)) - .span_label(span, format!("lifetime `{}` required", named)) - .emit(); + struct_span_err!( + self.tcx.sess, + span, + E0621, + "explicit lifetime required in {}", + error_var + ).span_label( + arg.pat.span, + format!("consider changing {} to `{}`", span_label_var, new_ty), + ) + .span_label(span, format!("lifetime `{}` required", named)) + .emit(); return true; } } diff --git a/src/librustc/infer/error_reporting/nice_region_error/util.rs b/src/librustc/infer/error_reporting/nice_region_error/util.rs index 04584f34a15..9a9b8fbbb94 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/util.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/util.rs @@ -11,7 +11,7 @@ //! Helper functions corresponding to lifetime errors due to //! anonymous regions. use hir; -use infer::InferCtxt; +use infer::error_reporting::nice_region_error::NiceRegionError; use ty::{self, Region, Ty}; use hir::def_id::DefId; use hir::map as hir_map; @@ -56,7 +56,7 @@ pub(super) struct FreeRegionInfo { pub is_impl_item: bool, } -impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { +impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { // This method walks the Type of the function body arguments using // `fold_regions()` function and returns the // &hir::Arg of the function argument corresponding to the anonymous @@ -86,13 +86,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { if let Some(node_id) = hir.as_local_node_id(id) { if let Some(body_id) = hir.maybe_body_owned_by(node_id) { let body = hir.body(body_id); - if let Some(tables) = self.in_progress_tables { + if let Some(tables) = self.tables { body.arguments .iter() .enumerate() .filter_map(|(index, arg)| { // May return None; sometimes the tables are not yet populated. - let ty = tables.borrow().node_id_to_type_opt(arg.hir_id)?; + let ty = tables.node_id_to_type_opt(arg.hir_id)?; let mut found_anon_region = false; let new_arg_ty = self.tcx.fold_regions(&ty, &mut false, |r, _| { if *r == *anon_region {