From 109c30f3d4783f569eb4d9350e6045a1e96af80d Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Sat, 28 Dec 2019 19:09:42 -0600 Subject: [PATCH 1/8] More separation of error reporting from region inference --- .../borrow_check/diagnostics/region_errors.rs | 33 +++----- src/librustc_mir/borrow_check/mod.rs | 77 ++++++++++--------- .../borrow_check/region_infer/mod.rs | 64 ++++++--------- .../borrow_check/region_infer/values.rs | 2 +- 4 files changed, 77 insertions(+), 99 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index dc63fa80275e1..bf4a12b12a5c2 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -1,8 +1,7 @@ //! Error reporting machinery for lifetime errors. use rustc::infer::{ - error_reporting::nice_region_error::NiceRegionError, region_constraints::GenericKind, - InferCtxt, NLLRegionVariableOrigin, + error_reporting::nice_region_error::NiceRegionError, InferCtxt, NLLRegionVariableOrigin, }; use rustc::mir::{Body, ConstraintCategory, Location}; use rustc::ty::{self, RegionVid, Ty}; @@ -16,8 +15,11 @@ use std::collections::VecDeque; use crate::util::borrowck_errors; use crate::borrow_check::{ - constraints::OutlivesConstraint, nll::ConstraintDescription, - region_infer::RegionInferenceContext, type_check::Locations, universal_regions::DefiningTy, + constraints::OutlivesConstraint, + nll::ConstraintDescription, + region_infer::{values::RegionElement, RegionInferenceContext, TypeTest}, + type_check::Locations, + universal_regions::DefiningTy, MirBorrowckCtxt, }; @@ -62,23 +64,8 @@ crate type RegionErrors<'tcx> = Vec>; #[derive(Clone, Debug)] crate enum RegionErrorKind<'tcx> { - /// An error for a type test: `T: 'a` does not live long enough. - TypeTestDoesNotLiveLongEnough { - /// The span of the type test. - span: Span, - /// The generic type of the type test. - generic: GenericKind<'tcx>, - }, - - /// A generic bound failure for a type test. - TypeTestGenericBoundError { - /// The span of the type test. - span: Span, - /// The generic type of the type test. - generic: GenericKind<'tcx>, - /// The lower bound region. - lower_bound_region: ty::Region<'tcx>, - }, + /// A generic bound failure for a type test (`T: 'a`). + TypeTestError { type_test: TypeTest<'tcx> }, /// An unexpected hidden region for an opaque type. UnexpectedHiddenRegion { @@ -94,8 +81,8 @@ crate enum RegionErrorKind<'tcx> { BoundUniversalRegionError { /// The placeholder free region. longer_fr: RegionVid, - /// The region that erroneously must be outlived by `longer_fr`. - error_region: RegionVid, + /// The region element that erroneously must be outlived by `longer_fr`. + error_element: RegionElement, /// The origin of the placeholder region. fr_origin: NLLRegionVariableOrigin, }, diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 7b0a103fd0038..fc1b723c27120 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -631,7 +631,7 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx debug!( "visit_terminator_drop \ - loc: {:?} term: {:?} drop_place: {:?} drop_place_ty: {:?} span: {:?}", + loc: {:?} term: {:?} drop_place: {:?} drop_place_ty: {:?} span: {:?}", loc, term, drop_place, drop_place_ty, span ); @@ -1477,38 +1477,42 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { for nll_error in nll_errors.into_iter() { match nll_error { - RegionErrorKind::TypeTestDoesNotLiveLongEnough { span, generic } => { - // FIXME. We should handle this case better. It - // indicates that we have e.g., some region variable - // whose value is like `'a+'b` where `'a` and `'b` are - // distinct unrelated univesal regions that are not - // known to outlive one another. It'd be nice to have - // some examples where this arises to decide how best - // to report it; we could probably handle it by - // iterating over the universal regions and reporting - // an error that multiple bounds are required. - self.infcx - .tcx - .sess - .struct_span_err(span, &format!("`{}` does not live long enough", generic)) - .buffer(&mut self.errors_buffer); - } - - RegionErrorKind::TypeTestGenericBoundError { - span, - generic, - lower_bound_region, - } => { - let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); - self.infcx - .construct_generic_bound_failure( - region_scope_tree, - span, - None, - generic, - lower_bound_region, - ) - .buffer(&mut self.errors_buffer); + RegionErrorKind::TypeTestError { type_test } => { + // Try to convert the lower-bound region into something named we can print for the user. + let lower_bound_region = + self.nonlexical_regioncx.to_error_region(type_test.lower_bound); + + // Skip duplicate-ish errors. + let type_test_span = type_test.locations.span(&self.body); + + if let Some(lower_bound_region) = lower_bound_region { + let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); + self.infcx + .construct_generic_bound_failure( + region_scope_tree, + type_test_span, + None, + type_test.generic_kind, + lower_bound_region, + ) + .buffer(&mut self.errors_buffer); + } else { + // FIXME. We should handle this case better. It indicates that we have + // e.g., some region variable whose value is like `'a+'b` where `'a` and + // `'b` are distinct unrelated univesal regions that are not known to + // outlive one another. It'd be nice to have some examples where this + // arises to decide how best to report it; we could probably handle it by + // iterating over the universal regions and reporting an error that + // multiple bounds are required. + self.infcx + .tcx + .sess + .struct_span_err( + type_test_span, + &format!("`{}` does not live long enough", type_test.generic_kind), + ) + .buffer(&mut self.errors_buffer); + } } RegionErrorKind::UnexpectedHiddenRegion { @@ -1530,8 +1534,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { RegionErrorKind::BoundUniversalRegionError { longer_fr, fr_origin, - error_region, + error_element, } => { + let error_region = + self.nonlexical_regioncx.region_from_element(longer_fr, error_element); + // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span( &self.body, @@ -2225,7 +2232,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let upvar = &self.upvars[field.index()]; debug!( "upvar.mutability={:?} local_mutation_is_allowed={:?} \ - place={:?}", + place={:?}", upvar, is_local_mutation_allowed, place ); match (upvar.mutability, is_local_mutation_allowed) { diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 7d2384f8a7de7..baa1c5b617dca 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -838,39 +838,20 @@ impl<'tcx> RegionInferenceContext<'tcx> { } // Type-test failed. Report the error. - - // Try to convert the lower-bound region into something named we can print for the user. - let lower_bound_region = self.to_error_region(type_test.lower_bound); - - // Skip duplicate-ish errors. - let type_test_span = type_test.locations.span(body); - let erased_generic_kind = tcx.erase_regions(&type_test.generic_kind); - if !deduplicate_errors.insert(( + let erased_generic_kind = infcx.tcx.erase_regions(&type_test.generic_kind); + if deduplicate_errors.insert(( erased_generic_kind, - lower_bound_region, + type_test.lower_bound, type_test.locations, )) { - continue; - } else { debug!( "check_type_test: reporting error for erased_generic_kind={:?}, \ lower_bound_region={:?}, \ type_test.locations={:?}", - erased_generic_kind, lower_bound_region, type_test.locations, + erased_generic_kind, type_test.lower_bound, type_test.locations, ); - } - if let Some(lower_bound_region) = lower_bound_region { - errors_buffer.push(RegionErrorKind::TypeTestGenericBoundError { - span: type_test_span, - generic: type_test.generic_kind, - lower_bound_region, - }); - } else { - errors_buffer.push(RegionErrorKind::TypeTestDoesNotLiveLongEnough { - span: type_test_span, - generic: type_test.generic_kind, - }); + errors_buffer.push(RegionErrorKind::TypeTestError { type_test: type_test.clone() }); } } } @@ -1355,7 +1336,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { for (longer_fr, shorter_fr) in subset_errors.into_iter() { debug!( "check_polonius_subset_errors: subset_error longer_fr={:?},\ - shorter_fr={:?}", + shorter_fr={:?}", longer_fr, shorter_fr ); @@ -1572,23 +1553,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { debug!("check_bound_universal_region: error_element = {:?}", error_element); // Find the region that introduced this `error_element`. - let error_region = match error_element { - RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l), - RegionElement::RootUniversalRegion(r) => r, - RegionElement::PlaceholderRegion(error_placeholder) => self - .definitions - .iter_enumerated() - .filter_map(|(r, definition)| match definition.origin { - NLLRegionVariableOrigin::Placeholder(p) if p == error_placeholder => Some(r), - _ => None, - }) - .next() - .unwrap(), - }; - errors_buffer.push(RegionErrorKind::BoundUniversalRegionError { longer_fr, - error_region, + error_element, fr_origin: NLLRegionVariableOrigin::Placeholder(placeholder), }); } @@ -1628,6 +1595,23 @@ impl<'tcx> RegionInferenceContext<'tcx> { }); } } + + /// Get the region outlived by `longer_fr` and live at `element`. + crate fn region_from_element(&self, longer_fr: RegionVid, element: RegionElement) -> RegionVid { + match element { + RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l), + RegionElement::RootUniversalRegion(r) => r, + RegionElement::PlaceholderRegion(error_placeholder) => self + .definitions + .iter_enumerated() + .filter_map(|(r, definition)| match definition.origin { + NLLRegionVariableOrigin::Placeholder(p) if p == error_placeholder => Some(r), + _ => None, + }) + .next() + .unwrap(), + } + } } impl<'tcx> RegionDefinition<'tcx> { diff --git a/src/librustc_mir/borrow_check/region_infer/values.rs b/src/librustc_mir/borrow_check/region_infer/values.rs index e17efebfef824..3126d44014b4e 100644 --- a/src/librustc_mir/borrow_check/region_infer/values.rs +++ b/src/librustc_mir/borrow_check/region_infer/values.rs @@ -114,7 +114,7 @@ rustc_index::newtype_index! { /// An individual element in a region value -- the value of a /// particular region variable consists of a set of these elements. -#[derive(Debug)] +#[derive(Debug, Clone)] crate enum RegionElement { /// A point in the control-flow graph. Location(Location), From c3e74f347e44e3a54f6ff094ef8b741589d69ab5 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Sat, 28 Dec 2019 19:25:57 -0600 Subject: [PATCH 2/8] Move some methods to region_infer/mod.rs --- .../diagnostics/explain_borrow.rs | 45 +- .../borrow_check/diagnostics/region_errors.rs | 431 +----------------- .../borrow_check/region_infer/mod.rs | 395 ++++++++++++++++ 3 files changed, 440 insertions(+), 431 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs index 9a0c99b07e6f1..afccf8a0922a7 100644 --- a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs +++ b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs @@ -2,12 +2,13 @@ use std::collections::VecDeque; +use rustc::infer::NLLRegionVariableOrigin; use rustc::mir::{ Body, CastKind, ConstraintCategory, FakeReadCause, Local, Location, Operand, Place, Rvalue, Statement, StatementKind, TerminatorKind, }; use rustc::ty::adjustment::PointerCast; -use rustc::ty::{self, TyCtxt}; +use rustc::ty::{self, RegionVid, TyCtxt}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_index::vec::IndexVec; @@ -15,8 +16,8 @@ use rustc_span::symbol::Symbol; use rustc_span::Span; use crate::borrow_check::{ - borrow_set::BorrowData, nll::ConstraintDescription, region_infer::Cause, MirBorrowckCtxt, - WriteKind, + borrow_set::BorrowData, diagnostics::RegionErrorNamingCtx, nll::ConstraintDescription, + region_infer::Cause, MirBorrowckCtxt, WriteKind, }; use super::{find_use, RegionName, UseSpans}; @@ -254,6 +255,32 @@ impl BorrowExplanation { } impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { + fn free_region_constraint_info( + &self, + borrow_region: RegionVid, + outlived_region: RegionVid, + ) -> (ConstraintCategory, bool, Span, Option) { + let (category, from_closure, span) = self.nonlexical_regioncx.best_blame_constraint( + &self.body, + borrow_region, + NLLRegionVariableOrigin::FreeRegion, + |r| { + self.nonlexical_regioncx.provides_universal_region( + r, + borrow_region, + outlived_region, + ) + }, + ); + + let mut renctx = RegionErrorNamingCtx::new(); + let outlived_fr_name = + self.nonlexical_regioncx.give_region_a_name(self, &mut renctx, outlived_region); + // TODO(mark-i-m): just return the region and let the caller name it + + (category, from_closure, span, outlived_fr_name) + } + /// Returns structured explanation for *why* the borrow contains the /// point from `location`. This is key for the "3-point errors" /// [described in the NLL RFC][d]. @@ -285,7 +312,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let borrow_region_vid = borrow.region; debug!("explain_why_borrow_contains_point: borrow_region_vid={:?}", borrow_region_vid); - let region_sub = regioncx.find_sub_region_live_at(borrow_region_vid, location); + let region_sub = + self.nonlexical_regioncx.find_sub_region_live_at(borrow_region_vid, location); debug!("explain_why_borrow_contains_point: region_sub={:?}", region_sub); match find_use::find(body, regioncx, tcx, region_sub, location) { @@ -330,9 +358,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { None => { if let Some(region) = regioncx.to_error_region_vid(borrow_region_vid) { - let (category, from_closure, span, region_name) = self - .nonlexical_regioncx - .free_region_constraint_info(self, borrow_region_vid, region); + let (category, from_closure, span, region_name) = + self.free_region_constraint_info(borrow_region_vid, region); if let Some(region_name) = region_name { let opt_place_desc = self.describe_place(borrow.borrowed_place.as_ref()); BorrowExplanation::MustBeValidFor { @@ -345,14 +372,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } else { debug!( "explain_why_borrow_contains_point: \ - Could not generate a region name" + Could not generate a region name" ); BorrowExplanation::Unexplained } } else { debug!( "explain_why_borrow_contains_point: \ - Could not generate an error region vid" + Could not generate an error region vid" ); BorrowExplanation::Unexplained } diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index bf4a12b12a5c2..f9ec4ccb52a09 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -3,22 +3,18 @@ use rustc::infer::{ error_reporting::nice_region_error::NiceRegionError, InferCtxt, NLLRegionVariableOrigin, }; -use rustc::mir::{Body, ConstraintCategory, Location}; +use rustc::mir::ConstraintCategory; use rustc::ty::{self, RegionVid, Ty}; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir::def_id::DefId; -use rustc_index::vec::IndexVec; use rustc_span::symbol::kw; use rustc_span::Span; -use std::collections::VecDeque; use crate::util::borrowck_errors; use crate::borrow_check::{ - constraints::OutlivesConstraint, nll::ConstraintDescription, region_infer::{values::RegionElement, RegionInferenceContext, TypeTest}, - type_check::Locations, universal_regions::DefiningTy, MirBorrowckCtxt, }; @@ -48,13 +44,6 @@ impl ConstraintDescription for ConstraintCategory { } } -#[derive(Copy, Clone, PartialEq, Eq, Debug)] -enum Trace { - StartRegion, - FromOutlivesConstraint(OutlivesConstraint), - NotVisited, -} - /// A collection of errors encountered during region inference. This is needed to efficiently /// report errors after borrow checking. /// @@ -142,270 +131,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - /// Tries to find the best constraint to blame for the fact that - /// `R: from_region`, where `R` is some region that meets - /// `target_test`. This works by following the constraint graph, - /// creating a constraint path that forces `R` to outlive - /// `from_region`, and then finding the best choices within that - /// path to blame. - fn best_blame_constraint( - &self, - body: &Body<'tcx>, - from_region: RegionVid, - from_region_origin: NLLRegionVariableOrigin, - target_test: impl Fn(RegionVid) -> bool, - ) -> (ConstraintCategory, bool, Span) { - debug!( - "best_blame_constraint(from_region={:?}, from_region_origin={:?})", - from_region, from_region_origin - ); - - // Find all paths - let (path, target_region) = - self.find_constraint_paths_between_regions(from_region, target_test).unwrap(); - debug!( - "best_blame_constraint: path={:#?}", - path.iter() - .map(|&c| format!( - "{:?} ({:?}: {:?})", - c, - self.constraint_sccs.scc(c.sup), - self.constraint_sccs.scc(c.sub), - )) - .collect::>() - ); - - // Classify each of the constraints along the path. - let mut categorized_path: Vec<(ConstraintCategory, bool, Span)> = path - .iter() - .map(|constraint| { - if constraint.category == ConstraintCategory::ClosureBounds { - self.retrieve_closure_constraint_info(body, &constraint) - } else { - (constraint.category, false, constraint.locations.span(body)) - } - }) - .collect(); - debug!("best_blame_constraint: categorized_path={:#?}", categorized_path); - - // To find the best span to cite, we first try to look for the - // final constraint that is interesting and where the `sup` is - // not unified with the ultimate target region. The reason - // for this is that we have a chain of constraints that lead - // from the source to the target region, something like: - // - // '0: '1 ('0 is the source) - // '1: '2 - // '2: '3 - // '3: '4 - // '4: '5 - // '5: '6 ('6 is the target) - // - // Some of those regions are unified with `'6` (in the same - // SCC). We want to screen those out. After that point, the - // "closest" constraint we have to the end is going to be the - // most likely to be the point where the value escapes -- but - // we still want to screen for an "interesting" point to - // highlight (e.g., a call site or something). - let target_scc = self.constraint_sccs.scc(target_region); - let mut range = 0..path.len(); - - // As noted above, when reporting an error, there is typically a chain of constraints - // leading from some "source" region which must outlive some "target" region. - // In most cases, we prefer to "blame" the constraints closer to the target -- - // but there is one exception. When constraints arise from higher-ranked subtyping, - // we generally prefer to blame the source value, - // as the "target" in this case tends to be some type annotation that the user gave. - // Therefore, if we find that the region origin is some instantiation - // of a higher-ranked region, we start our search from the "source" point - // rather than the "target", and we also tweak a few other things. - // - // An example might be this bit of Rust code: - // - // ```rust - // let x: fn(&'static ()) = |_| {}; - // let y: for<'a> fn(&'a ()) = x; - // ``` - // - // In MIR, this will be converted into a combination of assignments and type ascriptions. - // In particular, the 'static is imposed through a type ascription: - // - // ```rust - // x = ...; - // AscribeUserType(x, fn(&'static ()) - // y = x; - // ``` - // - // We wind up ultimately with constraints like - // - // ```rust - // !a: 'temp1 // from the `y = x` statement - // 'temp1: 'temp2 - // 'temp2: 'static // from the AscribeUserType - // ``` - // - // and here we prefer to blame the source (the y = x statement). - let blame_source = match from_region_origin { - NLLRegionVariableOrigin::FreeRegion - | NLLRegionVariableOrigin::Existential { from_forall: false } => true, - NLLRegionVariableOrigin::Placeholder(_) - | NLLRegionVariableOrigin::Existential { from_forall: true } => false, - }; - - let find_region = |i: &usize| { - let constraint = path[*i]; - - let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup); - - if blame_source { - match categorized_path[*i].0 { - ConstraintCategory::OpaqueType - | ConstraintCategory::Boring - | ConstraintCategory::BoringNoLocation - | ConstraintCategory::Internal => false, - ConstraintCategory::TypeAnnotation - | ConstraintCategory::Return - | ConstraintCategory::Yield => true, - _ => constraint_sup_scc != target_scc, - } - } else { - match categorized_path[*i].0 { - ConstraintCategory::OpaqueType - | ConstraintCategory::Boring - | ConstraintCategory::BoringNoLocation - | ConstraintCategory::Internal => false, - _ => true, - } - } - }; - - let best_choice = - if blame_source { range.rev().find(find_region) } else { range.find(find_region) }; - - debug!( - "best_blame_constraint: best_choice={:?} blame_source={}", - best_choice, blame_source - ); - - if let Some(i) = best_choice { - if let Some(next) = categorized_path.get(i + 1) { - if categorized_path[i].0 == ConstraintCategory::Return - && next.0 == ConstraintCategory::OpaqueType - { - // The return expression is being influenced by the return type being - // impl Trait, point at the return type and not the return expr. - return *next; - } - } - return categorized_path[i]; - } - - // If that search fails, that is.. unusual. Maybe everything - // is in the same SCC or something. In that case, find what - // appears to be the most interesting point to report to the - // user via an even more ad-hoc guess. - categorized_path.sort_by(|p0, p1| p0.0.cmp(&p1.0)); - debug!("`: sorted_path={:#?}", categorized_path); - - *categorized_path.first().unwrap() - } - - /// Walks the graph of constraints (where `'a: 'b` is considered - /// an edge `'a -> 'b`) to find all paths from `from_region` to - /// `to_region`. The paths are accumulated into the vector - /// `results`. The paths are stored as a series of - /// `ConstraintIndex` values -- in other words, a list of *edges*. - /// - /// Returns: a series of constraints as well as the region `R` - /// that passed the target test. - fn find_constraint_paths_between_regions( - &self, - from_region: RegionVid, - target_test: impl Fn(RegionVid) -> bool, - ) -> Option<(Vec, RegionVid)> { - let mut context = IndexVec::from_elem(Trace::NotVisited, &self.definitions); - context[from_region] = Trace::StartRegion; - - // Use a deque so that we do a breadth-first search. We will - // stop at the first match, which ought to be the shortest - // path (fewest constraints). - let mut deque = VecDeque::new(); - deque.push_back(from_region); - - while let Some(r) = deque.pop_front() { - debug!( - "find_constraint_paths_between_regions: from_region={:?} r={:?} value={}", - from_region, - r, - self.region_value_str(r), - ); - - // Check if we reached the region we were looking for. If so, - // we can reconstruct the path that led to it and return it. - if target_test(r) { - let mut result = vec![]; - let mut p = r; - loop { - match context[p] { - Trace::NotVisited => { - bug!("found unvisited region {:?} on path to {:?}", p, r) - } - - Trace::FromOutlivesConstraint(c) => { - result.push(c); - p = c.sup; - } - - Trace::StartRegion => { - result.reverse(); - return Some((result, r)); - } - } - } - } - - // Otherwise, walk over the outgoing constraints and - // enqueue any regions we find, keeping track of how we - // reached them. - - // A constraint like `'r: 'x` can come from our constraint - // graph. - let fr_static = self.universal_regions.fr_static; - let outgoing_edges_from_graph = - self.constraint_graph.outgoing_edges(r, &self.constraints, fr_static); - - // Always inline this closure because it can be hot. - let mut handle_constraint = #[inline(always)] - |constraint: OutlivesConstraint| { - debug_assert_eq!(constraint.sup, r); - let sub_region = constraint.sub; - if let Trace::NotVisited = context[sub_region] { - context[sub_region] = Trace::FromOutlivesConstraint(constraint); - deque.push_back(sub_region); + /// Returns `true` if a closure is inferred to be an `FnMut` closure. + crate fn is_closure_fn_mut(&self, infcx: &InferCtxt<'_, 'tcx>, fr: RegionVid) -> bool { + if let Some(ty::ReFree(free_region)) = self.to_error_region(fr) { + if let ty::BoundRegion::BrEnv = free_region.bound_region { + if let DefiningTy::Closure(def_id, substs) = self.universal_regions.defining_ty { + let closure_kind_ty = substs.as_closure().kind_ty(def_id, infcx.tcx); + return Some(ty::ClosureKind::FnMut) == closure_kind_ty.to_opt_closure_kind(); } - }; - - // This loop can be hot. - for constraint in outgoing_edges_from_graph { - handle_constraint(constraint); - } - - // Member constraints can also give rise to `'r: 'x` edges that - // were not part of the graph initially, so watch out for those. - // (But they are extremely rare; this loop is very cold.) - for constraint in self.applied_member_constraints(r) { - let p_c = &self.member_constraints[constraint.member_constraint_index]; - let constraint = OutlivesConstraint { - sup: r, - sub: constraint.min_choice, - locations: Locations::All(p_c.definition_span), - category: ConstraintCategory::OpaqueType, - }; - handle_constraint(constraint); } } - None + false } /// Report an error because the universal region `fr` was required to outlive @@ -484,30 +221,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - /// We have a constraint `fr1: fr2` that is not satisfied, where - /// `fr2` represents some universal region. Here, `r` is some - /// region where we know that `fr1: r` and this function has the - /// job of determining whether `r` is "to blame" for the fact that - /// `fr1: fr2` is required. - /// - /// This is true under two conditions: - /// - /// - `r == fr2` - /// - `fr2` is `'static` and `r` is some placeholder in a universe - /// that cannot be named by `fr1`; in that case, we will require - /// that `fr1: 'static` because it is the only way to `fr1: r` to - /// be satisfied. (See `add_incompatible_universe`.) - fn provides_universal_region(&self, r: RegionVid, fr1: RegionVid, fr2: RegionVid) -> bool { - debug!("provides_universal_region(r={:?}, fr1={:?}, fr2={:?})", r, fr1, fr2); - let result = { - r == fr2 || { - fr2 == self.universal_regions.fr_static && self.cannot_name_placeholder(fr1, r) - } - }; - debug!("provides_universal_region: result = {:?}", result); - result - } - /// Report a specialized error when `FnMut` closures return a reference to a captured variable. /// This function expects `fr` to be local and `outlived_fr` to not be local. /// @@ -817,130 +530,4 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } } - - crate fn free_region_constraint_info( - &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, - borrow_region: RegionVid, - outlived_region: RegionVid, - ) -> (ConstraintCategory, bool, Span, Option) { - let (category, from_closure, span) = self.best_blame_constraint( - &mbcx.body, - borrow_region, - NLLRegionVariableOrigin::FreeRegion, - |r| self.provides_universal_region(r, borrow_region, outlived_region), - ); - - let mut renctx = RegionErrorNamingCtx::new(); - let outlived_fr_name = self.give_region_a_name(mbcx, &mut renctx, outlived_region); - - (category, from_closure, span, outlived_fr_name) - } - - // Finds some region R such that `fr1: R` and `R` is live at - // `elem`. - crate fn find_sub_region_live_at(&self, fr1: RegionVid, elem: Location) -> RegionVid { - debug!("find_sub_region_live_at(fr1={:?}, elem={:?})", fr1, elem); - self.find_constraint_paths_between_regions(fr1, |r| { - // First look for some `r` such that `fr1: r` and `r` is live at `elem` - debug!( - "find_sub_region_live_at: liveness_constraints for {:?} are {:?}", - r, - self.liveness_constraints.region_value_str(r), - ); - self.liveness_constraints.contains(r, elem) - }) - .or_else(|| { - // If we fail to find that, we may find some `r` such that - // `fr1: r` and `r` is a placeholder from some universe - // `fr1` cannot name. This would force `fr1` to be - // `'static`. - self.find_constraint_paths_between_regions(fr1, |r| { - self.cannot_name_placeholder(fr1, r) - }) - }) - .or_else(|| { - // If we fail to find THAT, it may be that `fr1` is a - // placeholder that cannot "fit" into its SCC. In that - // case, there should be some `r` where `fr1: r`, both - // `fr1` and `r` are in the same SCC, and `fr1` is a - // placeholder that `r` cannot name. We can blame that - // edge. - self.find_constraint_paths_between_regions(fr1, |r| { - self.constraint_sccs.scc(fr1) == self.constraint_sccs.scc(r) - && self.cannot_name_placeholder(r, fr1) - }) - }) - .map(|(_path, r)| r) - .unwrap() - } - - // Finds a good span to blame for the fact that `fr1` outlives `fr2`. - crate fn find_outlives_blame_span( - &self, - body: &Body<'tcx>, - fr1: RegionVid, - fr1_origin: NLLRegionVariableOrigin, - fr2: RegionVid, - ) -> (ConstraintCategory, Span) { - let (category, _, span) = self.best_blame_constraint(body, fr1, fr1_origin, |r| { - self.provides_universal_region(r, fr1, fr2) - }); - (category, span) - } - - fn retrieve_closure_constraint_info( - &self, - body: &Body<'tcx>, - constraint: &OutlivesConstraint, - ) -> (ConstraintCategory, bool, Span) { - let loc = match constraint.locations { - Locations::All(span) => return (constraint.category, false, span), - Locations::Single(loc) => loc, - }; - - let opt_span_category = - self.closure_bounds_mapping[&loc].get(&(constraint.sup, constraint.sub)); - opt_span_category.map(|&(category, span)| (category, true, span)).unwrap_or(( - constraint.category, - false, - body.source_info(loc).span, - )) - } - - /// Returns `true` if a closure is inferred to be an `FnMut` closure. - crate fn is_closure_fn_mut(&self, infcx: &InferCtxt<'_, 'tcx>, fr: RegionVid) -> bool { - if let Some(ty::ReFree(free_region)) = self.to_error_region(fr) { - if let ty::BoundRegion::BrEnv = free_region.bound_region { - if let DefiningTy::Closure(def_id, substs) = self.universal_regions.defining_ty { - let closure_kind_ty = substs.as_closure().kind_ty(def_id, infcx.tcx); - return Some(ty::ClosureKind::FnMut) == closure_kind_ty.to_opt_closure_kind(); - } - } - } - - false - } - - /// If `r2` represents a placeholder region, then this returns - /// `true` if `r1` cannot name that placeholder in its - /// value; otherwise, returns `false`. - fn cannot_name_placeholder(&self, r1: RegionVid, r2: RegionVid) -> bool { - debug!("cannot_name_value_of(r1={:?}, r2={:?})", r1, r2); - - match self.definitions[r2].origin { - NLLRegionVariableOrigin::Placeholder(placeholder) => { - let universe1 = self.definitions[r1].universe; - debug!( - "cannot_name_value_of: universe1={:?} placeholder={:?}", - universe1, placeholder - ); - universe1.cannot_name(placeholder.universe) - } - - NLLRegionVariableOrigin::FreeRegion | NLLRegionVariableOrigin::Existential { .. } => { - false - } - } - } } diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index baa1c5b617dca..406b28e1e3256 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -1,3 +1,4 @@ +use std::collections::VecDeque; use std::rc::Rc; use rustc::infer::canonical::QueryOutlivesConstraint; @@ -225,6 +226,13 @@ enum RegionRelationCheckResult { Error, } +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +enum Trace { + StartRegion, + FromOutlivesConstraint(OutlivesConstraint), + NotVisited, +} + impl<'tcx> RegionInferenceContext<'tcx> { /// Creates a new region inference context with a total of /// `num_region_variables` valid inference variables; the first N @@ -1612,6 +1620,393 @@ impl<'tcx> RegionInferenceContext<'tcx> { .unwrap(), } } + + /// We have a constraint `fr1: fr2` that is not satisfied, where + /// `fr2` represents some universal region. Here, `r` is some + /// region where we know that `fr1: r` and this function has the + /// job of determining whether `r` is "to blame" for the fact that + /// `fr1: fr2` is required. + /// + /// This is true under two conditions: + /// + /// - `r == fr2` + /// - `fr2` is `'static` and `r` is some placeholder in a universe + /// that cannot be named by `fr1`; in that case, we will require + /// that `fr1: 'static` because it is the only way to `fr1: r` to + /// be satisfied. (See `add_incompatible_universe`.) + crate fn provides_universal_region( + &self, + r: RegionVid, + fr1: RegionVid, + fr2: RegionVid, + ) -> bool { + debug!("provides_universal_region(r={:?}, fr1={:?}, fr2={:?})", r, fr1, fr2); + let result = { + r == fr2 || { + fr2 == self.universal_regions.fr_static && self.cannot_name_placeholder(fr1, r) + } + }; + debug!("provides_universal_region: result = {:?}", result); + result + } + + /// If `r2` represents a placeholder region, then this returns + /// `true` if `r1` cannot name that placeholder in its + /// value; otherwise, returns `false`. + crate fn cannot_name_placeholder(&self, r1: RegionVid, r2: RegionVid) -> bool { + debug!("cannot_name_value_of(r1={:?}, r2={:?})", r1, r2); + + match self.definitions[r2].origin { + NLLRegionVariableOrigin::Placeholder(placeholder) => { + let universe1 = self.definitions[r1].universe; + debug!( + "cannot_name_value_of: universe1={:?} placeholder={:?}", + universe1, placeholder + ); + universe1.cannot_name(placeholder.universe) + } + + NLLRegionVariableOrigin::FreeRegion | NLLRegionVariableOrigin::Existential { .. } => { + false + } + } + } + + crate fn retrieve_closure_constraint_info( + &self, + body: &Body<'tcx>, + constraint: &OutlivesConstraint, + ) -> (ConstraintCategory, bool, Span) { + let loc = match constraint.locations { + Locations::All(span) => return (constraint.category, false, span), + Locations::Single(loc) => loc, + }; + + let opt_span_category = + self.closure_bounds_mapping[&loc].get(&(constraint.sup, constraint.sub)); + opt_span_category.map(|&(category, span)| (category, true, span)).unwrap_or(( + constraint.category, + false, + body.source_info(loc).span, + )) + } + + /// Finds a good span to blame for the fact that `fr1` outlives `fr2`. + crate fn find_outlives_blame_span( + &self, + body: &Body<'tcx>, + fr1: RegionVid, + fr1_origin: NLLRegionVariableOrigin, + fr2: RegionVid, + ) -> (ConstraintCategory, Span) { + let (category, _, span) = self.best_blame_constraint(body, fr1, fr1_origin, |r| { + self.provides_universal_region(r, fr1, fr2) + }); + (category, span) + } + + /// Walks the graph of constraints (where `'a: 'b` is considered + /// an edge `'a -> 'b`) to find all paths from `from_region` to + /// `to_region`. The paths are accumulated into the vector + /// `results`. The paths are stored as a series of + /// `ConstraintIndex` values -- in other words, a list of *edges*. + /// + /// Returns: a series of constraints as well as the region `R` + /// that passed the target test. + crate fn find_constraint_paths_between_regions( + &self, + from_region: RegionVid, + target_test: impl Fn(RegionVid) -> bool, + ) -> Option<(Vec, RegionVid)> { + let mut context = IndexVec::from_elem(Trace::NotVisited, &self.definitions); + context[from_region] = Trace::StartRegion; + + // Use a deque so that we do a breadth-first search. We will + // stop at the first match, which ought to be the shortest + // path (fewest constraints). + let mut deque = VecDeque::new(); + deque.push_back(from_region); + + while let Some(r) = deque.pop_front() { + debug!( + "find_constraint_paths_between_regions: from_region={:?} r={:?} value={}", + from_region, + r, + self.region_value_str(r), + ); + + // Check if we reached the region we were looking for. If so, + // we can reconstruct the path that led to it and return it. + if target_test(r) { + let mut result = vec![]; + let mut p = r; + loop { + match context[p] { + Trace::NotVisited => { + bug!("found unvisited region {:?} on path to {:?}", p, r) + } + + Trace::FromOutlivesConstraint(c) => { + result.push(c); + p = c.sup; + } + + Trace::StartRegion => { + result.reverse(); + return Some((result, r)); + } + } + } + } + + // Otherwise, walk over the outgoing constraints and + // enqueue any regions we find, keeping track of how we + // reached them. + + // A constraint like `'r: 'x` can come from our constraint + // graph. + let fr_static = self.universal_regions.fr_static; + let outgoing_edges_from_graph = + self.constraint_graph.outgoing_edges(r, &self.constraints, fr_static); + + // Always inline this closure because it can be hot. + let mut handle_constraint = #[inline(always)] + |constraint: OutlivesConstraint| { + debug_assert_eq!(constraint.sup, r); + let sub_region = constraint.sub; + if let Trace::NotVisited = context[sub_region] { + context[sub_region] = Trace::FromOutlivesConstraint(constraint); + deque.push_back(sub_region); + } + }; + + // This loop can be hot. + for constraint in outgoing_edges_from_graph { + handle_constraint(constraint); + } + + // Member constraints can also give rise to `'r: 'x` edges that + // were not part of the graph initially, so watch out for those. + // (But they are extremely rare; this loop is very cold.) + for constraint in self.applied_member_constraints(r) { + let p_c = &self.member_constraints[constraint.member_constraint_index]; + let constraint = OutlivesConstraint { + sup: r, + sub: constraint.min_choice, + locations: Locations::All(p_c.definition_span), + category: ConstraintCategory::OpaqueType, + }; + handle_constraint(constraint); + } + } + + None + } + + /// Finds some region R such that `fr1: R` and `R` is live at `elem`. + crate fn find_sub_region_live_at(&self, fr1: RegionVid, elem: Location) -> RegionVid { + debug!("find_sub_region_live_at(fr1={:?}, elem={:?})", fr1, elem); + self.find_constraint_paths_between_regions(fr1, |r| { + // First look for some `r` such that `fr1: r` and `r` is live at `elem` + debug!( + "find_sub_region_live_at: liveness_constraints for {:?} are {:?}", + r, + self.liveness_constraints.region_value_str(r), + ); + self.liveness_constraints.contains(r, elem) + }) + .or_else(|| { + // If we fail to find that, we may find some `r` such that + // `fr1: r` and `r` is a placeholder from some universe + // `fr1` cannot name. This would force `fr1` to be + // `'static`. + self.find_constraint_paths_between_regions(fr1, |r| { + self.cannot_name_placeholder(fr1, r) + }) + }) + .or_else(|| { + // If we fail to find THAT, it may be that `fr1` is a + // placeholder that cannot "fit" into its SCC. In that + // case, there should be some `r` where `fr1: r`, both + // `fr1` and `r` are in the same SCC, and `fr1` is a + // placeholder that `r` cannot name. We can blame that + // edge. + self.find_constraint_paths_between_regions(fr1, |r| { + self.constraint_sccs.scc(fr1) == self.constraint_sccs.scc(r) + && self.cannot_name_placeholder(r, fr1) + }) + }) + .map(|(_path, r)| r) + .unwrap() + } + + /// Tries to find the best constraint to blame for the fact that + /// `R: from_region`, where `R` is some region that meets + /// `target_test`. This works by following the constraint graph, + /// creating a constraint path that forces `R` to outlive + /// `from_region`, and then finding the best choices within that + /// path to blame. + crate fn best_blame_constraint( + &self, + body: &Body<'tcx>, + from_region: RegionVid, + from_region_origin: NLLRegionVariableOrigin, + target_test: impl Fn(RegionVid) -> bool, + ) -> (ConstraintCategory, bool, Span) { + debug!( + "best_blame_constraint(from_region={:?}, from_region_origin={:?})", + from_region, from_region_origin + ); + + // Find all paths + let (path, target_region) = + self.find_constraint_paths_between_regions(from_region, target_test).unwrap(); + debug!( + "best_blame_constraint: path={:#?}", + path.iter() + .map(|&c| format!( + "{:?} ({:?}: {:?})", + c, + self.constraint_sccs.scc(c.sup), + self.constraint_sccs.scc(c.sub), + )) + .collect::>() + ); + + // Classify each of the constraints along the path. + let mut categorized_path: Vec<(ConstraintCategory, bool, Span)> = path + .iter() + .map(|constraint| { + if constraint.category == ConstraintCategory::ClosureBounds { + self.retrieve_closure_constraint_info(body, &constraint) + } else { + (constraint.category, false, constraint.locations.span(body)) + } + }) + .collect(); + debug!("best_blame_constraint: categorized_path={:#?}", categorized_path); + + // To find the best span to cite, we first try to look for the + // final constraint that is interesting and where the `sup` is + // not unified with the ultimate target region. The reason + // for this is that we have a chain of constraints that lead + // from the source to the target region, something like: + // + // '0: '1 ('0 is the source) + // '1: '2 + // '2: '3 + // '3: '4 + // '4: '5 + // '5: '6 ('6 is the target) + // + // Some of those regions are unified with `'6` (in the same + // SCC). We want to screen those out. After that point, the + // "closest" constraint we have to the end is going to be the + // most likely to be the point where the value escapes -- but + // we still want to screen for an "interesting" point to + // highlight (e.g., a call site or something). + let target_scc = self.constraint_sccs.scc(target_region); + let mut range = 0..path.len(); + + // As noted above, when reporting an error, there is typically a chain of constraints + // leading from some "source" region which must outlive some "target" region. + // In most cases, we prefer to "blame" the constraints closer to the target -- + // but there is one exception. When constraints arise from higher-ranked subtyping, + // we generally prefer to blame the source value, + // as the "target" in this case tends to be some type annotation that the user gave. + // Therefore, if we find that the region origin is some instantiation + // of a higher-ranked region, we start our search from the "source" point + // rather than the "target", and we also tweak a few other things. + // + // An example might be this bit of Rust code: + // + // ```rust + // let x: fn(&'static ()) = |_| {}; + // let y: for<'a> fn(&'a ()) = x; + // ``` + // + // In MIR, this will be converted into a combination of assignments and type ascriptions. + // In particular, the 'static is imposed through a type ascription: + // + // ```rust + // x = ...; + // AscribeUserType(x, fn(&'static ()) + // y = x; + // ``` + // + // We wind up ultimately with constraints like + // + // ```rust + // !a: 'temp1 // from the `y = x` statement + // 'temp1: 'temp2 + // 'temp2: 'static // from the AscribeUserType + // ``` + // + // and here we prefer to blame the source (the y = x statement). + let blame_source = match from_region_origin { + NLLRegionVariableOrigin::FreeRegion + | NLLRegionVariableOrigin::Existential { from_forall: false } => true, + NLLRegionVariableOrigin::Placeholder(_) + | NLLRegionVariableOrigin::Existential { from_forall: true } => false, + }; + + let find_region = |i: &usize| { + let constraint = path[*i]; + + let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup); + + if blame_source { + match categorized_path[*i].0 { + ConstraintCategory::OpaqueType + | ConstraintCategory::Boring + | ConstraintCategory::BoringNoLocation + | ConstraintCategory::Internal => false, + ConstraintCategory::TypeAnnotation + | ConstraintCategory::Return + | ConstraintCategory::Yield => true, + _ => constraint_sup_scc != target_scc, + } + } else { + match categorized_path[*i].0 { + ConstraintCategory::OpaqueType + | ConstraintCategory::Boring + | ConstraintCategory::BoringNoLocation + | ConstraintCategory::Internal => false, + _ => true, + } + } + }; + + let best_choice = + if blame_source { range.rev().find(find_region) } else { range.find(find_region) }; + + debug!( + "best_blame_constraint: best_choice={:?} blame_source={}", + best_choice, blame_source + ); + + if let Some(i) = best_choice { + if let Some(next) = categorized_path.get(i + 1) { + if categorized_path[i].0 == ConstraintCategory::Return + && next.0 == ConstraintCategory::OpaqueType + { + // The return expression is being influenced by the return type being + // impl Trait, point at the return type and not the return expr. + return *next; + } + } + return categorized_path[i]; + } + + // If that search fails, that is.. unusual. Maybe everything + // is in the same SCC or something. In that case, find what + // appears to be the most interesting point to report to the + // user via an even more ad-hoc guess. + categorized_path.sort_by(|p0, p1| p0.0.cmp(&p1.0)); + debug!("`: sorted_path={:#?}", categorized_path); + + *categorized_path.first().unwrap() + } } impl<'tcx> RegionDefinition<'tcx> { From 736348ac41b00657bd8d5d6b8acae1eeef9985c7 Mon Sep 17 00:00:00 2001 From: mark Date: Fri, 20 Dec 2019 23:59:31 -0600 Subject: [PATCH 3/8] Move a bunch of methods to inherent impl MirBorrowckCtxt --- .../diagnostics/explain_borrow.rs | 6 +- .../diagnostics/outlives_suggestion.rs | 4 +- .../borrow_check/diagnostics/region_errors.rs | 144 +++++++++--------- .../borrow_check/diagnostics/region_name.rs | 111 +++++++------- src/librustc_mir/borrow_check/mod.rs | 10 +- 5 files changed, 130 insertions(+), 145 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs index afccf8a0922a7..45c6c845cd156 100644 --- a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs +++ b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs @@ -274,9 +274,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); let mut renctx = RegionErrorNamingCtx::new(); - let outlived_fr_name = - self.nonlexical_regioncx.give_region_a_name(self, &mut renctx, outlived_region); - // TODO(mark-i-m): just return the region and let the caller name it + let outlived_fr_name = self.give_region_a_name(&mut renctx, outlived_region); (category, from_closure, span, outlived_fr_name) } @@ -357,7 +355,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } None => { - if let Some(region) = regioncx.to_error_region_vid(borrow_region_vid) { + if let Some(region) = self.to_error_region_vid(borrow_region_vid) { let (category, from_closure, span, region_name) = self.free_region_constraint_info(borrow_region_vid, region); if let Some(region_name) = region_name { diff --git a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs index 1425c22e461cf..be067fcf4f261 100644 --- a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs +++ b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs @@ -80,9 +80,7 @@ impl OutlivesSuggestionBuilder { renctx: &mut RegionErrorNamingCtx, region: RegionVid, ) -> Option { - mbcx.nonlexical_regioncx - .give_region_a_name(mbcx, renctx, region) - .filter(Self::region_name_is_suggestable) + mbcx.give_region_a_name(renctx, region).filter(Self::region_name_is_suggestable) } /// Compiles a list of all suggestions to be printed in the final big suggestion. diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index f9ec4ccb52a09..b0be82164d876 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -1,8 +1,6 @@ //! Error reporting machinery for lifetime errors. -use rustc::infer::{ - error_reporting::nice_region_error::NiceRegionError, InferCtxt, NLLRegionVariableOrigin, -}; +use rustc::infer::{error_reporting::nice_region_error::NiceRegionError, NLLRegionVariableOrigin}; use rustc::mir::ConstraintCategory; use rustc::ty::{self, RegionVid, Ty}; use rustc_errors::{Applicability, DiagnosticBuilder}; @@ -14,7 +12,7 @@ use crate::util::borrowck_errors; use crate::borrow_check::{ nll::ConstraintDescription, - region_infer::{values::RegionElement, RegionInferenceContext, TypeTest}, + region_infer::{values::RegionElement, TypeTest}, universal_regions::DefiningTy, MirBorrowckCtxt, }; @@ -104,26 +102,28 @@ pub struct ErrorConstraintInfo { pub(super) span: Span, } -impl<'tcx> RegionInferenceContext<'tcx> { +impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// Converts a region inference variable into a `ty::Region` that /// we can use for error reporting. If `r` is universally bound, /// then we use the name that we have on record for it. If `r` is /// existentially bound, then we check its inferred value and try /// to find a good name from that. Returns `None` if we can't find /// one (e.g., this is just some random part of the CFG). - pub fn to_error_region(&self, r: RegionVid) -> Option> { - self.to_error_region_vid(r).and_then(|r| self.definitions[r].external_name) + // TODO(mark-i-m): make this private when we move report_region_errors here... + crate fn to_error_region(&self, r: RegionVid) -> Option> { + self.to_error_region_vid(r) + .and_then(|r| self.nonlexical_regioncx.definitions[r].external_name) } - /// Returns the [RegionVid] corresponding to the region returned by + /// Returns the `RegionVid` corresponding to the region returned by /// `to_error_region`. - pub fn to_error_region_vid(&self, r: RegionVid) -> Option { - if self.universal_regions.is_universal_region(r) { + pub(super) fn to_error_region_vid(&self, r: RegionVid) -> Option { + if self.nonlexical_regioncx.universal_regions.is_universal_region(r) { Some(r) } else { - let r_scc = self.constraint_sccs.scc(r); - let upper_bound = self.universal_upper_bound(r); - if self.scc_values.contains(r_scc, upper_bound) { + let r_scc = self.nonlexical_regioncx.constraint_sccs.scc(r); + let upper_bound = self.nonlexical_regioncx.universal_upper_bound(r); + if self.nonlexical_regioncx.scc_values.contains(r_scc, upper_bound) { self.to_error_region_vid(upper_bound) } else { None @@ -132,11 +132,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { } /// Returns `true` if a closure is inferred to be an `FnMut` closure. - crate fn is_closure_fn_mut(&self, infcx: &InferCtxt<'_, 'tcx>, fr: RegionVid) -> bool { + fn is_closure_fn_mut(&self, fr: RegionVid) -> bool { if let Some(ty::ReFree(free_region)) = self.to_error_region(fr) { if let ty::BoundRegion::BrEnv = free_region.bound_region { - if let DefiningTy::Closure(def_id, substs) = self.universal_regions.defining_ty { - let closure_kind_ty = substs.as_closure().kind_ty(def_id, infcx.tcx); + if let DefiningTy::Closure(def_id, substs) = + self.nonlexical_regioncx.universal_regions.defining_ty + { + let closure_kind_ty = substs.as_closure().kind_ty(def_id, self.infcx.tcx); return Some(ty::ClosureKind::FnMut) == closure_kind_ty.to_opt_closure_kind(); } } @@ -153,34 +155,35 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` /// /// Here we would be invoked with `fr = 'a` and `outlived_fr = `'b`. - pub(in crate::borrow_check) fn report_error<'a>( - &'a self, - mbcx: &MirBorrowckCtxt<'a, 'tcx>, + pub(in crate::borrow_check) fn report_error( + &mut self, fr: RegionVid, fr_origin: NLLRegionVariableOrigin, outlived_fr: RegionVid, outlives_suggestion: &mut OutlivesSuggestionBuilder, renctx: &mut RegionErrorNamingCtx, - ) -> DiagnosticBuilder<'a> { + ) { debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr); - let (category, _, span) = self.best_blame_constraint(&mbcx.body, fr, fr_origin, |r| { - self.provides_universal_region(r, fr, outlived_fr) - }); + let (category, _, span) = + self.nonlexical_regioncx.best_blame_constraint(&self.body, fr, fr_origin, |r| { + self.nonlexical_regioncx.provides_universal_region(r, fr, outlived_fr) + }); debug!("report_error: category={:?} {:?}", category, span); // Check if we can use one of the "nice region errors". if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { - let tables = mbcx.infcx.tcx.typeck_tables_of(mbcx.mir_def_id); - let nice = NiceRegionError::new_from_span(mbcx.infcx, span, o, f, Some(tables)); + let tables = self.infcx.tcx.typeck_tables_of(self.mir_def_id); + let nice = NiceRegionError::new_from_span(self.infcx, span, o, f, Some(tables)); if let Some(diag) = nice.try_report_from_nll() { - return diag; + diag.buffer(&mut self.errors_buffer); + return; } } let (fr_is_local, outlived_fr_is_local): (bool, bool) = ( - self.universal_regions.is_local_free_region(fr), - self.universal_regions.is_local_free_region(outlived_fr), + self.nonlexical_regioncx.universal_regions.is_local_free_region(fr), + self.nonlexical_regioncx.universal_regions.is_local_free_region(outlived_fr), ); debug!( @@ -197,28 +200,30 @@ impl<'tcx> RegionInferenceContext<'tcx> { span, }; - match (category, fr_is_local, outlived_fr_is_local) { - (ConstraintCategory::Return, true, false) if self.is_closure_fn_mut(mbcx.infcx, fr) => { - self.report_fnmut_error(mbcx, &errci, renctx) + let diag = match (category, fr_is_local, outlived_fr_is_local) { + (ConstraintCategory::Return, true, false) if self.is_closure_fn_mut(fr) => { + self.report_fnmut_error(&errci, renctx) } (ConstraintCategory::Assignment, true, false) | (ConstraintCategory::CallArgument, true, false) => { - let mut db = self.report_escaping_data_error(mbcx, &errci, renctx); + let mut db = self.report_escaping_data_error(&errci, renctx); - outlives_suggestion.intermediate_suggestion(mbcx, &errci, renctx, &mut db); + outlives_suggestion.intermediate_suggestion(self, &errci, renctx, &mut db); outlives_suggestion.collect_constraint(fr, outlived_fr); db } _ => { - let mut db = self.report_general_error(mbcx, &errci, renctx); + let mut db = self.report_general_error(&errci, renctx); - outlives_suggestion.intermediate_suggestion(mbcx, &errci, renctx, &mut db); + outlives_suggestion.intermediate_suggestion(self, &errci, renctx, &mut db); outlives_suggestion.collect_constraint(fr, outlived_fr); db } - } + }; + + diag.buffer(&mut self.errors_buffer); } /// Report a specialized error when `FnMut` closures return a reference to a captured variable. @@ -239,13 +244,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` fn report_fnmut_error( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, errci: &ErrorConstraintInfo, renctx: &mut RegionErrorNamingCtx, - ) -> DiagnosticBuilder<'_> { + ) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { outlived_fr, span, .. } = errci; - let mut diag = mbcx + let mut diag = self .infcx .tcx .sess @@ -253,7 +257,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { // We should check if the return type of this closure is in fact a closure - in that // case, we can special case the error further. - let return_type_is_closure = self.universal_regions.unnormalized_output_ty.is_closure(); + let return_type_is_closure = + self.nonlexical_regioncx.universal_regions.unnormalized_output_ty.is_closure(); let message = if return_type_is_closure { "returns a closure that contains a reference to a captured variable, which then \ escapes the closure body" @@ -263,7 +268,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { diag.span_label(*span, message); - match self.give_region_a_name(mbcx, renctx, *outlived_fr).unwrap().source { + match self.give_region_a_name(renctx, *outlived_fr).unwrap().source { RegionNameSource::NamedEarlyBoundRegion(fr_span) | RegionNameSource::NamedFreeRegion(fr_span) | RegionNameSource::SynthesizedFreeEnvRegion(fr_span, _) @@ -300,28 +305,27 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` fn report_escaping_data_error( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, errci: &ErrorConstraintInfo, renctx: &mut RegionErrorNamingCtx, - ) -> DiagnosticBuilder<'_> { + ) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { span, category, .. } = errci; - let fr_name_and_span = self.get_var_name_and_span_for_region( - mbcx.infcx.tcx, - &mbcx.body, - &mbcx.local_names, - &mbcx.upvars, + let fr_name_and_span = self.nonlexical_regioncx.get_var_name_and_span_for_region( + self.infcx.tcx, + &self.body, + &self.local_names, + &self.upvars, errci.fr, ); - let outlived_fr_name_and_span = self.get_var_name_and_span_for_region( - mbcx.infcx.tcx, - &mbcx.body, - &mbcx.local_names, - &mbcx.upvars, + let outlived_fr_name_and_span = self.nonlexical_regioncx.get_var_name_and_span_for_region( + self.infcx.tcx, + &self.body, + &self.local_names, + &self.upvars, errci.outlived_fr, ); - let escapes_from = match self.universal_regions.defining_ty { + let escapes_from = match self.nonlexical_regioncx.universal_regions.defining_ty { DefiningTy::Closure(..) => "closure", DefiningTy::Generator(..) => "generator", DefiningTy::FnDef(..) => "function", @@ -335,14 +339,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { || escapes_from == "const" { return self.report_general_error( - mbcx, &ErrorConstraintInfo { fr_is_local: true, outlived_fr_is_local: false, ..*errci }, renctx, ); } let mut diag = - borrowck_errors::borrowed_data_escapes_closure(mbcx.infcx.tcx, *span, escapes_from); + borrowck_errors::borrowed_data_escapes_closure(self.infcx.tcx, *span, escapes_from); if let Some((Some(outlived_fr_name), outlived_fr_span)) = outlived_fr_name_and_span { diag.span_label( @@ -386,10 +389,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` fn report_general_error( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, errci: &ErrorConstraintInfo, renctx: &mut RegionErrorNamingCtx, - ) -> DiagnosticBuilder<'_> { + ) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { fr, fr_is_local, @@ -401,14 +403,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { } = errci; let mut diag = - mbcx.infcx.tcx.sess.struct_span_err(*span, "lifetime may not live long enough"); + self.infcx.tcx.sess.struct_span_err(*span, "lifetime may not live long enough"); let mir_def_name = - if mbcx.infcx.tcx.is_closure(mbcx.mir_def_id) { "closure" } else { "function" }; + if self.infcx.tcx.is_closure(self.mir_def_id) { "closure" } else { "function" }; - let fr_name = self.give_region_a_name(mbcx, renctx, *fr).unwrap(); + let fr_name = self.give_region_a_name(renctx, *fr).unwrap(); fr_name.highlight_region_name(&mut diag); - let outlived_fr_name = self.give_region_a_name(mbcx, renctx, *outlived_fr).unwrap(); + let outlived_fr_name = self.give_region_a_name(renctx, *outlived_fr).unwrap(); outlived_fr_name.highlight_region_name(&mut diag); match (category, outlived_fr_is_local, fr_is_local) { @@ -435,7 +437,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - self.add_static_impl_trait_suggestion(mbcx.infcx, &mut diag, *fr, fr_name, *outlived_fr); + self.add_static_impl_trait_suggestion(&mut diag, *fr, fr_name, *outlived_fr); diag } @@ -451,8 +453,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` fn add_static_impl_trait_suggestion( &self, - infcx: &InferCtxt<'_, 'tcx>, - diag: &mut DiagnosticBuilder<'_>, + diag: &mut DiagnosticBuilder<'tcx>, fr: RegionVid, // We need to pass `fr_name` - computing it again will label it twice. fr_name: RegionName, @@ -461,11 +462,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { if let (Some(f), Some(ty::RegionKind::ReStatic)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { - if let Some((ty::TyS { kind: ty::Opaque(did, substs), .. }, _)) = infcx + if let Some((ty::TyS { kind: ty::Opaque(did, substs), .. }, _)) = self + .infcx .tcx .is_suitable_region(f) .map(|r| r.def_id) - .map(|id| infcx.tcx.return_type_impl_trait(id)) + .map(|id| self.infcx.tcx.return_type_impl_trait(id)) .unwrap_or(None) { // Check whether or not the impl trait return type is intended to capture @@ -473,8 +475,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { // // eg. check for `impl Trait + 'static` instead of `impl Trait`. let has_static_predicate = { - let predicates_of = infcx.tcx.predicates_of(*did); - let bounds = predicates_of.instantiate(infcx.tcx, substs); + let predicates_of = self.infcx.tcx.predicates_of(*did); + let bounds = predicates_of.instantiate(self.infcx.tcx, substs); let mut found = false; for predicate in bounds.predicates { @@ -502,8 +504,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { diag.help(&format!("consider replacing `{}` with `{}`", fr_name, static_str)); } else { // Otherwise, we should suggest adding a constraint on the return type. - let span = infcx.tcx.def_span(*did); - if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { + let span = self.infcx.tcx.def_span(*did); + if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) { let suggestable_fr_name = if fr_name.was_named() { fr_name.to_string() } else { diff --git a/src/librustc_mir/borrow_check/diagnostics/region_name.rs b/src/librustc_mir/borrow_check/diagnostics/region_name.rs index 734e3861c62de..e5dfb0f70eee2 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_name.rs @@ -2,7 +2,7 @@ use std::fmt::{self, Display}; use rustc::ty::print::RegionHighlightMode; use rustc::ty::subst::{GenericArgKind, SubstsRef}; -use rustc::ty::{self, RegionVid, Ty, TyCtxt}; +use rustc::ty::{self, RegionVid, Ty}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::DiagnosticBuilder; use rustc_hir as hir; @@ -10,10 +10,7 @@ use rustc_hir::def::{DefKind, Res}; use rustc_span::symbol::kw; use rustc_span::{symbol::Symbol, Span, DUMMY_SP}; -use crate::borrow_check::{ - nll::ToRegionVid, region_infer::RegionInferenceContext, universal_regions::DefiningTy, - MirBorrowckCtxt, -}; +use crate::borrow_check::{nll::ToRegionVid, universal_regions::DefiningTy, MirBorrowckCtxt}; /// A name for a particular region used in emitting diagnostics. This name could be a generated /// name like `'1`, a name used by the user like `'a`, or a name like `'static`. @@ -161,7 +158,7 @@ impl Display for RegionName { } } -impl<'tcx> RegionInferenceContext<'tcx> { +impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// Maps from an internal MIR region vid to something that we can /// report to the user. In some cases, the region vids will map /// directly to lifetimes that the user has a name for (e.g., @@ -189,24 +186,23 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// and then return the name `'1` for us to use. crate fn give_region_a_name( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, renctx: &mut RegionErrorNamingCtx, fr: RegionVid, ) -> Option { debug!("give_region_a_name(fr={:?}, counter={:?})", fr, renctx.counter); - assert!(self.universal_regions.is_universal_region(fr)); + assert!(self.nonlexical_regioncx.universal_regions.is_universal_region(fr)); if let Some(value) = renctx.get(&fr) { return Some(value.clone()); } let value = self - .give_name_from_error_region(mbcx, fr, renctx) - .or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(mbcx, fr, renctx)) - .or_else(|| self.give_name_if_anonymous_region_appears_in_upvars(mbcx, fr, renctx)) - .or_else(|| self.give_name_if_anonymous_region_appears_in_output(mbcx, fr, renctx)) - .or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(mbcx, fr, renctx)); + .give_name_from_error_region(fr, renctx) + .or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr, renctx)) + .or_else(|| self.give_name_if_anonymous_region_appears_in_upvars(fr, renctx)) + .or_else(|| self.give_name_if_anonymous_region_appears_in_output(fr, renctx)) + .or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(fr, renctx)); if let Some(ref value) = value { renctx.insert(fr, value.clone()); @@ -222,13 +218,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// named variants. fn give_name_from_error_region( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, fr: RegionVid, renctx: &mut RegionErrorNamingCtx, ) -> Option { let error_region = self.to_error_region(fr)?; - let tcx = mbcx.infcx.tcx; + let tcx = self.infcx.tcx; debug!("give_region_a_name: error_region = {:?}", error_region); match error_region { @@ -276,13 +271,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { } ty::BoundRegion::BrEnv => { - let mir_hir_id = mbcx + let mir_hir_id = self .infcx .tcx .hir() - .as_local_hir_id(mbcx.mir_def_id) + .as_local_hir_id(self.mir_def_id) .expect("non-local mir"); - let def_ty = self.universal_regions.defining_ty; + let def_ty = self.nonlexical_regioncx.universal_regions.defining_ty; if let DefiningTy::Closure(def_id, substs) = def_ty { let args_span = if let hir::ExprKind::Closure(_, _, _, span, _) = @@ -346,38 +341,34 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` fn give_name_if_anonymous_region_appears_in_arguments( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, fr: RegionVid, renctx: &mut RegionErrorNamingCtx, ) -> Option { - let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs(); - let argument_index = self.get_argument_index_for_region(mbcx.infcx.tcx, fr)?; - - let arg_ty = - self.universal_regions.unnormalized_input_tys[implicit_inputs + argument_index]; - if let Some(region_name) = self.give_name_if_we_can_match_hir_ty_from_argument( - mbcx, - fr, - arg_ty, - argument_index, - renctx, - ) { + let implicit_inputs = + self.nonlexical_regioncx.universal_regions.defining_ty.implicit_inputs(); + let argument_index = + self.nonlexical_regioncx.get_argument_index_for_region(self.infcx.tcx, fr)?; + + let arg_ty = self.nonlexical_regioncx.universal_regions.unnormalized_input_tys + [implicit_inputs + argument_index]; + if let Some(region_name) = + self.give_name_if_we_can_match_hir_ty_from_argument(fr, arg_ty, argument_index, renctx) + { return Some(region_name); } - self.give_name_if_we_cannot_match_hir_ty(mbcx, fr, arg_ty, renctx) + self.give_name_if_we_cannot_match_hir_ty(fr, arg_ty, renctx) } fn give_name_if_we_can_match_hir_ty_from_argument( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, needle_fr: RegionVid, argument_ty: Ty<'tcx>, argument_index: usize, renctx: &mut RegionErrorNamingCtx, ) -> Option { - let mir_hir_id = mbcx.infcx.tcx.hir().as_local_hir_id(mbcx.mir_def_id)?; - let fn_decl = mbcx.infcx.tcx.hir().fn_decl_by_hir_id(mir_hir_id)?; + let mir_hir_id = self.infcx.tcx.hir().as_local_hir_id(self.mir_def_id)?; + let fn_decl = self.infcx.tcx.hir().fn_decl_by_hir_id(mir_hir_id)?; let argument_hir_ty: &hir::Ty<'_> = fn_decl.inputs.get(argument_index)?; match argument_hir_ty.kind { // This indicates a variable with no type annotation, like @@ -388,7 +379,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { hir::TyKind::Infer => None, _ => self.give_name_if_we_can_match_hir_ty( - mbcx.infcx.tcx, needle_fr, argument_ty, argument_hir_ty, @@ -410,7 +400,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` fn give_name_if_we_cannot_match_hir_ty( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, needle_fr: RegionVid, argument_ty: Ty<'tcx>, renctx: &mut RegionErrorNamingCtx, @@ -418,7 +407,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { let counter = renctx.counter; let mut highlight = RegionHighlightMode::default(); highlight.highlighting_region_vid(needle_fr, counter); - let type_name = mbcx.infcx.extract_type_name(&argument_ty, Some(highlight)).0; + let type_name = self.infcx.extract_type_name(&argument_ty, Some(highlight)).0; debug!( "give_name_if_we_cannot_match_hir_ty: type_name={:?} needle_fr={:?}", @@ -426,10 +415,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { ); let assigned_region_name = if type_name.find(&format!("'{}", counter)).is_some() { // Only add a label if we can confirm that a region was labelled. - let argument_index = self.get_argument_index_for_region(mbcx.infcx.tcx, needle_fr)?; - let (_, span) = self.get_argument_name_and_span_for_region( - &mbcx.body, - &mbcx.local_names, + let argument_index = self + .nonlexical_regioncx + .get_argument_index_for_region(self.infcx.tcx, needle_fr)?; + let (_, span) = self.nonlexical_regioncx.get_argument_name_and_span_for_region( + &self.body, + &self.local_names, argument_index, ); @@ -470,7 +461,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// to highlighting that closest type instead. fn give_name_if_we_can_match_hir_ty( &self, - tcx: TyCtxt<'tcx>, needle_fr: RegionVid, argument_ty: Ty<'tcx>, argument_hir_ty: &hir::Ty<'_>, @@ -495,7 +485,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { let region_name = renctx.synthesize_region_name(); // Just grab the first character, the `&`. - let source_map = tcx.sess.source_map(); + let source_map = self.infcx.tcx.sess.source_map(); let ampersand_span = source_map.start_point(hir_ty.span); return Some(RegionName { @@ -665,13 +655,16 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// ``` fn give_name_if_anonymous_region_appears_in_upvars( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, fr: RegionVid, renctx: &mut RegionErrorNamingCtx, ) -> Option { - let upvar_index = self.get_upvar_index_for_region(mbcx.infcx.tcx, fr)?; - let (upvar_name, upvar_span) = - self.get_upvar_name_and_span_for_region(mbcx.infcx.tcx, &mbcx.upvars, upvar_index); + let upvar_index = + self.nonlexical_regioncx.get_upvar_index_for_region(self.infcx.tcx, fr)?; + let (upvar_name, upvar_span) = self.nonlexical_regioncx.get_upvar_name_and_span_for_region( + self.infcx.tcx, + &self.upvars, + upvar_index, + ); let region_name = renctx.synthesize_region_name(); Some(RegionName { @@ -686,13 +679,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// or be early bound (named, not in argument). fn give_name_if_anonymous_region_appears_in_output( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, fr: RegionVid, renctx: &mut RegionErrorNamingCtx, ) -> Option { - let tcx = mbcx.infcx.tcx; + let tcx = self.infcx.tcx; - let return_ty = self.universal_regions.unnormalized_output_ty; + let return_ty = self.nonlexical_regioncx.universal_regions.unnormalized_output_ty; debug!("give_name_if_anonymous_region_appears_in_output: return_ty = {:?}", return_ty); if !tcx.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr) { return None; @@ -700,9 +692,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { let mut highlight = RegionHighlightMode::default(); highlight.highlighting_region_vid(fr, renctx.counter); - let type_name = mbcx.infcx.extract_type_name(&return_ty, Some(highlight)).0; + let type_name = self.infcx.extract_type_name(&return_ty, Some(highlight)).0; - let mir_hir_id = tcx.hir().as_local_hir_id(mbcx.mir_def_id).expect("non-local mir"); + let mir_hir_id = tcx.hir().as_local_hir_id(self.mir_def_id).expect("non-local mir"); let (return_span, mir_description) = match tcx.hir().get(mir_hir_id) { hir::Node::Expr(hir::Expr { @@ -719,7 +711,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { kind: hir::ImplItemKind::Method(method_sig, _), .. }) => (method_sig.decl.output.span(), ""), - _ => (mbcx.body.span, ""), + _ => (self.body.span, ""), }; Some(RegionName { @@ -737,16 +729,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { fn give_name_if_anonymous_region_appears_in_yield_ty( &self, - mbcx: &MirBorrowckCtxt<'_, 'tcx>, fr: RegionVid, renctx: &mut RegionErrorNamingCtx, ) -> Option { // Note: generators from `async fn` yield `()`, so we don't have to // worry about them here. - let yield_ty = self.universal_regions.yield_ty?; + let yield_ty = self.nonlexical_regioncx.universal_regions.yield_ty?; debug!("give_name_if_anonymous_region_appears_in_yield_ty: yield_ty = {:?}", yield_ty,); - let tcx = mbcx.infcx.tcx; + let tcx = self.infcx.tcx; if !tcx.any_free_region_meets(&yield_ty, |r| r.to_region_vid() == fr) { return None; @@ -754,15 +745,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { let mut highlight = RegionHighlightMode::default(); highlight.highlighting_region_vid(fr, renctx.counter); - let type_name = mbcx.infcx.extract_type_name(&yield_ty, Some(highlight)).0; + let type_name = self.infcx.extract_type_name(&yield_ty, Some(highlight)).0; - let mir_hir_id = tcx.hir().as_local_hir_id(mbcx.mir_def_id).expect("non-local mir"); + let mir_hir_id = tcx.hir().as_local_hir_id(self.mir_def_id).expect("non-local mir"); let yield_span = match tcx.hir().get(mir_hir_id) { hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, _, _, span, _), .. }) => (tcx.sess.source_map().end_point(*span)), - _ => mbcx.body.span, + _ => self.body.span, }; debug!( diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index fc1b723c27120..209df021ba0d1 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1471,7 +1471,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are // buffered in the `MirBorrowckCtxt`. - // FIXME(mark-i-m): Would be great to get rid of the naming context. + // TODO(mark-i-m): Would be great to get rid of the naming context. let mut region_naming = RegionErrorNamingCtx::new(); let mut outlives_suggestion = OutlivesSuggestionBuilder::default(); @@ -1479,8 +1479,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { match nll_error { RegionErrorKind::TypeTestError { type_test } => { // Try to convert the lower-bound region into something named we can print for the user. - let lower_bound_region = - self.nonlexical_regioncx.to_error_region(type_test.lower_bound); + let lower_bound_region = self.to_error_region(type_test.lower_bound); // Skip duplicate-ish errors. let type_test_span = type_test.locations.span(&self.body); @@ -1557,16 +1556,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => { if is_reported { - let db = self.nonlexical_regioncx.report_error( - self, + self.report_error( longer_fr, fr_origin, shorter_fr, &mut outlives_suggestion, &mut region_naming, ); - - db.buffer(&mut self.errors_buffer); } else { // We only report the first error, so as not to overwhelm the user. See // `RegRegionErrorKind` docs. From 786db7399fa8214aa2412fbeb18033524bfa8b34 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Sat, 28 Dec 2019 19:28:50 -0600 Subject: [PATCH 4/8] Move report_region_errors to region_errors.rs --- .../borrow_check/diagnostics/region_errors.rs | 126 +++++++++++++++++- src/librustc_mir/borrow_check/mod.rs | 123 +---------------- 2 files changed, 125 insertions(+), 124 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index b0be82164d876..5980ff0073253 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -1,6 +1,8 @@ //! Error reporting machinery for lifetime errors. -use rustc::infer::{error_reporting::nice_region_error::NiceRegionError, NLLRegionVariableOrigin}; +use rustc::infer::{ + error_reporting::nice_region_error::NiceRegionError, opaque_types, NLLRegionVariableOrigin, +}; use rustc::mir::ConstraintCategory; use rustc::ty::{self, RegionVid, Ty}; use rustc_errors::{Applicability, DiagnosticBuilder}; @@ -109,8 +111,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// existentially bound, then we check its inferred value and try /// to find a good name from that. Returns `None` if we can't find /// one (e.g., this is just some random part of the CFG). - // TODO(mark-i-m): make this private when we move report_region_errors here... - crate fn to_error_region(&self, r: RegionVid) -> Option> { + pub(super) fn to_error_region(&self, r: RegionVid) -> Option> { self.to_error_region_vid(r) .and_then(|r| self.nonlexical_regioncx.definitions[r].external_name) } @@ -147,6 +148,125 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { false } + /// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`. + pub(in crate::borrow_check) fn report_region_errors(&mut self, nll_errors: RegionErrors<'tcx>) { + // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are + // buffered in the `MirBorrowckCtxt`. + + // TODO(mark-i-m): Would be great to get rid of the naming context. + let mut region_naming = RegionErrorNamingCtx::new(); + let mut outlives_suggestion = OutlivesSuggestionBuilder::default(); + + for nll_error in nll_errors.into_iter() { + match nll_error { + RegionErrorKind::TypeTestError { type_test } => { + // Try to convert the lower-bound region into something named we can print for the user. + let lower_bound_region = self.to_error_region(type_test.lower_bound); + + // Skip duplicate-ish errors. + let type_test_span = type_test.locations.span(&self.body); + + if let Some(lower_bound_region) = lower_bound_region { + let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); + self.infcx + .construct_generic_bound_failure( + region_scope_tree, + type_test_span, + None, + type_test.generic_kind, + lower_bound_region, + ) + .buffer(&mut self.errors_buffer); + } else { + // FIXME. We should handle this case better. It + // indicates that we have e.g., some region variable + // whose value is like `'a+'b` where `'a` and `'b` are + // distinct unrelated univesal regions that are not + // known to outlive one another. It'd be nice to have + // some examples where this arises to decide how best + // to report it; we could probably handle it by + // iterating over the universal regions and reporting + // an error that multiple bounds are required. + self.infcx + .tcx + .sess + .struct_span_err( + type_test_span, + &format!("`{}` does not live long enough", type_test.generic_kind), + ) + .buffer(&mut self.errors_buffer); + } + } + + RegionErrorKind::UnexpectedHiddenRegion { + opaque_type_def_id, + hidden_ty, + member_region, + } => { + let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); + opaque_types::unexpected_hidden_region_diagnostic( + self.infcx.tcx, + Some(region_scope_tree), + opaque_type_def_id, + hidden_ty, + member_region, + ) + .buffer(&mut self.errors_buffer); + } + + RegionErrorKind::BoundUniversalRegionError { + longer_fr, + fr_origin, + error_element, + } => { + let error_region = + self.nonlexical_regioncx.region_from_element(longer_fr, error_element); + + // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. + let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span( + &self.body, + longer_fr, + fr_origin, + error_region, + ); + + // FIXME: improve this error message + self.infcx + .tcx + .sess + .struct_span_err(span, "higher-ranked subtype error") + .buffer(&mut self.errors_buffer); + } + + RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => { + if is_reported { + self.report_error( + longer_fr, + fr_origin, + shorter_fr, + &mut outlives_suggestion, + &mut region_naming, + ); + } else { + // We only report the first error, so as not to overwhelm the user. See + // `RegRegionErrorKind` docs. + // + // FIXME: currently we do nothing with these, but perhaps we can do better? + // FIXME: try collecting these constraints on the outlives suggestion + // builder. Does it make the suggestions any better? + debug!( + "Unreported region error: can't prove that {:?}: {:?}", + longer_fr, shorter_fr + ); + } + } + } + } + + // Emit one outlives suggestions for each MIR def we borrowck + outlives_suggestion.add_suggestion(self, &mut region_naming); + } + /// Report an error because the universal region `fr` was required to outlive /// `outlived_fr` but it is not known to do so. For example: /// diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 209df021ba0d1..22037ac6c81b2 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1,6 +1,6 @@ //! This query borrow-checks the MIR to (further) ensure it is not broken. -use rustc::infer::{opaque_types, InferCtxt}; +use rustc::infer::InferCtxt; use rustc::lint::builtin::MUTABLE_BORROW_RESERVATION_CONFLICT; use rustc::lint::builtin::UNUSED_MUT; use rustc::mir::{ @@ -39,9 +39,7 @@ use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; use crate::transform::MirSource; -use self::diagnostics::{ - AccessKind, OutlivesSuggestionBuilder, RegionErrorKind, RegionErrorNamingCtx, RegionErrors, -}; +use self::diagnostics::AccessKind; use self::flows::Flows; use self::location::LocationTable; use self::prefixes::PrefixSet; @@ -1465,123 +1463,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // initial reservation. } } - - /// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`. - fn report_region_errors(&mut self, nll_errors: RegionErrors<'tcx>) { - // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are - // buffered in the `MirBorrowckCtxt`. - - // TODO(mark-i-m): Would be great to get rid of the naming context. - let mut region_naming = RegionErrorNamingCtx::new(); - let mut outlives_suggestion = OutlivesSuggestionBuilder::default(); - - for nll_error in nll_errors.into_iter() { - match nll_error { - RegionErrorKind::TypeTestError { type_test } => { - // Try to convert the lower-bound region into something named we can print for the user. - let lower_bound_region = self.to_error_region(type_test.lower_bound); - - // Skip duplicate-ish errors. - let type_test_span = type_test.locations.span(&self.body); - - if let Some(lower_bound_region) = lower_bound_region { - let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); - self.infcx - .construct_generic_bound_failure( - region_scope_tree, - type_test_span, - None, - type_test.generic_kind, - lower_bound_region, - ) - .buffer(&mut self.errors_buffer); - } else { - // FIXME. We should handle this case better. It indicates that we have - // e.g., some region variable whose value is like `'a+'b` where `'a` and - // `'b` are distinct unrelated univesal regions that are not known to - // outlive one another. It'd be nice to have some examples where this - // arises to decide how best to report it; we could probably handle it by - // iterating over the universal regions and reporting an error that - // multiple bounds are required. - self.infcx - .tcx - .sess - .struct_span_err( - type_test_span, - &format!("`{}` does not live long enough", type_test.generic_kind), - ) - .buffer(&mut self.errors_buffer); - } - } - - RegionErrorKind::UnexpectedHiddenRegion { - opaque_type_def_id, - hidden_ty, - member_region, - } => { - let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); - opaque_types::unexpected_hidden_region_diagnostic( - self.infcx.tcx, - Some(region_scope_tree), - opaque_type_def_id, - hidden_ty, - member_region, - ) - .buffer(&mut self.errors_buffer); - } - - RegionErrorKind::BoundUniversalRegionError { - longer_fr, - fr_origin, - error_element, - } => { - let error_region = - self.nonlexical_regioncx.region_from_element(longer_fr, error_element); - - // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. - let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span( - &self.body, - longer_fr, - fr_origin, - error_region, - ); - - // FIXME: improve this error message - self.infcx - .tcx - .sess - .struct_span_err(span, "higher-ranked subtype error") - .buffer(&mut self.errors_buffer); - } - - RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => { - if is_reported { - self.report_error( - longer_fr, - fr_origin, - shorter_fr, - &mut outlives_suggestion, - &mut region_naming, - ); - } else { - // We only report the first error, so as not to overwhelm the user. See - // `RegRegionErrorKind` docs. - // - // FIXME: currently we do nothing with these, but perhaps we can do better? - // FIXME: try collecting these constraints on the outlives suggestion - // builder. Does it make the suggestions any better? - debug!( - "Unreported region error: can't prove that {:?}: {:?}", - longer_fr, shorter_fr - ); - } - } - } - } - - // Emit one outlives suggestions for each MIR def we borrowck - outlives_suggestion.add_suggestion(self, &mut region_naming); - } } impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { From 66c5d5b70697dd98468a88b455dc99681c810db1 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Sat, 28 Dec 2019 19:35:18 -0600 Subject: [PATCH 5/8] Privatize the fields of RegionInferenceContext --- .../borrow_check/diagnostics/region_errors.rs | 18 +++--- .../borrow_check/diagnostics/region_name.rs | 12 ++-- .../borrow_check/diagnostics/var_name.rs | 16 ++--- .../borrow_check/region_infer/mod.rs | 63 ++++++++++++++----- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 5980ff0073253..dcba790ad7e48 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -113,18 +113,18 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// one (e.g., this is just some random part of the CFG). pub(super) fn to_error_region(&self, r: RegionVid) -> Option> { self.to_error_region_vid(r) - .and_then(|r| self.nonlexical_regioncx.definitions[r].external_name) + .and_then(|r| self.nonlexical_regioncx.region_definition(r).external_name) } /// Returns the `RegionVid` corresponding to the region returned by /// `to_error_region`. pub(super) fn to_error_region_vid(&self, r: RegionVid) -> Option { - if self.nonlexical_regioncx.universal_regions.is_universal_region(r) { + if self.nonlexical_regioncx.universal_regions().is_universal_region(r) { Some(r) } else { - let r_scc = self.nonlexical_regioncx.constraint_sccs.scc(r); let upper_bound = self.nonlexical_regioncx.universal_upper_bound(r); - if self.nonlexical_regioncx.scc_values.contains(r_scc, upper_bound) { + + if self.nonlexical_regioncx.upper_bound_in_region_scc(r, upper_bound) { self.to_error_region_vid(upper_bound) } else { None @@ -137,7 +137,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { if let Some(ty::ReFree(free_region)) = self.to_error_region(fr) { if let ty::BoundRegion::BrEnv = free_region.bound_region { if let DefiningTy::Closure(def_id, substs) = - self.nonlexical_regioncx.universal_regions.defining_ty + self.nonlexical_regioncx.universal_regions().defining_ty { let closure_kind_ty = substs.as_closure().kind_ty(def_id, self.infcx.tcx); return Some(ty::ClosureKind::FnMut) == closure_kind_ty.to_opt_closure_kind(); @@ -302,8 +302,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } let (fr_is_local, outlived_fr_is_local): (bool, bool) = ( - self.nonlexical_regioncx.universal_regions.is_local_free_region(fr), - self.nonlexical_regioncx.universal_regions.is_local_free_region(outlived_fr), + self.nonlexical_regioncx.universal_regions().is_local_free_region(fr), + self.nonlexical_regioncx.universal_regions().is_local_free_region(outlived_fr), ); debug!( @@ -378,7 +378,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // We should check if the return type of this closure is in fact a closure - in that // case, we can special case the error further. let return_type_is_closure = - self.nonlexical_regioncx.universal_regions.unnormalized_output_ty.is_closure(); + self.nonlexical_regioncx.universal_regions().unnormalized_output_ty.is_closure(); let message = if return_type_is_closure { "returns a closure that contains a reference to a captured variable, which then \ escapes the closure body" @@ -445,7 +445,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { errci.outlived_fr, ); - let escapes_from = match self.nonlexical_regioncx.universal_regions.defining_ty { + let escapes_from = match self.nonlexical_regioncx.universal_regions().defining_ty { DefiningTy::Closure(..) => "closure", DefiningTy::Generator(..) => "generator", DefiningTy::FnDef(..) => "function", diff --git a/src/librustc_mir/borrow_check/diagnostics/region_name.rs b/src/librustc_mir/borrow_check/diagnostics/region_name.rs index e5dfb0f70eee2..b6bfa30b4dcdf 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_name.rs @@ -191,7 +191,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ) -> Option { debug!("give_region_a_name(fr={:?}, counter={:?})", fr, renctx.counter); - assert!(self.nonlexical_regioncx.universal_regions.is_universal_region(fr)); + assert!(self.nonlexical_regioncx.universal_regions().is_universal_region(fr)); if let Some(value) = renctx.get(&fr) { return Some(value.clone()); @@ -277,7 +277,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { .hir() .as_local_hir_id(self.mir_def_id) .expect("non-local mir"); - let def_ty = self.nonlexical_regioncx.universal_regions.defining_ty; + let def_ty = self.nonlexical_regioncx.universal_regions().defining_ty; if let DefiningTy::Closure(def_id, substs) = def_ty { let args_span = if let hir::ExprKind::Closure(_, _, _, span, _) = @@ -345,11 +345,11 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { renctx: &mut RegionErrorNamingCtx, ) -> Option { let implicit_inputs = - self.nonlexical_regioncx.universal_regions.defining_ty.implicit_inputs(); + self.nonlexical_regioncx.universal_regions().defining_ty.implicit_inputs(); let argument_index = self.nonlexical_regioncx.get_argument_index_for_region(self.infcx.tcx, fr)?; - let arg_ty = self.nonlexical_regioncx.universal_regions.unnormalized_input_tys + let arg_ty = self.nonlexical_regioncx.universal_regions().unnormalized_input_tys [implicit_inputs + argument_index]; if let Some(region_name) = self.give_name_if_we_can_match_hir_ty_from_argument(fr, arg_ty, argument_index, renctx) @@ -684,7 +684,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ) -> Option { let tcx = self.infcx.tcx; - let return_ty = self.nonlexical_regioncx.universal_regions.unnormalized_output_ty; + let return_ty = self.nonlexical_regioncx.universal_regions().unnormalized_output_ty; debug!("give_name_if_anonymous_region_appears_in_output: return_ty = {:?}", return_ty); if !tcx.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr) { return None; @@ -734,7 +734,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ) -> Option { // Note: generators from `async fn` yield `()`, so we don't have to // worry about them here. - let yield_ty = self.nonlexical_regioncx.universal_regions.yield_ty?; + let yield_ty = self.nonlexical_regioncx.universal_regions().yield_ty?; debug!("give_name_if_anonymous_region_appears_in_yield_ty: yield_ty = {:?}", yield_ty,); let tcx = self.infcx.tcx; diff --git a/src/librustc_mir/borrow_check/diagnostics/var_name.rs b/src/librustc_mir/borrow_check/diagnostics/var_name.rs index 9e3d2a7d5f63c..5f3585ce8b119 100644 --- a/src/librustc_mir/borrow_check/diagnostics/var_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/var_name.rs @@ -16,7 +16,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fr: RegionVid, ) -> Option<(Option, Span)> { debug!("get_var_name_and_span_for_region(fr={:?})", fr); - assert!(self.universal_regions.is_universal_region(fr)); + assert!(self.universal_regions().is_universal_region(fr)); debug!("get_var_name_and_span_for_region: attempting upvar"); self.get_upvar_index_for_region(tcx, fr) @@ -35,7 +35,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// Search the upvars (if any) to find one that references fr. Return its index. crate fn get_upvar_index_for_region(&self, tcx: TyCtxt<'tcx>, fr: RegionVid) -> Option { let upvar_index = - self.universal_regions.defining_ty.upvar_tys(tcx).position(|upvar_ty| { + self.universal_regions().defining_ty.upvar_tys(tcx).position(|upvar_ty| { debug!("get_upvar_index_for_region: upvar_ty={:?}", upvar_ty); tcx.any_free_region_meets(&upvar_ty, |r| { let r = r.to_region_vid(); @@ -44,7 +44,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { }) })?; - let upvar_ty = self.universal_regions.defining_ty.upvar_tys(tcx).nth(upvar_index); + let upvar_ty = self.universal_regions().defining_ty.upvar_tys(tcx).nth(upvar_index); debug!( "get_upvar_index_for_region: found {:?} in upvar {} which has type {:?}", @@ -85,9 +85,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { tcx: TyCtxt<'tcx>, fr: RegionVid, ) -> Option { - let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs(); + let implicit_inputs = self.universal_regions().defining_ty.implicit_inputs(); let argument_index = - self.universal_regions.unnormalized_input_tys.iter().skip(implicit_inputs).position( + self.universal_regions().unnormalized_input_tys.iter().skip(implicit_inputs).position( |arg_ty| { debug!("get_argument_index_for_region: arg_ty = {:?}", arg_ty); tcx.any_free_region_meets(arg_ty, |r| r.to_region_vid() == fr) @@ -96,7 +96,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { debug!( "get_argument_index_for_region: found {:?} in argument {} which has type {:?}", - fr, argument_index, self.universal_regions.unnormalized_input_tys[argument_index], + fr, + argument_index, + self.universal_regions().unnormalized_input_tys[argument_index], ); Some(argument_index) @@ -110,7 +112,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { local_names: &IndexVec>, argument_index: usize, ) -> (Option, Span) { - let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs(); + let implicit_inputs = self.universal_regions().defining_ty.implicit_inputs(); let argument_local = Local::new(implicit_inputs + argument_index + 1); debug!("get_argument_name_and_span_for_region: argument_local={:?}", argument_local); diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 406b28e1e3256..359f75cac8376 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -44,49 +44,48 @@ pub struct RegionInferenceContext<'tcx> { /// variables are identified by their index (`RegionVid`). The /// definition contains information about where the region came /// from as well as its final inferred value. - pub(in crate::borrow_check) definitions: IndexVec>, + definitions: IndexVec>, /// The liveness constraints added to each region. For most /// regions, these start out empty and steadily grow, though for /// each universally quantified region R they start out containing /// the entire CFG and `end(R)`. - pub(in crate::borrow_check) liveness_constraints: LivenessValues, + liveness_constraints: LivenessValues, /// The outlives constraints computed by the type-check. - pub(in crate::borrow_check) constraints: Rc, + constraints: Rc, /// The constraint-set, but in graph form, making it easy to traverse /// the constraints adjacent to a particular region. Used to construct /// the SCC (see `constraint_sccs`) and for error reporting. - pub(in crate::borrow_check) constraint_graph: Rc, + constraint_graph: Rc, /// The SCC computed from `constraints` and the constraint /// graph. We have an edge from SCC A to SCC B if `A: B`. Used to /// compute the values of each region. - pub(in crate::borrow_check) constraint_sccs: Rc>, + constraint_sccs: Rc>, /// Reverse of the SCC constraint graph -- i.e., an edge `A -> B` /// exists if `B: A`. Computed lazilly. - pub(in crate::borrow_check) rev_constraint_graph: Option>>, + rev_constraint_graph: Option>>, /// The "R0 member of [R1..Rn]" constraints, indexed by SCC. - pub(in crate::borrow_check) member_constraints: - Rc>, + member_constraints: Rc>, /// Records the member constraints that we applied to each scc. /// This is useful for error reporting. Once constraint /// propagation is done, this vector is sorted according to /// `member_region_scc`. - pub(in crate::borrow_check) member_constraints_applied: Vec, + member_constraints_applied: Vec, /// Map closure bounds to a `Span` that should be used for error reporting. - pub(in crate::borrow_check) closure_bounds_mapping: + closure_bounds_mapping: FxHashMap>, /// Contains the minimum universe of any variable within the same /// SCC. We will ensure that no SCC contains values that are not /// visible from this index. - pub(in crate::borrow_check) scc_universes: IndexVec, + scc_universes: IndexVec, /// Contains a "representative" from each SCC. This will be the /// minimal RegionVid belonging to that universe. It is used as a @@ -95,23 +94,23 @@ pub struct RegionInferenceContext<'tcx> { /// of its SCC and be sure that -- if they have the same repr -- /// they *must* be equal (though not having the same repr does not /// mean they are unequal). - pub(in crate::borrow_check) scc_representatives: IndexVec, + scc_representatives: IndexVec, /// The final inferred values of the region variables; we compute /// one value per SCC. To get the value for any given *region*, /// you first find which scc it is a part of. - pub(in crate::borrow_check) scc_values: RegionValues, + scc_values: RegionValues, /// Type constraints that we check after solving. - pub(in crate::borrow_check) type_tests: Vec>, + type_tests: Vec>, /// Information about the universally quantified regions in scope /// on this function. - pub(in crate::borrow_check) universal_regions: Rc>, + universal_regions: Rc>, /// Information about how the universally quantified regions in /// scope on this function relate to one another. - pub(in crate::borrow_check) universal_region_relations: Rc>, + universal_region_relations: Rc>, } /// Each time that `apply_member_constraint` is successful, it appends @@ -1840,6 +1839,38 @@ impl<'tcx> RegionInferenceContext<'tcx> { .unwrap() } + /// Get the region outlived by `longer_fr` and live at `element`. + crate fn region_from_element(&self, longer_fr: RegionVid, element: RegionElement) -> RegionVid { + match element { + RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l), + RegionElement::RootUniversalRegion(r) => r, + RegionElement::PlaceholderRegion(error_placeholder) => self + .definitions + .iter_enumerated() + .filter_map(|(r, definition)| match definition.origin { + NLLRegionVariableOrigin::Placeholder(p) if p == error_placeholder => Some(r), + _ => None, + }) + .next() + .unwrap(), + } + } + + /// Get the region definition of `r`. + crate fn region_definition(&self, r: RegionVid) -> &RegionDefinition<'tcx> { + &self.definitions[r] + } + + /// Check if the SCC of `r` contains `upper`. + crate fn upper_bound_in_region_scc(&self, r: RegionVid, upper: RegionVid) -> bool { + let r_scc = self.constraint_sccs.scc(r); + self.scc_values.contains(r_scc, upper) + } + + crate fn universal_regions(&self) -> Rc> { + self.universal_regions.clone() + } + /// Tries to find the best constraint to blame for the fact that /// `R: from_region`, where `R` is some region that meets /// `target_test`. This works by following the constraint graph, From 234b9301087ac21dd1dec5e6cad59cb76346ee6c Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Sat, 28 Dec 2019 20:02:20 -0600 Subject: [PATCH 6/8] rename nonlexical_regioncx -> regioncx --- .../diagnostics/explain_borrow.rs | 15 +++------ .../borrow_check/diagnostics/region_errors.rs | 32 +++++++++---------- .../borrow_check/diagnostics/region_name.rs | 28 +++++++--------- src/librustc_mir/borrow_check/mod.rs | 7 ++-- 4 files changed, 34 insertions(+), 48 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs index 45c6c845cd156..ca5cf5bc741a0 100644 --- a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs +++ b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs @@ -260,17 +260,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { borrow_region: RegionVid, outlived_region: RegionVid, ) -> (ConstraintCategory, bool, Span, Option) { - let (category, from_closure, span) = self.nonlexical_regioncx.best_blame_constraint( + let (category, from_closure, span) = self.regioncx.best_blame_constraint( &self.body, borrow_region, NLLRegionVariableOrigin::FreeRegion, - |r| { - self.nonlexical_regioncx.provides_universal_region( - r, - borrow_region, - outlived_region, - ) - }, + |r| self.regioncx.provides_universal_region(r, borrow_region, outlived_region), ); let mut renctx = RegionErrorNamingCtx::new(); @@ -303,15 +297,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { location, borrow, kind_place ); - let regioncx = &self.nonlexical_regioncx; + let regioncx = &self.regioncx; let body: &Body<'_> = &self.body; let tcx = self.infcx.tcx; let borrow_region_vid = borrow.region; debug!("explain_why_borrow_contains_point: borrow_region_vid={:?}", borrow_region_vid); - let region_sub = - self.nonlexical_regioncx.find_sub_region_live_at(borrow_region_vid, location); + let region_sub = self.regioncx.find_sub_region_live_at(borrow_region_vid, location); debug!("explain_why_borrow_contains_point: region_sub={:?}", region_sub); match find_use::find(body, regioncx, tcx, region_sub, location) { diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index dcba790ad7e48..f3dc8e72e3016 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -112,19 +112,18 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// to find a good name from that. Returns `None` if we can't find /// one (e.g., this is just some random part of the CFG). pub(super) fn to_error_region(&self, r: RegionVid) -> Option> { - self.to_error_region_vid(r) - .and_then(|r| self.nonlexical_regioncx.region_definition(r).external_name) + self.to_error_region_vid(r).and_then(|r| self.regioncx.region_definition(r).external_name) } /// Returns the `RegionVid` corresponding to the region returned by /// `to_error_region`. pub(super) fn to_error_region_vid(&self, r: RegionVid) -> Option { - if self.nonlexical_regioncx.universal_regions().is_universal_region(r) { + if self.regioncx.universal_regions().is_universal_region(r) { Some(r) } else { - let upper_bound = self.nonlexical_regioncx.universal_upper_bound(r); + let upper_bound = self.regioncx.universal_upper_bound(r); - if self.nonlexical_regioncx.upper_bound_in_region_scc(r, upper_bound) { + if self.regioncx.upper_bound_in_region_scc(r, upper_bound) { self.to_error_region_vid(upper_bound) } else { None @@ -137,7 +136,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { if let Some(ty::ReFree(free_region)) = self.to_error_region(fr) { if let ty::BoundRegion::BrEnv = free_region.bound_region { if let DefiningTy::Closure(def_id, substs) = - self.nonlexical_regioncx.universal_regions().defining_ty + self.regioncx.universal_regions().defining_ty { let closure_kind_ty = substs.as_closure().kind_ty(def_id, self.infcx.tcx); return Some(ty::ClosureKind::FnMut) == closure_kind_ty.to_opt_closure_kind(); @@ -219,11 +218,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { fr_origin, error_element, } => { - let error_region = - self.nonlexical_regioncx.region_from_element(longer_fr, error_element); + let error_region = self.regioncx.region_from_element(longer_fr, error_element); // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. - let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span( + let (_, span) = self.regioncx.find_outlives_blame_span( &self.body, longer_fr, fr_origin, @@ -286,8 +284,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr); let (category, _, span) = - self.nonlexical_regioncx.best_blame_constraint(&self.body, fr, fr_origin, |r| { - self.nonlexical_regioncx.provides_universal_region(r, fr, outlived_fr) + self.regioncx.best_blame_constraint(&self.body, fr, fr_origin, |r| { + self.regioncx.provides_universal_region(r, fr, outlived_fr) }); debug!("report_error: category={:?} {:?}", category, span); @@ -302,8 +300,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } let (fr_is_local, outlived_fr_is_local): (bool, bool) = ( - self.nonlexical_regioncx.universal_regions().is_local_free_region(fr), - self.nonlexical_regioncx.universal_regions().is_local_free_region(outlived_fr), + self.regioncx.universal_regions().is_local_free_region(fr), + self.regioncx.universal_regions().is_local_free_region(outlived_fr), ); debug!( @@ -378,7 +376,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // We should check if the return type of this closure is in fact a closure - in that // case, we can special case the error further. let return_type_is_closure = - self.nonlexical_regioncx.universal_regions().unnormalized_output_ty.is_closure(); + self.regioncx.universal_regions().unnormalized_output_ty.is_closure(); let message = if return_type_is_closure { "returns a closure that contains a reference to a captured variable, which then \ escapes the closure body" @@ -430,14 +428,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { span, category, .. } = errci; - let fr_name_and_span = self.nonlexical_regioncx.get_var_name_and_span_for_region( + let fr_name_and_span = self.regioncx.get_var_name_and_span_for_region( self.infcx.tcx, &self.body, &self.local_names, &self.upvars, errci.fr, ); - let outlived_fr_name_and_span = self.nonlexical_regioncx.get_var_name_and_span_for_region( + let outlived_fr_name_and_span = self.regioncx.get_var_name_and_span_for_region( self.infcx.tcx, &self.body, &self.local_names, @@ -445,7 +443,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { errci.outlived_fr, ); - let escapes_from = match self.nonlexical_regioncx.universal_regions().defining_ty { + let escapes_from = match self.regioncx.universal_regions().defining_ty { DefiningTy::Closure(..) => "closure", DefiningTy::Generator(..) => "generator", DefiningTy::FnDef(..) => "function", diff --git a/src/librustc_mir/borrow_check/diagnostics/region_name.rs b/src/librustc_mir/borrow_check/diagnostics/region_name.rs index b6bfa30b4dcdf..7ce29713240ce 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_name.rs @@ -191,7 +191,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ) -> Option { debug!("give_region_a_name(fr={:?}, counter={:?})", fr, renctx.counter); - assert!(self.nonlexical_regioncx.universal_regions().is_universal_region(fr)); + assert!(self.regioncx.universal_regions().is_universal_region(fr)); if let Some(value) = renctx.get(&fr) { return Some(value.clone()); @@ -277,7 +277,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { .hir() .as_local_hir_id(self.mir_def_id) .expect("non-local mir"); - let def_ty = self.nonlexical_regioncx.universal_regions().defining_ty; + let def_ty = self.regioncx.universal_regions().defining_ty; if let DefiningTy::Closure(def_id, substs) = def_ty { let args_span = if let hir::ExprKind::Closure(_, _, _, span, _) = @@ -344,12 +344,10 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { fr: RegionVid, renctx: &mut RegionErrorNamingCtx, ) -> Option { - let implicit_inputs = - self.nonlexical_regioncx.universal_regions().defining_ty.implicit_inputs(); - let argument_index = - self.nonlexical_regioncx.get_argument_index_for_region(self.infcx.tcx, fr)?; + let implicit_inputs = self.regioncx.universal_regions().defining_ty.implicit_inputs(); + let argument_index = self.regioncx.get_argument_index_for_region(self.infcx.tcx, fr)?; - let arg_ty = self.nonlexical_regioncx.universal_regions().unnormalized_input_tys + let arg_ty = self.regioncx.universal_regions().unnormalized_input_tys [implicit_inputs + argument_index]; if let Some(region_name) = self.give_name_if_we_can_match_hir_ty_from_argument(fr, arg_ty, argument_index, renctx) @@ -415,10 +413,9 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ); let assigned_region_name = if type_name.find(&format!("'{}", counter)).is_some() { // Only add a label if we can confirm that a region was labelled. - let argument_index = self - .nonlexical_regioncx - .get_argument_index_for_region(self.infcx.tcx, needle_fr)?; - let (_, span) = self.nonlexical_regioncx.get_argument_name_and_span_for_region( + let argument_index = + self.regioncx.get_argument_index_for_region(self.infcx.tcx, needle_fr)?; + let (_, span) = self.regioncx.get_argument_name_and_span_for_region( &self.body, &self.local_names, argument_index, @@ -658,9 +655,8 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { fr: RegionVid, renctx: &mut RegionErrorNamingCtx, ) -> Option { - let upvar_index = - self.nonlexical_regioncx.get_upvar_index_for_region(self.infcx.tcx, fr)?; - let (upvar_name, upvar_span) = self.nonlexical_regioncx.get_upvar_name_and_span_for_region( + let upvar_index = self.regioncx.get_upvar_index_for_region(self.infcx.tcx, fr)?; + let (upvar_name, upvar_span) = self.regioncx.get_upvar_name_and_span_for_region( self.infcx.tcx, &self.upvars, upvar_index, @@ -684,7 +680,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ) -> Option { let tcx = self.infcx.tcx; - let return_ty = self.nonlexical_regioncx.universal_regions().unnormalized_output_ty; + let return_ty = self.regioncx.universal_regions().unnormalized_output_ty; debug!("give_name_if_anonymous_region_appears_in_output: return_ty = {:?}", return_ty); if !tcx.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr) { return None; @@ -734,7 +730,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ) -> Option { // Note: generators from `async fn` yield `()`, so we don't have to // worry about them here. - let yield_ty = self.nonlexical_regioncx.universal_regions().yield_ty?; + let yield_ty = self.regioncx.universal_regions().yield_ty?; debug!("give_name_if_anonymous_region_appears_in_yield_ty: yield_ty = {:?}", yield_ty,); let tcx = self.infcx.tcx; diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 22037ac6c81b2..0ad9d4492b208 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -283,7 +283,7 @@ fn do_mir_borrowck<'a, 'tcx>( move_error_reported: BTreeMap::new(), uninitialized_error_reported: Default::default(), errors_buffer, - nonlexical_regioncx: regioncx, + regioncx, used_mut: Default::default(), used_mut_upvars: SmallVec::new(), borrow_set, @@ -474,10 +474,9 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { /// If the function we're checking is a closure, then we'll need to report back the list of /// mutable upvars that have been used. This field keeps track of them. used_mut_upvars: SmallVec<[Field; 8]>, - /// Non-lexical region inference context, if NLL is enabled. This - /// contains the results from region inference and lets us e.g. + /// Region inference context. This contains the results from region inference and lets us e.g. /// find out which CFG points are contained in each borrow region. - nonlexical_regioncx: Rc>, + regioncx: Rc>, /// The set of borrows extracted from the MIR borrow_set: Rc>, From a804868bbf578dada5249ac3c79b7b524684c78e Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Sat, 28 Dec 2019 20:36:42 -0600 Subject: [PATCH 7/8] Get rid of RegionErrorNamingContext --- .../diagnostics/explain_borrow.rs | 7 +- .../borrow_check/diagnostics/mod.rs | 2 +- .../diagnostics/outlives_suggestion.rs | 25 +--- .../borrow_check/diagnostics/region_errors.rs | 51 +++---- .../borrow_check/diagnostics/region_name.rs | 138 ++++++------------ src/librustc_mir/borrow_check/mod.rs | 15 +- .../borrow_check/region_infer/mod.rs | 17 --- .../ui/c-variadic/variadic-ffi-4.nll.stderr | 4 +- 8 files changed, 88 insertions(+), 171 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs index ca5cf5bc741a0..01b7c5645fe8b 100644 --- a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs +++ b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs @@ -16,8 +16,8 @@ use rustc_span::symbol::Symbol; use rustc_span::Span; use crate::borrow_check::{ - borrow_set::BorrowData, diagnostics::RegionErrorNamingCtx, nll::ConstraintDescription, - region_infer::Cause, MirBorrowckCtxt, WriteKind, + borrow_set::BorrowData, nll::ConstraintDescription, region_infer::Cause, MirBorrowckCtxt, + WriteKind, }; use super::{find_use, RegionName, UseSpans}; @@ -267,8 +267,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { |r| self.regioncx.provides_universal_region(r, borrow_region, outlived_region), ); - let mut renctx = RegionErrorNamingCtx::new(); - let outlived_fr_name = self.give_region_a_name(&mut renctx, outlived_region); + let outlived_fr_name = self.give_region_a_name(outlived_region); (category, from_closure, span, outlived_fr_name) } diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index 3f3bdb9d36c76..0fc73d33f9001 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -32,7 +32,7 @@ mod region_errors; crate use mutability_errors::AccessKind; crate use outlives_suggestion::OutlivesSuggestionBuilder; crate use region_errors::{ErrorConstraintInfo, RegionErrorKind, RegionErrors}; -crate use region_name::{RegionErrorNamingCtx, RegionName, RegionNameSource}; +crate use region_name::{RegionName, RegionNameSource}; pub(super) struct IncludingDowncast(pub(super) bool); diff --git a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs index be067fcf4f261..ee9489078bdb9 100644 --- a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs +++ b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs @@ -12,7 +12,7 @@ use smallvec::SmallVec; use crate::borrow_check::MirBorrowckCtxt; -use super::{ErrorConstraintInfo, RegionErrorNamingCtx, RegionName, RegionNameSource}; +use super::{ErrorConstraintInfo, RegionName, RegionNameSource}; /// The different things we could suggest. enum SuggestedConstraint { @@ -77,17 +77,15 @@ impl OutlivesSuggestionBuilder { fn region_vid_to_name( &self, mbcx: &MirBorrowckCtxt<'_, '_>, - renctx: &mut RegionErrorNamingCtx, region: RegionVid, ) -> Option { - mbcx.give_region_a_name(renctx, region).filter(Self::region_name_is_suggestable) + mbcx.give_region_a_name(region).filter(Self::region_name_is_suggestable) } /// Compiles a list of all suggestions to be printed in the final big suggestion. fn compile_all_suggestions( &self, mbcx: &MirBorrowckCtxt<'_, '_>, - renctx: &mut RegionErrorNamingCtx, ) -> SmallVec<[SuggestedConstraint; 2]> { let mut suggested = SmallVec::new(); @@ -96,7 +94,7 @@ impl OutlivesSuggestionBuilder { let mut unified_already = FxHashSet::default(); for (fr, outlived) in &self.constraints_to_add { - let fr_name = if let Some(fr_name) = self.region_vid_to_name(mbcx, renctx, *fr) { + let fr_name = if let Some(fr_name) = self.region_vid_to_name(mbcx, *fr) { fr_name } else { continue; @@ -105,9 +103,7 @@ impl OutlivesSuggestionBuilder { let outlived = outlived .iter() // if there is a `None`, we will just omit that constraint - .filter_map(|fr| { - self.region_vid_to_name(mbcx, renctx, *fr).map(|rname| (fr, rname)) - }) + .filter_map(|fr| self.region_vid_to_name(mbcx, *fr).map(|rname| (fr, rname))) .collect::>(); // No suggestable outlived lifetimes. @@ -171,12 +167,11 @@ impl OutlivesSuggestionBuilder { &mut self, mbcx: &MirBorrowckCtxt<'_, '_>, errci: &ErrorConstraintInfo, - renctx: &mut RegionErrorNamingCtx, diag: &mut DiagnosticBuilder<'_>, ) { // Emit an intermediate note. - let fr_name = self.region_vid_to_name(mbcx, renctx, errci.fr); - let outlived_fr_name = self.region_vid_to_name(mbcx, renctx, errci.outlived_fr); + let fr_name = self.region_vid_to_name(mbcx, errci.fr); + let outlived_fr_name = self.region_vid_to_name(mbcx, errci.outlived_fr); if let (Some(fr_name), Some(outlived_fr_name)) = (fr_name, outlived_fr_name) { if let RegionNameSource::Static = outlived_fr_name.source { @@ -192,11 +187,7 @@ impl OutlivesSuggestionBuilder { /// If there is a suggestion to emit, add a diagnostic to the buffer. This is the final /// suggestion including all collected constraints. - crate fn add_suggestion( - &self, - mbcx: &mut MirBorrowckCtxt<'_, '_>, - renctx: &mut RegionErrorNamingCtx, - ) { + crate fn add_suggestion(&self, mbcx: &mut MirBorrowckCtxt<'_, '_>) { // No constraints to add? Done. if self.constraints_to_add.is_empty() { debug!("No constraints to suggest."); @@ -213,7 +204,7 @@ impl OutlivesSuggestionBuilder { } // Get all suggestable constraints. - let suggested = self.compile_all_suggestions(mbcx, renctx); + let suggested = self.compile_all_suggestions(mbcx); // If there are no suggestable constraints... if suggested.is_empty() { diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index f3dc8e72e3016..729a679e92a32 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -19,7 +19,7 @@ use crate::borrow_check::{ MirBorrowckCtxt, }; -use super::{OutlivesSuggestionBuilder, RegionErrorNamingCtx, RegionName, RegionNameSource}; +use super::{OutlivesSuggestionBuilder, RegionName, RegionNameSource}; impl ConstraintDescription for ConstraintCategory { fn description(&self) -> &'static str { @@ -152,8 +152,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are // buffered in the `MirBorrowckCtxt`. - // TODO(mark-i-m): Would be great to get rid of the naming context. - let mut region_naming = RegionErrorNamingCtx::new(); let mut outlives_suggestion = OutlivesSuggestionBuilder::default(); for nll_error in nll_errors.into_iter() { @@ -243,7 +241,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { fr_origin, shorter_fr, &mut outlives_suggestion, - &mut region_naming, ); } else { // We only report the first error, so as not to overwhelm the user. See @@ -262,7 +259,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } // Emit one outlives suggestions for each MIR def we borrowck - outlives_suggestion.add_suggestion(self, &mut region_naming); + outlives_suggestion.add_suggestion(self); } /// Report an error because the universal region `fr` was required to outlive @@ -279,7 +276,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { fr_origin: NLLRegionVariableOrigin, outlived_fr: RegionVid, outlives_suggestion: &mut OutlivesSuggestionBuilder, - renctx: &mut RegionErrorNamingCtx, ) { debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr); @@ -320,21 +316,21 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let diag = match (category, fr_is_local, outlived_fr_is_local) { (ConstraintCategory::Return, true, false) if self.is_closure_fn_mut(fr) => { - self.report_fnmut_error(&errci, renctx) + self.report_fnmut_error(&errci) } (ConstraintCategory::Assignment, true, false) | (ConstraintCategory::CallArgument, true, false) => { - let mut db = self.report_escaping_data_error(&errci, renctx); + let mut db = self.report_escaping_data_error(&errci); - outlives_suggestion.intermediate_suggestion(self, &errci, renctx, &mut db); + outlives_suggestion.intermediate_suggestion(self, &errci, &mut db); outlives_suggestion.collect_constraint(fr, outlived_fr); db } _ => { - let mut db = self.report_general_error(&errci, renctx); + let mut db = self.report_general_error(&errci); - outlives_suggestion.intermediate_suggestion(self, &errci, renctx, &mut db); + outlives_suggestion.intermediate_suggestion(self, &errci, &mut db); outlives_suggestion.collect_constraint(fr, outlived_fr); db @@ -360,11 +356,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// executing... /// = note: ...therefore, returned references to captured variables will escape the closure /// ``` - fn report_fnmut_error( - &self, - errci: &ErrorConstraintInfo, - renctx: &mut RegionErrorNamingCtx, - ) -> DiagnosticBuilder<'tcx> { + fn report_fnmut_error(&self, errci: &ErrorConstraintInfo) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { outlived_fr, span, .. } = errci; let mut diag = self @@ -386,7 +378,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { diag.span_label(*span, message); - match self.give_region_a_name(renctx, *outlived_fr).unwrap().source { + match self.give_region_a_name(*outlived_fr).unwrap().source { RegionNameSource::NamedEarlyBoundRegion(fr_span) | RegionNameSource::NamedFreeRegion(fr_span) | RegionNameSource::SynthesizedFreeEnvRegion(fr_span, _) @@ -421,11 +413,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// LL | ref_obj(x) /// | ^^^^^^^^^^ `x` escapes the function body here /// ``` - fn report_escaping_data_error( - &self, - errci: &ErrorConstraintInfo, - renctx: &mut RegionErrorNamingCtx, - ) -> DiagnosticBuilder<'tcx> { + fn report_escaping_data_error(&self, errci: &ErrorConstraintInfo) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { span, category, .. } = errci; let fr_name_and_span = self.regioncx.get_var_name_and_span_for_region( @@ -456,10 +444,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { || (*category == ConstraintCategory::Assignment && escapes_from == "function") || escapes_from == "const" { - return self.report_general_error( - &ErrorConstraintInfo { fr_is_local: true, outlived_fr_is_local: false, ..*errci }, - renctx, - ); + return self.report_general_error(&ErrorConstraintInfo { + fr_is_local: true, + outlived_fr_is_local: false, + ..*errci + }); } let mut diag = @@ -505,11 +494,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// | ^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it /// | is returning data with lifetime `'b` /// ``` - fn report_general_error( - &self, - errci: &ErrorConstraintInfo, - renctx: &mut RegionErrorNamingCtx, - ) -> DiagnosticBuilder<'tcx> { + fn report_general_error(&self, errci: &ErrorConstraintInfo) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { fr, fr_is_local, @@ -526,9 +511,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let mir_def_name = if self.infcx.tcx.is_closure(self.mir_def_id) { "closure" } else { "function" }; - let fr_name = self.give_region_a_name(renctx, *fr).unwrap(); + let fr_name = self.give_region_a_name(*fr).unwrap(); fr_name.highlight_region_name(&mut diag); - let outlived_fr_name = self.give_region_a_name(renctx, *outlived_fr).unwrap(); + let outlived_fr_name = self.give_region_a_name(*outlived_fr).unwrap(); outlived_fr_name.highlight_region_name(&mut diag); match (category, outlived_fr_is_local, fr_is_local) { diff --git a/src/librustc_mir/borrow_check/diagnostics/region_name.rs b/src/librustc_mir/borrow_check/diagnostics/region_name.rs index 7ce29713240ce..f7aeec5e76ee9 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_name.rs @@ -3,7 +3,6 @@ use std::fmt::{self, Display}; use rustc::ty::print::RegionHighlightMode; use rustc::ty::subst::{GenericArgKind, SubstsRef}; use rustc::ty::{self, RegionVid, Ty}; -use rustc_data_structures::fx::FxHashMap; use rustc_errors::DiagnosticBuilder; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; @@ -52,46 +51,6 @@ crate enum RegionNameSource { AnonRegionFromAsyncFn(Span), } -/// Records region names that have been assigned before so that we can use the same ones in later -/// diagnostics. -#[derive(Debug, Clone)] -crate struct RegionErrorNamingCtx { - /// Record the region names generated for each region in the given - /// MIR def so that we can reuse them later in help/error messages. - renctx: FxHashMap, - - /// The counter for generating new region names. - counter: usize, -} - -impl RegionErrorNamingCtx { - crate fn new() -> Self { - Self { counter: 1, renctx: FxHashMap::default() } - } - - /// Get the name of `region` if it has previously been named. - crate fn get(&self, region: &RegionVid) -> Option<&RegionName> { - self.renctx.get(region) - } - - /// Give `region` the name `name`. - crate fn insert(&mut self, region: RegionVid, name: RegionName) { - self.renctx.insert(region, name); - } - - /// Creates a synthetic region named `'N`, where `N` is the next value of the counter. Then, - /// increment the counter. - /// - /// The name is not memoized. A separate call to `insert` should be made later. (Currently, - /// this happens at the end of `give_region_a_name`). - crate fn synthesize_region_name(&mut self) -> Symbol { - let c = self.counter; - self.counter += 1; - - Symbol::intern(&format!("'{:?}", c)) - } -} - impl RegionName { crate fn was_named(&self) -> bool { match self.source { @@ -159,6 +118,17 @@ impl Display for RegionName { } impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { + /// Generate a synthetic region named `'N`, where `N` is the next value of the counter. Then, + /// increment the counter. + /// + /// This is _not_ idempotent. Call `give_region_a_name` when possible. + fn synthesize_region_name(&self) -> Symbol { + let mut counter = self.next_region_name.try_borrow_mut().unwrap(); + let c = *counter; + *counter += 1; + Symbol::intern(&format!("'{:?}", c)) + } + /// Maps from an internal MIR region vid to something that we can /// report to the user. In some cases, the region vids will map /// directly to lifetimes that the user has a name for (e.g., @@ -167,6 +137,8 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// that end, this function takes a "diagnostic" so that it can /// create auxiliary notes as needed. /// + /// The names are memoized, so this is both cheap to recompute and idempotent. + /// /// Example (function arguments): /// /// Suppose we are trying to give a name to the lifetime of the @@ -184,28 +156,28 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// ``` /// /// and then return the name `'1` for us to use. - crate fn give_region_a_name( - &self, - renctx: &mut RegionErrorNamingCtx, - fr: RegionVid, - ) -> Option { - debug!("give_region_a_name(fr={:?}, counter={:?})", fr, renctx.counter); + crate fn give_region_a_name(&self, fr: RegionVid) -> Option { + debug!( + "give_region_a_name(fr={:?}, counter={:?})", + fr, + self.next_region_name.try_borrow().unwrap() + ); assert!(self.regioncx.universal_regions().is_universal_region(fr)); - if let Some(value) = renctx.get(&fr) { + if let Some(value) = self.region_names.try_borrow_mut().unwrap().get(&fr) { return Some(value.clone()); } let value = self - .give_name_from_error_region(fr, renctx) - .or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr, renctx)) - .or_else(|| self.give_name_if_anonymous_region_appears_in_upvars(fr, renctx)) - .or_else(|| self.give_name_if_anonymous_region_appears_in_output(fr, renctx)) - .or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(fr, renctx)); + .give_name_from_error_region(fr) + .or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr)) + .or_else(|| self.give_name_if_anonymous_region_appears_in_upvars(fr)) + .or_else(|| self.give_name_if_anonymous_region_appears_in_output(fr)) + .or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(fr)); if let Some(ref value) = value { - renctx.insert(fr, value.clone()); + self.region_names.try_borrow_mut().unwrap().insert(fr, value.clone()); } debug!("give_region_a_name: gave name {:?}", value); @@ -216,11 +188,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// *user* has a name for. In that case, we'll be able to map /// `fr` to a `Region<'tcx>`, and that region will be one of /// named variants. - fn give_name_from_error_region( - &self, - fr: RegionVid, - renctx: &mut RegionErrorNamingCtx, - ) -> Option { + fn give_name_from_error_region(&self, fr: RegionVid) -> Option { let error_region = self.to_error_region(fr)?; let tcx = self.infcx.tcx; @@ -262,7 +230,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { // happen if we have an elided name in an async fn for example: the // compiler will generate a region named `'_`, but reporting such a name is // not actually useful, so we synthesize a name for it instead. - let name = renctx.synthesize_region_name(); + let name = self.synthesize_region_name(); Some(RegionName { name, source: RegionNameSource::AnonRegionFromAsyncFn(span), @@ -287,7 +255,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { } else { bug!("Closure is not defined by a closure expr"); }; - let region_name = renctx.synthesize_region_name(); + let region_name = self.synthesize_region_name(); let closure_kind_ty = substs.as_closure().kind_ty(def_id, tcx); let note = match closure_kind_ty.to_opt_closure_kind() { @@ -342,7 +310,6 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { fn give_name_if_anonymous_region_appears_in_arguments( &self, fr: RegionVid, - renctx: &mut RegionErrorNamingCtx, ) -> Option { let implicit_inputs = self.regioncx.universal_regions().defining_ty.implicit_inputs(); let argument_index = self.regioncx.get_argument_index_for_region(self.infcx.tcx, fr)?; @@ -350,12 +317,12 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { let arg_ty = self.regioncx.universal_regions().unnormalized_input_tys [implicit_inputs + argument_index]; if let Some(region_name) = - self.give_name_if_we_can_match_hir_ty_from_argument(fr, arg_ty, argument_index, renctx) + self.give_name_if_we_can_match_hir_ty_from_argument(fr, arg_ty, argument_index) { return Some(region_name); } - self.give_name_if_we_cannot_match_hir_ty(fr, arg_ty, renctx) + self.give_name_if_we_cannot_match_hir_ty(fr, arg_ty) } fn give_name_if_we_can_match_hir_ty_from_argument( @@ -363,7 +330,6 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { needle_fr: RegionVid, argument_ty: Ty<'tcx>, argument_index: usize, - renctx: &mut RegionErrorNamingCtx, ) -> Option { let mir_hir_id = self.infcx.tcx.hir().as_local_hir_id(self.mir_def_id)?; let fn_decl = self.infcx.tcx.hir().fn_decl_by_hir_id(mir_hir_id)?; @@ -376,12 +342,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { // (`give_name_if_anonymous_region_appears_in_arguments`). hir::TyKind::Infer => None, - _ => self.give_name_if_we_can_match_hir_ty( - needle_fr, - argument_ty, - argument_hir_ty, - renctx, - ), + _ => self.give_name_if_we_can_match_hir_ty(needle_fr, argument_ty, argument_hir_ty), } } @@ -400,9 +361,8 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { &self, needle_fr: RegionVid, argument_ty: Ty<'tcx>, - renctx: &mut RegionErrorNamingCtx, ) -> Option { - let counter = renctx.counter; + let counter = *self.next_region_name.try_borrow().unwrap(); let mut highlight = RegionHighlightMode::default(); highlight.highlighting_region_vid(needle_fr, counter); let type_name = self.infcx.extract_type_name(&argument_ty, Some(highlight)).0; @@ -425,7 +385,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { // This counter value will already have been used, so this function will increment // it so the next value will be used next and return the region name that would // have been used. - name: renctx.synthesize_region_name(), + name: self.synthesize_region_name(), source: RegionNameSource::CannotMatchHirTy(span, type_name), }) } else { @@ -461,7 +421,6 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { needle_fr: RegionVid, argument_ty: Ty<'tcx>, argument_hir_ty: &hir::Ty<'_>, - renctx: &mut RegionErrorNamingCtx, ) -> Option { let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty<'_>)> = &mut vec![(argument_ty, argument_hir_ty)]; @@ -479,7 +438,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { hir::TyKind::Rptr(_lifetime, referent_hir_ty), ) => { if region.to_region_vid() == needle_fr { - let region_name = renctx.synthesize_region_name(); + let region_name = self.synthesize_region_name(); // Just grab the first character, the `&`. let source_map = self.infcx.tcx.sess.source_map(); @@ -512,7 +471,6 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { substs, needle_fr, last_segment, - renctx, search_stack, ) { return Some(name); @@ -557,7 +515,6 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { substs: SubstsRef<'tcx>, needle_fr: RegionVid, last_segment: &'hir hir::PathSegment<'hir>, - renctx: &mut RegionErrorNamingCtx, search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty<'hir>)>, ) -> Option { // Did the user give explicit arguments? (e.g., `Foo<..>`) @@ -569,7 +526,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { | hir::LifetimeName::Error | hir::LifetimeName::Static | hir::LifetimeName::Underscore => { - let region_name = renctx.synthesize_region_name(); + let region_name = self.synthesize_region_name(); let ampersand_span = lifetime.span; Some(RegionName { name: region_name, @@ -650,18 +607,14 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// | let x = Some(&22); /// - fully elaborated type of `x` is `Option<&'1 u32>` /// ``` - fn give_name_if_anonymous_region_appears_in_upvars( - &self, - fr: RegionVid, - renctx: &mut RegionErrorNamingCtx, - ) -> Option { + fn give_name_if_anonymous_region_appears_in_upvars(&self, fr: RegionVid) -> Option { let upvar_index = self.regioncx.get_upvar_index_for_region(self.infcx.tcx, fr)?; let (upvar_name, upvar_span) = self.regioncx.get_upvar_name_and_span_for_region( self.infcx.tcx, &self.upvars, upvar_index, ); - let region_name = renctx.synthesize_region_name(); + let region_name = self.synthesize_region_name(); Some(RegionName { name: region_name, @@ -673,11 +626,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// must be a closure since, in a free fn, such an argument would /// have to either also appear in an argument (if using elision) /// or be early bound (named, not in argument). - fn give_name_if_anonymous_region_appears_in_output( - &self, - fr: RegionVid, - renctx: &mut RegionErrorNamingCtx, - ) -> Option { + fn give_name_if_anonymous_region_appears_in_output(&self, fr: RegionVid) -> Option { let tcx = self.infcx.tcx; let return_ty = self.regioncx.universal_regions().unnormalized_output_ty; @@ -687,7 +636,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { } let mut highlight = RegionHighlightMode::default(); - highlight.highlighting_region_vid(fr, renctx.counter); + highlight.highlighting_region_vid(fr, *self.next_region_name.try_borrow().unwrap()); let type_name = self.infcx.extract_type_name(&return_ty, Some(highlight)).0; let mir_hir_id = tcx.hir().as_local_hir_id(self.mir_def_id).expect("non-local mir"); @@ -714,7 +663,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { // This counter value will already have been used, so this function will increment it // so the next value will be used next and return the region name that would have been // used. - name: renctx.synthesize_region_name(), + name: self.synthesize_region_name(), source: RegionNameSource::AnonRegionFromOutput( return_span, mir_description.to_string(), @@ -726,7 +675,6 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { fn give_name_if_anonymous_region_appears_in_yield_ty( &self, fr: RegionVid, - renctx: &mut RegionErrorNamingCtx, ) -> Option { // Note: generators from `async fn` yield `()`, so we don't have to // worry about them here. @@ -740,7 +688,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { } let mut highlight = RegionHighlightMode::default(); - highlight.highlighting_region_vid(fr, renctx.counter); + highlight.highlighting_region_vid(fr, *self.next_region_name.try_borrow().unwrap()); let type_name = self.infcx.extract_type_name(&yield_ty, Some(highlight)).0; let mir_hir_id = tcx.hir().as_local_hir_id(self.mir_def_id).expect("non-local mir"); @@ -759,7 +707,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { ); Some(RegionName { - name: renctx.synthesize_region_name(), + name: self.synthesize_region_name(), source: RegionNameSource::AnonRegionFromYieldTy(yield_span, type_name), }) } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 0ad9d4492b208..90927069242b1 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -11,7 +11,8 @@ use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind}; use rustc::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, StatementKind}; use rustc::mir::{Terminator, TerminatorKind}; use rustc::ty::query::Providers; -use rustc::ty::{self, TyCtxt}; +use rustc::ty::{self, RegionVid, TyCtxt}; + use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::graph::dominators::Dominators; use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder}; @@ -21,6 +22,7 @@ use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use smallvec::SmallVec; +use std::cell::RefCell; use std::collections::BTreeMap; use std::mem; use std::rc::Rc; @@ -39,7 +41,7 @@ use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; use crate::transform::MirSource; -use self::diagnostics::AccessKind; +use self::diagnostics::{AccessKind, RegionName}; use self::flows::Flows; use self::location::LocationTable; use self::prefixes::PrefixSet; @@ -290,6 +292,8 @@ fn do_mir_borrowck<'a, 'tcx>( dominators, upvars, local_names, + region_names: RefCell::default(), + next_region_name: RefCell::new(1), }; // Compute and report region errors, if any. @@ -489,6 +493,13 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { /// Names of local (user) variables (extracted from `var_debug_info`). local_names: IndexVec>, + + /// Record the region names generated for each region in the given + /// MIR def so that we can reuse them later in help/error messages. + region_names: RefCell>, + + /// The counter for generating new region names. + next_region_name: RefCell, } // Check that: diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 359f75cac8376..180bb95cab9f4 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -1603,23 +1603,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - /// Get the region outlived by `longer_fr` and live at `element`. - crate fn region_from_element(&self, longer_fr: RegionVid, element: RegionElement) -> RegionVid { - match element { - RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l), - RegionElement::RootUniversalRegion(r) => r, - RegionElement::PlaceholderRegion(error_placeholder) => self - .definitions - .iter_enumerated() - .filter_map(|(r, definition)| match definition.origin { - NLLRegionVariableOrigin::Placeholder(p) if p == error_placeholder => Some(r), - _ => None, - }) - .next() - .unwrap(), - } - } - /// We have a constraint `fr1: fr2` that is not satisfied, where /// `fr2` represents some universal region. Here, `r` is some /// region where we know that `fr1: r` and this function has the diff --git a/src/test/ui/c-variadic/variadic-ffi-4.nll.stderr b/src/test/ui/c-variadic/variadic-ffi-4.nll.stderr index 8b70b15fa6e50..89107e799bd22 100644 --- a/src/test/ui/c-variadic/variadic-ffi-4.nll.stderr +++ b/src/test/ui/c-variadic/variadic-ffi-4.nll.stderr @@ -87,12 +87,12 @@ error[E0597]: `ap1` does not live long enough --> $DIR/variadic-ffi-4.rs:24:11 | LL | pub unsafe extern "C" fn no_escape4(_: usize, ap0: &mut VaListImpl, mut ap1: ...) { - | - let's call the lifetime of this reference `'1` + | - let's call the lifetime of this reference `'3` LL | ap0 = &mut ap1; | ------^^^^^^^^ | | | | | borrowed value does not live long enough - | assignment requires that `ap1` is borrowed for `'1` + | assignment requires that `ap1` is borrowed for `'3` ... LL | } | - `ap1` dropped here while still borrowed From f05e40eb992fce44db348cfb1e8ecb4699e9cdd6 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 1 Jan 2020 14:35:08 -0600 Subject: [PATCH 8/8] address review comments --- .../borrow_check/diagnostics/region_errors.rs | 15 +++++++-------- .../borrow_check/diagnostics/region_name.rs | 4 +--- src/librustc_mir/borrow_check/region_infer/mod.rs | 6 ++++-- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 729a679e92a32..b999dfa303103 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -138,8 +138,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { if let DefiningTy::Closure(def_id, substs) = self.regioncx.universal_regions().defining_ty { - let closure_kind_ty = substs.as_closure().kind_ty(def_id, self.infcx.tcx); - return Some(ty::ClosureKind::FnMut) == closure_kind_ty.to_opt_closure_kind(); + return substs.as_closure().kind(def_id, self.infcx.tcx) + == ty::ClosureKind::FnMut; } } } @@ -160,7 +160,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // Try to convert the lower-bound region into something named we can print for the user. let lower_bound_region = self.to_error_region(type_test.lower_bound); - // Skip duplicate-ish errors. let type_test_span = type_test.locations.span(&self.body); if let Some(lower_bound_region) = lower_bound_region { @@ -236,7 +235,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => { if is_reported { - self.report_error( + self.report_region_error( longer_fr, fr_origin, shorter_fr, @@ -270,21 +269,21 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// ``` /// /// Here we would be invoked with `fr = 'a` and `outlived_fr = `'b`. - pub(in crate::borrow_check) fn report_error( + pub(in crate::borrow_check) fn report_region_error( &mut self, fr: RegionVid, fr_origin: NLLRegionVariableOrigin, outlived_fr: RegionVid, outlives_suggestion: &mut OutlivesSuggestionBuilder, ) { - debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr); + debug!("report_region_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr); let (category, _, span) = self.regioncx.best_blame_constraint(&self.body, fr, fr_origin, |r| { self.regioncx.provides_universal_region(r, fr, outlived_fr) }); - debug!("report_error: category={:?} {:?}", category, span); + debug!("report_region_error: category={:?} {:?}", category, span); // Check if we can use one of the "nice region errors". if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { let tables = self.infcx.tcx.typeck_tables_of(self.mir_def_id); @@ -301,7 +300,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ); debug!( - "report_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}", + "report_region_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}", fr_is_local, outlived_fr_is_local, category ); diff --git a/src/librustc_mir/borrow_check/diagnostics/region_name.rs b/src/librustc_mir/borrow_check/diagnostics/region_name.rs index f7aeec5e76ee9..47eb2d8940af4 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_name.rs @@ -123,9 +123,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// /// This is _not_ idempotent. Call `give_region_a_name` when possible. fn synthesize_region_name(&self) -> Symbol { - let mut counter = self.next_region_name.try_borrow_mut().unwrap(); - let c = *counter; - *counter += 1; + let c = self.next_region_name.replace_with(|counter| *counter + 1); Symbol::intern(&format!("'{:?}", c)) } diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 180bb95cab9f4..26d9cf2e0450f 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -846,6 +846,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { // Type-test failed. Report the error. let erased_generic_kind = infcx.tcx.erase_regions(&type_test.generic_kind); + + // Skip duplicate-ish errors. if deduplicate_errors.insert(( erased_generic_kind, type_test.lower_bound, @@ -1850,8 +1852,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.scc_values.contains(r_scc, upper) } - crate fn universal_regions(&self) -> Rc> { - self.universal_regions.clone() + crate fn universal_regions(&self) -> &UniversalRegions<'tcx> { + self.universal_regions.as_ref() } /// Tries to find the best constraint to blame for the fact that