From f65dfa298e4a410679bb8594d4e64fef8928ff66 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Mon, 30 Dec 2019 15:16:28 +0100 Subject: [PATCH 1/5] perf: Avoid allocating intermediate Vecs --- src/librustc/infer/outlives/obligations.rs | 6 +- src/librustc/infer/outlives/verify.rs | 103 +++++++++++---------- src/librustc/traits/mod.rs | 2 +- src/librustc/ty/outlives.rs | 12 +-- src/librustc/ty/sty.rs | 32 +++---- 5 files changed, 80 insertions(+), 75 deletions(-) diff --git a/src/librustc/infer/outlives/obligations.rs b/src/librustc/infer/outlives/obligations.rs index 45e4a84589eb5..18f1338d1a1e0 100644 --- a/src/librustc/infer/outlives/obligations.rs +++ b/src/librustc/infer/outlives/obligations.rs @@ -289,7 +289,7 @@ where components: &[Component<'tcx>], region: ty::Region<'tcx>, ) { - for component in components.iter() { + for component in components { let origin = origin.clone(); match component { Component::Region(region1) => { @@ -367,8 +367,8 @@ where // Compute the bounds we can derive from the environment. This // is an "approximate" match -- in some cases, these bounds // may not apply. - let mut approx_env_bounds = - self.verify_bound.projection_approx_declared_bounds_from_env(projection_ty); + let mut approx_env_bounds: Vec<_> = + self.verify_bound.projection_approx_declared_bounds_from_env(projection_ty).collect(); debug!("projection_must_outlive: approx_env_bounds={:?}", approx_env_bounds); // Remove outlives bounds that we get from the environment but diff --git a/src/librustc/infer/outlives/verify.rs b/src/librustc/infer/outlives/verify.rs index 0380d0e35e78d..3808d9bb98c07 100644 --- a/src/librustc/infer/outlives/verify.rs +++ b/src/librustc/infer/outlives/verify.rs @@ -51,10 +51,8 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { // Start with anything like `T: 'a` we can scrape from the // environment - let param_bounds = self - .declared_generic_bounds_from_env(GenericKind::Param(param_ty)) - .into_iter() - .map(|outlives| outlives.1); + let param_bounds = + self.declared_generic_bounds_from_env(param_ty).map(|outlives| outlives.1); // Add in the default bound of fn body that applies to all in // scope type parameters: @@ -79,12 +77,15 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { pub fn projection_approx_declared_bounds_from_env( &self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> Vec, ty::Region<'tcx>>> { - let projection_ty = GenericKind::Projection(projection_ty).to_ty(self.tcx); - let erased_projection_ty = self.tcx.erase_regions(&projection_ty); - self.declared_generic_bounds_from_env_with_compare_fn(|ty| { + ) -> impl Iterator, ty::Region<'tcx>>> + 'cx + Captures<'tcx> + { + let tcx = self.tcx; + + let projection_ty = GenericKind::Projection(projection_ty).to_ty(tcx); + let erased_projection_ty = tcx.erase_regions(&projection_ty); + self.declared_generic_bounds_from_env_with_compare_fn(move |ty| { if let ty::Projection(..) = ty.kind { - let erased_ty = self.tcx.erase_regions(&ty); + let erased_ty = tcx.erase_regions(&ty); erased_ty == erased_projection_ty } else { false @@ -95,10 +96,13 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// Searches the where-clauses in scope for regions that /// `projection_ty` is known to outlive. Currently requires an /// exact match. - pub fn projection_declared_bounds_from_trait( - &self, + pub fn projection_declared_bounds_from_trait<'a>( + &'a self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> impl Iterator> + 'cx + Captures<'tcx> { + ) -> impl Iterator> + 'cx + Captures<'tcx> + 'a + where + 'a: 'cx, + { self.declared_projection_bounds_from_trait(projection_ty) } @@ -127,7 +131,6 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { // Extend with bounds that we can find from the trait. let trait_bounds = self .projection_declared_bounds_from_trait(projection_ty) - .into_iter() .map(|r| VerifyBound::OutlivedBy(r)); // see the extensive comment in projection_must_outlive @@ -138,17 +141,18 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { } fn recursive_type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { - let mut bounds = ty.walk_shallow().map(|subty| self.type_bound(subty)).collect::>(); - - let mut regions = smallvec![]; - ty.push_regions(&mut regions); - regions.retain(|r| !r.is_late_bound()); // ignore late-bound regions - bounds.push(VerifyBound::AllBounds( - regions.into_iter().map(|r| VerifyBound::OutlivedBy(r)).collect(), - )); - - // remove bounds that must hold, since they are not interesting - bounds.retain(|b| !b.must_hold()); + let mut bounds = ty + .walk_shallow() + .map(|subty| self.type_bound(subty)) + .chain(Some(VerifyBound::AllBounds( + // ignore late-bound regions + ty.regions() + .filter(|r| !r.is_late_bound()) + .map(|r| VerifyBound::OutlivedBy(r)) + .collect(), + ))) + .filter(|b| !b.must_hold()) + .collect::>(); if bounds.len() == 1 { bounds.pop().unwrap() } else { VerifyBound::AllBounds(bounds) } } @@ -161,16 +165,18 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// bounds, but all the bounds it returns can be relied upon. fn declared_generic_bounds_from_env( &self, - generic: GenericKind<'tcx>, - ) -> Vec, ty::Region<'tcx>>> { + generic: ty::ParamTy, + ) -> impl Iterator, ty::Region<'tcx>>> + 'cx + Captures<'tcx> + { let generic_ty = generic.to_ty(self.tcx); - self.declared_generic_bounds_from_env_with_compare_fn(|ty| ty == generic_ty) + self.declared_generic_bounds_from_env_with_compare_fn(move |ty| ty == generic_ty) } fn declared_generic_bounds_from_env_with_compare_fn( &self, - compare_ty: impl Fn(Ty<'tcx>) -> bool, - ) -> Vec, ty::Region<'tcx>>> { + compare_ty: impl Fn(Ty<'tcx>) -> bool + Clone + 'tcx + 'cx, + ) -> impl Iterator, ty::Region<'tcx>>> + 'cx + Captures<'tcx> + { let tcx = self.tcx; // To start, collect bounds from user environment. Note that @@ -180,7 +186,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { // like `T` and `T::Item`. It may not work as well for things // like `>::Item`. let c_b = self.param_env.caller_bounds; - let param_bounds = self.collect_outlives_from_predicate_list(&compare_ty, c_b); + let param_bounds = self.collect_outlives_from_predicate_list(compare_ty.clone(), c_b); // Next, collect regions we scraped from the well-formedness // constraints in the fn signature. To do that, we walk the list @@ -193,7 +199,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { // The problem is that the type of `x` is `&'a A`. To be // well-formed, then, A must be lower-generic by `'a`, but we // don't know that this holds from first principles. - let from_region_bound_pairs = self.region_bound_pairs.iter().filter_map(|&(r, p)| { + let from_region_bound_pairs = self.region_bound_pairs.iter().filter_map(move |&(r, p)| { debug!( "declared_generic_bounds_from_env_with_compare_fn: region_bound_pair = {:?}", (r, p) @@ -202,15 +208,12 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { compare_ty(p_ty).then_some(ty::OutlivesPredicate(p_ty, r)) }); - param_bounds - .chain(from_region_bound_pairs) - .inspect(|bound| { - debug!( - "declared_generic_bounds_from_env_with_compare_fn: result predicate = {:?}", - bound - ) - }) - .collect() + param_bounds.chain(from_region_bound_pairs).inspect(|bound| { + debug!( + "declared_generic_bounds_from_env_with_compare_fn: result predicate = {:?}", + bound + ) + }) } /// Given a projection like `>::Bar`, returns any bounds @@ -225,10 +228,13 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// then this function would return `'x`. This is subject to the /// limitations around higher-ranked bounds described in /// `region_bounds_declared_on_associated_item`. - fn declared_projection_bounds_from_trait( - &self, + fn declared_projection_bounds_from_trait<'a>( + &'a self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> impl Iterator> + 'cx + Captures<'tcx> { + ) -> impl Iterator> + 'cx + Captures<'tcx> + 'a + where + 'a: 'cx, + { debug!("projection_bounds(projection_ty={:?})", projection_ty); let tcx = self.tcx; self.region_bounds_declared_on_associated_item(projection_ty.item_def_id) @@ -265,10 +271,13 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// /// This is for simplicity, and because we are not really smart /// enough to cope with such bounds anywhere. - fn region_bounds_declared_on_associated_item( - &self, + fn region_bounds_declared_on_associated_item<'a>( + &'a self, assoc_item_def_id: DefId, - ) -> impl Iterator> + 'cx + Captures<'tcx> { + ) -> impl Iterator> + 'cx + Captures<'tcx> + 'a + where + 'a: 'cx, + { let tcx = self.tcx; let assoc_item = tcx.associated_item(assoc_item_def_id); let trait_def_id = assoc_item.container.assert_trait(); @@ -291,7 +300,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// otherwise want a precise match. fn collect_outlives_from_predicate_list( &self, - compare_ty: impl Fn(Ty<'tcx>) -> bool, + compare_ty: impl Fn(Ty<'tcx>) -> bool + 'tcx, predicates: impl IntoIterator>>, ) -> impl Iterator, ty::Region<'tcx>>> { predicates diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 3ba673d1a7d49..bc3bc98485352 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -59,7 +59,7 @@ pub use self::specialize::find_associated_item; pub use self::specialize::specialization_graph::FutureCompatOverlapError; pub use self::specialize::specialization_graph::FutureCompatOverlapErrorKind; pub use self::specialize::{specialization_graph, translate_substs, OverlapError}; -pub use self::util::{elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs}; +pub use self::util::{elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs, Elaborator}; pub use self::util::{expand_trait_aliases, TraitAliasExpander}; pub use self::util::{ supertrait_def_ids, supertraits, transitive_bounds, SupertraitDefIds, Supertraits, diff --git a/src/librustc/ty/outlives.rs b/src/librustc/ty/outlives.rs index 383ccbd337a7d..1c28260c7d87e 100644 --- a/src/librustc/ty/outlives.rs +++ b/src/librustc/ty/outlives.rs @@ -157,7 +157,7 @@ impl<'tcx> TyCtxt<'tcx> { // list is maintained explicitly, because bound regions // themselves can be readily identified. - push_region_constraints(ty, out); + out.extend(region_constraints(ty)); for subty in ty.walk_shallow() { self.compute_components(subty, out); } @@ -167,16 +167,14 @@ impl<'tcx> TyCtxt<'tcx> { fn capture_components(&self, ty: Ty<'tcx>) -> Vec> { let mut temp = smallvec![]; - push_region_constraints(ty, &mut temp); + temp.extend(region_constraints(ty)); for subty in ty.walk_shallow() { self.compute_components(subty, &mut temp); } - temp.into_iter().collect() + temp.into_vec() } } -fn push_region_constraints<'tcx>(ty: Ty<'tcx>, out: &mut SmallVec<[Component<'tcx>; 4]>) { - let mut regions = smallvec![]; - ty.push_regions(&mut regions); - out.extend(regions.iter().filter(|&r| !r.is_late_bound()).map(|r| Component::Region(r))); +fn region_constraints<'tcx>(ty: Ty<'tcx>) -> impl Iterator> { + ty.regions().filter(|&r| !r.is_late_bound()).map(|r| Component::Region(r)) } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index aeda2eb1a15c0..a48a59f78f9c0 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -22,7 +22,6 @@ use rustc_index::vec::Idx; use rustc_macros::HashStable; use rustc_span::symbol::{kw, Symbol}; use rustc_target::spec::abi; -use smallvec::SmallVec; use std::borrow::Cow; use std::cmp::Ordering; use std::marker::PhantomData; @@ -2208,26 +2207,25 @@ impl<'tcx> TyS<'tcx> { /// Pushes onto `out` the regions directly referenced from this type (but not /// types reachable from this type via `walk_tys`). This ignores late-bound /// regions binders. - pub fn push_regions(&self, out: &mut SmallVec<[ty::Region<'tcx>; 4]>) { - match self.kind { - Ref(region, _, _) => { - out.push(region); - } - Dynamic(ref obj, region) => { - out.push(region); - if let Some(principal) = obj.principal() { - out.extend(principal.skip_binder().substs.regions()); - } - } - Adt(_, substs) | Opaque(_, substs) => out.extend(substs.regions()), - Closure(_, ref substs) | Generator(_, ref substs, _) => out.extend(substs.regions()), + pub fn regions(&self) -> impl Iterator> { + let (opt_region, opt_subst_regions) = match self.kind { + Ref(region, _, _) => (Some(region), None), + Dynamic(ref obj, region) => ( + Some(region), + obj.principal().map(|principal| principal.skip_binder().substs.regions()), + ), + Adt(_, substs) | Opaque(_, substs) => (None, Some(substs.regions())), + Closure(_, ref substs) | Generator(_, ref substs, _) => (None, Some(substs.regions())), Projection(ref data) | UnnormalizedProjection(ref data) => { - out.extend(data.substs.regions()) + (None, Some(data.substs.regions())) } FnDef(..) | FnPtr(_) | GeneratorWitness(..) | Bool | Char | Int(_) | Uint(_) | Float(_) | Str | Array(..) | Slice(_) | RawPtr(_) | Never | Tuple(..) - | Foreign(..) | Param(_) | Bound(..) | Placeholder(..) | Infer(_) | Error => {} - } + | Foreign(..) | Param(_) | Bound(..) | Placeholder(..) | Infer(_) | Error => { + (None, None) + } + }; + opt_region.into_iter().chain(opt_subst_regions.into_iter().flatten()) } /// When we create a closure, we record its kind (i.e., what trait From 85d9dae5f417d3e251d2047013b2fa80f1eef428 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Fri, 3 Jan 2020 11:38:30 +0100 Subject: [PATCH 2/5] perf: Reuse the Elaborator used in outlives/ty.rs Profiling the build of https://github.com/Marwes/combine shows that ~1.3% of the time were spent resizing and rehashing the hash set used in `Elaborator`. By reusing the `Elaborator` between calls this should be mostly eliminated. It does result in some awkwardness since the `RefCell` needs to be borrowed in the returned `Iterator` but it works fine for the current code. --- src/librustc/infer/outlives/verify.rs | 46 +++++++++----- src/librustc/traits/util.rs | 89 +++++++++++++++------------ 2 files changed, 80 insertions(+), 55 deletions(-) diff --git a/src/librustc/infer/outlives/verify.rs b/src/librustc/infer/outlives/verify.rs index 3808d9bb98c07..8311bb3e160d8 100644 --- a/src/librustc/infer/outlives/verify.rs +++ b/src/librustc/infer/outlives/verify.rs @@ -1,3 +1,5 @@ +use std::cell::RefCell; + use crate::infer::outlives::env::RegionBoundPairs; use crate::infer::{GenericKind, VerifyBound}; use crate::traits; @@ -17,6 +19,7 @@ pub struct VerifyBoundCx<'cx, 'tcx> { region_bound_pairs: &'cx RegionBoundPairs<'tcx>, implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, + elaborator: RefCell>, } impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { @@ -26,7 +29,13 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, ) -> Self { - Self { tcx, region_bound_pairs, implicit_region_bound, param_env } + Self { + tcx, + region_bound_pairs, + implicit_region_bound, + param_env, + elaborator: RefCell::new(traits::Elaborator::new(tcx)), + } } /// Returns a "verify bound" that encodes what we know about @@ -113,10 +122,10 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs); // Search the env for where clauses like `P: 'a`. - let env_bounds = self - .projection_approx_declared_bounds_from_env(projection_ty) - .into_iter() - .map(|ty::OutlivesPredicate(ty, r)| { + let mut bounds = Vec::new(); + + bounds.extend(self.projection_approx_declared_bounds_from_env(projection_ty).map( + |ty::OutlivesPredicate(ty, r)| { let vb = VerifyBound::OutlivedBy(r); if ty == projection_ty_as_ty { // Micro-optimize if this is an exact match (this @@ -126,18 +135,20 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { } else { VerifyBound::IfEq(ty, Box::new(vb)) } - }); + }, + )); // Extend with bounds that we can find from the trait. - let trait_bounds = self - .projection_declared_bounds_from_trait(projection_ty) - .map(|r| VerifyBound::OutlivedBy(r)); + bounds.extend( + self.projection_declared_bounds_from_trait(projection_ty) + .map(|r| VerifyBound::OutlivedBy(r)), + ); // see the extensive comment in projection_must_outlive let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs); let recursive_bound = self.recursive_type_bound(ty); - VerifyBound::AnyBound(env_bounds.chain(trait_bounds).collect()).or(recursive_bound) + VerifyBound::AnyBound(bounds).or(recursive_bound) } fn recursive_type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { @@ -154,7 +165,11 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { .filter(|b| !b.must_hold()) .collect::>(); - if bounds.len() == 1 { bounds.pop().unwrap() } else { VerifyBound::AllBounds(bounds) } + if bounds.len() == 1 { + bounds.pop().unwrap() + } else { + VerifyBound::AllBounds(bounds) + } } /// Searches the environment for where-clauses like `G: 'a` where @@ -281,13 +296,16 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { let tcx = self.tcx; let assoc_item = tcx.associated_item(assoc_item_def_id); let trait_def_id = assoc_item.container.assert_trait(); - let trait_predicates = - tcx.predicates_of(trait_def_id).predicates.iter().map(|(p, _)| *p).collect(); + let trait_predicates = tcx.predicates_of(trait_def_id).predicates.iter().map(|(p, _)| *p); + let mut elaborator = self.elaborator.borrow_mut(); + elaborator.clear(); + elaborator.extend(trait_predicates); let identity_substs = InternalSubsts::identity_for_item(tcx, assoc_item_def_id); let identity_proj = tcx.mk_projection(assoc_item_def_id, identity_substs); + self.collect_outlives_from_predicate_list( move |ty| ty == identity_proj, - traits::elaborate_predicates(tcx, trait_predicates), + std::iter::from_fn(move || elaborator.next()), ) .map(|b| b.1) } diff --git a/src/librustc/traits/util.rs b/src/librustc/traits/util.rs index 14cfe7cda4e28..5d88129a73671 100644 --- a/src/librustc/traits/util.rs +++ b/src/librustc/traits/util.rs @@ -120,6 +120,15 @@ pub fn elaborate_predicates<'tcx>( } impl Elaborator<'tcx> { + pub fn new(tcx: TyCtxt<'tcx>) -> Self { + Self { stack: Vec::new(), visited: PredicateSet::new(tcx) } + } + + pub fn clear(&mut self) { + self.stack.clear(); + self.visited.set.clear(); + } + pub fn filter_to_traits(self) -> FilterToTraits { FilterToTraits::new(self) } @@ -137,14 +146,7 @@ impl Elaborator<'tcx> { .map(|(pred, _)| pred.subst_supertrait(tcx, &data.to_poly_trait_ref())); debug!("super_predicates: data={:?} predicates={:?}", data, predicates.clone()); - // Only keep those bounds that we haven't already seen. - // This is necessary to prevent infinite recursion in some - // cases. One common case is when people define - // `trait Sized: Sized { }` rather than `trait Sized { }`. - let visited = &mut self.visited; - let predicates = predicates.filter(|pred| visited.insert(pred)); - - self.stack.extend(predicates); + self.extend(predicates); } ty::Predicate::WellFormed(..) => { // Currently, we do not elaborate WF predicates, @@ -192,46 +194,51 @@ impl Elaborator<'tcx> { return; } - let visited = &mut self.visited; let mut components = smallvec![]; tcx.push_outlives_components(ty_max, &mut components); - self.stack.extend( - components - .into_iter() - .filter_map(|component| match component { - Component::Region(r) => { - if r.is_late_bound() { - None - } else { - Some(ty::Predicate::RegionOutlives(ty::Binder::dummy( - ty::OutlivesPredicate(r, r_min), - ))) - } - } - - Component::Param(p) => { - let ty = tcx.mk_ty_param(p.index, p.name); - Some(ty::Predicate::TypeOutlives(ty::Binder::dummy( - ty::OutlivesPredicate(ty, r_min), - ))) - } - - Component::UnresolvedInferenceVariable(_) => None, - - Component::Projection(_) | Component::EscapingProjection(_) => { - // We can probably do more here. This - // corresponds to a case like `>::U: 'b`. - None - } - }) - .filter(|p| visited.insert(p)), - ); + self.extend(components.into_iter().filter_map(|component| match component { + Component::Region(r) => { + if r.is_late_bound() { + None + } else { + Some(ty::Predicate::RegionOutlives(ty::Binder::dummy( + ty::OutlivesPredicate(r, r_min), + ))) + } + } + + Component::Param(p) => { + let ty = tcx.mk_ty_param(p.index, p.name); + Some(ty::Predicate::TypeOutlives(ty::Binder::dummy(ty::OutlivesPredicate( + ty, r_min, + )))) + } + + Component::UnresolvedInferenceVariable(_) => None, + + Component::Projection(_) | Component::EscapingProjection(_) => { + // We can probably do more here. This + // corresponds to a case like `>::U: 'b`. + None + } + })); } } } } +impl<'tcx> Extend> for Elaborator<'tcx> { + fn extend>>(&mut self, iter: I) { + let visited = &mut self.visited; + // Only keep those bounds that we haven't already seen. + // This is necessary to prevent infinite recursion in some + // cases. One common case is when people define + // `trait Sized: Sized { }` rather than `trait Sized { }`. + self.stack.extend(iter.into_iter().filter(|pred| visited.insert(pred))); + } +} + impl Iterator for Elaborator<'tcx> { type Item = ty::Predicate<'tcx>; From 02d1504a7e9c90bbb0c70acc1d4a10b0ad11ac89 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Sat, 4 Jan 2020 01:02:34 +0100 Subject: [PATCH 3/5] fix docs --- src/librustc/infer/outlives/verify.rs | 10 +++++----- src/librustc/infer/region_constraints/mod.rs | 15 ++++++++++----- src/librustc/ty/sty.rs | 2 +- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/librustc/infer/outlives/verify.rs b/src/librustc/infer/outlives/verify.rs index 8311bb3e160d8..7bf5c75632de5 100644 --- a/src/librustc/infer/outlives/verify.rs +++ b/src/librustc/infer/outlives/verify.rs @@ -144,11 +144,11 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { .map(|r| VerifyBound::OutlivedBy(r)), ); - // see the extensive comment in projection_must_outlive - let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs); - let recursive_bound = self.recursive_type_bound(ty); - - VerifyBound::AnyBound(bounds).or(recursive_bound) + VerifyBound::AnyBound(bounds).or(|| { + // see the extensive comment in projection_must_outlive + let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs); + self.recursive_type_bound(ty) + }) } fn recursive_type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { diff --git a/src/librustc/infer/region_constraints/mod.rs b/src/librustc/infer/region_constraints/mod.rs index f218bf1134f7f..33cd2ba48d063 100644 --- a/src/librustc/infer/region_constraints/mod.rs +++ b/src/librustc/infer/region_constraints/mod.rs @@ -904,13 +904,18 @@ impl<'tcx> VerifyBound<'tcx> { } } - pub fn or(self, vb: VerifyBound<'tcx>) -> VerifyBound<'tcx> { - if self.must_hold() || vb.cannot_hold() { + pub fn or(self, vb: impl FnOnce() -> VerifyBound<'tcx>) -> VerifyBound<'tcx> { + if self.must_hold() { self - } else if self.cannot_hold() || vb.must_hold() { - vb } else { - VerifyBound::AnyBound(vec![self, vb]) + let vb = vb(); + if vb.cannot_hold() { + self + } else if self.cannot_hold() || vb.must_hold() { + vb + } else { + VerifyBound::AnyBound(vec![self, vb]) + } } } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index a48a59f78f9c0..6602e2f2172f5 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -2204,7 +2204,7 @@ impl<'tcx> TyS<'tcx> { } } - /// Pushes onto `out` the regions directly referenced from this type (but not + /// Returns the regions directly referenced from this type (but not /// types reachable from this type via `walk_tys`). This ignores late-bound /// regions binders. pub fn regions(&self) -> impl Iterator> { From f7f40b9fc5a62ca1066f12afd4a05a4bf58cc484 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Sat, 4 Jan 2020 15:05:37 +0100 Subject: [PATCH 4/5] refactor: Don't require a `RefCell` in VerifyBoundCx Avoids the risk of attempting to borrow `elaborator` twice by accident. The need for `Captures2` is odd, but it seems like `impl Trait` kept generating lifetime constraints that forced `'a` or `'cx` to be identical to `'tcx` otherwise (due to `'tcx` being invariant I think). --- src/librustc/infer/outlives/obligations.rs | 2 +- src/librustc/infer/outlives/verify.rs | 50 +++++++++------------- src/librustc/util/captures.rs | 6 +++ 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/librustc/infer/outlives/obligations.rs b/src/librustc/infer/outlives/obligations.rs index 18f1338d1a1e0..ddb76f777a5e9 100644 --- a/src/librustc/infer/outlives/obligations.rs +++ b/src/librustc/infer/outlives/obligations.rs @@ -454,7 +454,7 @@ where } } -impl<'cx, 'tcx> TypeOutlivesDelegate<'tcx> for &'cx InferCtxt<'cx, 'tcx> { +impl<'cx, 'tcx> TypeOutlivesDelegate<'tcx> for &'_ InferCtxt<'cx, 'tcx> { fn push_sub_region_constraint( &mut self, origin: SubregionOrigin<'tcx>, diff --git a/src/librustc/infer/outlives/verify.rs b/src/librustc/infer/outlives/verify.rs index 7bf5c75632de5..b13b9fcd5b915 100644 --- a/src/librustc/infer/outlives/verify.rs +++ b/src/librustc/infer/outlives/verify.rs @@ -5,7 +5,7 @@ use crate::infer::{GenericKind, VerifyBound}; use crate::traits; use crate::ty::subst::{InternalSubsts, Subst}; use crate::ty::{self, Ty, TyCtxt}; -use crate::util::captures::Captures; +use crate::util::captures::{Captures, Captures2}; use rustc_hir::def_id::DefId; /// The `TypeOutlives` struct has the job of "lowering" a `T: 'a` @@ -19,7 +19,7 @@ pub struct VerifyBoundCx<'cx, 'tcx> { region_bound_pairs: &'cx RegionBoundPairs<'tcx>, implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, - elaborator: RefCell>, + elaborator: traits::Elaborator<'tcx>, } impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { @@ -34,20 +34,20 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { region_bound_pairs, implicit_region_bound, param_env, - elaborator: RefCell::new(traits::Elaborator::new(tcx)), + elaborator: traits::Elaborator::new(tcx), } } /// Returns a "verify bound" that encodes what we know about /// `generic` and the regions it outlives. - pub fn generic_bound(&self, generic: GenericKind<'tcx>) -> VerifyBound<'tcx> { + pub fn generic_bound(&mut self, generic: GenericKind<'tcx>) -> VerifyBound<'tcx> { match generic { GenericKind::Param(param_ty) => self.param_bound(param_ty), GenericKind::Projection(projection_ty) => self.projection_bound(projection_ty), } } - fn type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { + fn type_bound(&mut self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { match ty.kind { ty::Param(p) => self.param_bound(p), ty::Projection(data) => self.projection_bound(data), @@ -86,7 +86,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { pub fn projection_approx_declared_bounds_from_env( &self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> impl Iterator, ty::Region<'tcx>>> + 'cx + Captures<'tcx> + ) -> impl Iterator, ty::Region<'tcx>>> + Captures2<'cx, 'tcx> { let tcx = self.tcx; @@ -106,16 +106,13 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// `projection_ty` is known to outlive. Currently requires an /// exact match. pub fn projection_declared_bounds_from_trait<'a>( - &'a self, + &'a mut self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> impl Iterator> + 'cx + Captures<'tcx> + 'a - where - 'a: 'cx, - { + ) -> impl Iterator> + 'a + Captures2<'cx, 'tcx> { self.declared_projection_bounds_from_trait(projection_ty) } - pub fn projection_bound(&self, projection_ty: ty::ProjectionTy<'tcx>) -> VerifyBound<'tcx> { + pub fn projection_bound(&mut self, projection_ty: ty::ProjectionTy<'tcx>) -> VerifyBound<'tcx> { debug!("projection_bound(projection_ty={:?})", projection_ty); let projection_ty_as_ty = @@ -151,7 +148,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { }) } - fn recursive_type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { + fn recursive_type_bound(&mut self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { let mut bounds = ty .walk_shallow() .map(|subty| self.type_bound(subty)) @@ -181,7 +178,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { fn declared_generic_bounds_from_env( &self, generic: ty::ParamTy, - ) -> impl Iterator, ty::Region<'tcx>>> + 'cx + Captures<'tcx> + ) -> impl Iterator, ty::Region<'tcx>>> + Captures2<'cx, 'tcx> { let generic_ty = generic.to_ty(self.tcx); self.declared_generic_bounds_from_env_with_compare_fn(move |ty| ty == generic_ty) @@ -189,8 +186,8 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { fn declared_generic_bounds_from_env_with_compare_fn( &self, - compare_ty: impl Fn(Ty<'tcx>) -> bool + Clone + 'tcx + 'cx, - ) -> impl Iterator, ty::Region<'tcx>>> + 'cx + Captures<'tcx> + compare_ty: impl Fn(Ty<'tcx>) -> bool + Clone + 'tcx + Captures<'cx>, + ) -> impl Iterator, ty::Region<'tcx>>> + Captures2<'cx, 'tcx> { let tcx = self.tcx; @@ -201,7 +198,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { // like `T` and `T::Item`. It may not work as well for things // like `>::Item`. let c_b = self.param_env.caller_bounds; - let param_bounds = self.collect_outlives_from_predicate_list(compare_ty.clone(), c_b); + let param_bounds = Self::collect_outlives_from_predicate_list(compare_ty.clone(), c_b); // Next, collect regions we scraped from the well-formedness // constraints in the fn signature. To do that, we walk the list @@ -244,12 +241,9 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// limitations around higher-ranked bounds described in /// `region_bounds_declared_on_associated_item`. fn declared_projection_bounds_from_trait<'a>( - &'a self, + &'a mut self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> impl Iterator> + 'cx + Captures<'tcx> + 'a - where - 'a: 'cx, - { + ) -> impl Iterator> + 'a + Captures2<'cx, 'tcx> { debug!("projection_bounds(projection_ty={:?})", projection_ty); let tcx = self.tcx; self.region_bounds_declared_on_associated_item(projection_ty.item_def_id) @@ -287,23 +281,20 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// This is for simplicity, and because we are not really smart /// enough to cope with such bounds anywhere. fn region_bounds_declared_on_associated_item<'a>( - &'a self, + &'a mut self, assoc_item_def_id: DefId, - ) -> impl Iterator> + 'cx + Captures<'tcx> + 'a - where - 'a: 'cx, - { + ) -> impl Iterator> + 'a + Captures2<'cx, 'tcx> { let tcx = self.tcx; let assoc_item = tcx.associated_item(assoc_item_def_id); let trait_def_id = assoc_item.container.assert_trait(); let trait_predicates = tcx.predicates_of(trait_def_id).predicates.iter().map(|(p, _)| *p); - let mut elaborator = self.elaborator.borrow_mut(); + let elaborator = &mut self.elaborator; elaborator.clear(); elaborator.extend(trait_predicates); let identity_substs = InternalSubsts::identity_for_item(tcx, assoc_item_def_id); let identity_proj = tcx.mk_projection(assoc_item_def_id, identity_substs); - self.collect_outlives_from_predicate_list( + Self::collect_outlives_from_predicate_list( move |ty| ty == identity_proj, std::iter::from_fn(move || elaborator.next()), ) @@ -317,7 +308,6 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// that does not involve inference variables and where you /// otherwise want a precise match. fn collect_outlives_from_predicate_list( - &self, compare_ty: impl Fn(Ty<'tcx>) -> bool + 'tcx, predicates: impl IntoIterator>>, ) -> impl Iterator, ty::Region<'tcx>>> { diff --git a/src/librustc/util/captures.rs b/src/librustc/util/captures.rs index 26b90ebfd5f11..7e5dd482d6e96 100644 --- a/src/librustc/util/captures.rs +++ b/src/librustc/util/captures.rs @@ -8,3 +8,9 @@ pub trait Captures<'a> {} impl<'a, T: ?Sized> Captures<'a> for T {} + +// FIXME(eddyb) false positive, the lifetime parameter is "phantom" but needed. +#[allow(unused_lifetimes)] +pub trait Captures2<'a, 'b> {} + +impl<'a, 'b, T: ?Sized> Captures2<'a, 'b> for T {} From 365ccdeae5ec96c836c769803729c5adc4a61210 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Sat, 4 Jan 2020 16:29:11 +0100 Subject: [PATCH 5/5] Hoist the reused Elaborator out so it actually gets reused --- src/librustc/infer/outlives/obligations.rs | 33 +++++++++++++------ src/librustc/infer/outlives/verify.rs | 31 ++++++----------- src/librustc/util/captures.rs | 6 +++- .../type_check/constraint_conversion.rs | 2 ++ 4 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/librustc/infer/outlives/obligations.rs b/src/librustc/infer/outlives/obligations.rs index ddb76f777a5e9..4586c08a4a04f 100644 --- a/src/librustc/infer/outlives/obligations.rs +++ b/src/librustc/infer/outlives/obligations.rs @@ -62,6 +62,7 @@ use crate::infer::outlives::env::RegionBoundPairs; use crate::infer::outlives::verify::VerifyBoundCx; use crate::infer::{self, GenericKind, InferCtxt, RegionObligation, SubregionOrigin, VerifyBound}; +use crate::traits; use crate::traits::ObligationCause; use crate::ty::outlives::Component; use crate::ty::subst::GenericArgKind; @@ -154,6 +155,8 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> { let my_region_obligations = self.take_registered_region_obligations(); + let mut elaborator = traits::Elaborator::new(self.tcx); + for (body_id, RegionObligation { sup_type, sub_region, origin }) in my_region_obligations { debug!( "process_registered_region_obligations: sup_type={:?} sub_region={:?} origin={:?}", @@ -169,6 +172,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> { ®ion_bound_pairs, implicit_region_bound, param_env, + &mut elaborator, ); outlives.type_must_outlive(origin, sup_type, sub_region); } else { @@ -191,15 +195,16 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> { ty: Ty<'tcx>, region: ty::Region<'tcx>, ) { - let outlives = &mut TypeOutlives::new( + let ty = self.resolve_vars_if_possible(&ty); + TypeOutlives::new( self, self.tcx, region_bound_pairs, implicit_region_bound, param_env, - ); - let ty = self.resolve_vars_if_possible(&ty); - outlives.type_must_outlive(origin, ty, region); + &mut traits::Elaborator::new(self.tcx), + ) + .type_must_outlive(origin, ty, region); } } @@ -209,7 +214,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> { /// via a "delegate" of type `D` -- this is usually the `infcx`, which /// accrues them into the `region_obligations` code, but for NLL we /// use something else. -pub struct TypeOutlives<'cx, 'tcx, D> +pub struct TypeOutlives<'cx, 'tcx, 'e, D> where D: TypeOutlivesDelegate<'tcx>, { @@ -217,7 +222,7 @@ where // of these fields. delegate: D, tcx: TyCtxt<'tcx>, - verify_bound: VerifyBoundCx<'cx, 'tcx>, + verify_bound: VerifyBoundCx<'cx, 'tcx, 'e>, } pub trait TypeOutlivesDelegate<'tcx> { @@ -237,7 +242,7 @@ pub trait TypeOutlivesDelegate<'tcx> { ); } -impl<'cx, 'tcx, D> TypeOutlives<'cx, 'tcx, D> +impl<'cx, 'tcx, 'e, D> TypeOutlives<'cx, 'tcx, 'e, D> where D: TypeOutlivesDelegate<'tcx>, { @@ -247,6 +252,7 @@ where region_bound_pairs: &'cx RegionBoundPairs<'tcx>, implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, + elaborator: &'e mut traits::Elaborator<'tcx>, ) -> Self { Self { delegate, @@ -256,6 +262,7 @@ where region_bound_pairs, implicit_region_bound, param_env, + elaborator, ), } } @@ -273,7 +280,9 @@ where origin: infer::SubregionOrigin<'tcx>, ty: Ty<'tcx>, region: ty::Region<'tcx>, - ) { + ) where + 'tcx: 'e, + { debug!("type_must_outlive(ty={:?}, region={:?}, origin={:?})", ty, region, origin); assert!(!ty.has_escaping_bound_vars()); @@ -288,7 +297,9 @@ where origin: infer::SubregionOrigin<'tcx>, components: &[Component<'tcx>], region: ty::Region<'tcx>, - ) { + ) where + 'tcx: 'e, + { for component in components { let origin = origin.clone(); match component { @@ -338,7 +349,9 @@ where origin: infer::SubregionOrigin<'tcx>, region: ty::Region<'tcx>, projection_ty: ty::ProjectionTy<'tcx>, - ) { + ) where + 'tcx: 'e, + { debug!( "projection_must_outlive(region={:?}, projection_ty={:?}, origin={:?})", region, projection_ty, origin diff --git a/src/librustc/infer/outlives/verify.rs b/src/librustc/infer/outlives/verify.rs index b13b9fcd5b915..29d25541e8654 100644 --- a/src/librustc/infer/outlives/verify.rs +++ b/src/librustc/infer/outlives/verify.rs @@ -1,11 +1,9 @@ -use std::cell::RefCell; - use crate::infer::outlives::env::RegionBoundPairs; use crate::infer::{GenericKind, VerifyBound}; use crate::traits; use crate::ty::subst::{InternalSubsts, Subst}; use crate::ty::{self, Ty, TyCtxt}; -use crate::util::captures::{Captures, Captures2}; +use crate::util::captures::{Captures, Captures2, Captures3}; use rustc_hir::def_id::DefId; /// The `TypeOutlives` struct has the job of "lowering" a `T: 'a` @@ -14,28 +12,23 @@ use rustc_hir::def_id::DefId; /// via a "delegate" of type `D` -- this is usually the `infcx`, which /// accrues them into the `region_obligations` code, but for NLL we /// use something else. -pub struct VerifyBoundCx<'cx, 'tcx> { +pub struct VerifyBoundCx<'cx, 'tcx, 'e> { tcx: TyCtxt<'tcx>, region_bound_pairs: &'cx RegionBoundPairs<'tcx>, implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, - elaborator: traits::Elaborator<'tcx>, + elaborator: &'e mut traits::Elaborator<'tcx>, } -impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { +impl<'cx, 'tcx, 'e> VerifyBoundCx<'cx, 'tcx, 'e> { pub fn new( tcx: TyCtxt<'tcx>, region_bound_pairs: &'cx RegionBoundPairs<'tcx>, implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, + elaborator: &'e mut traits::Elaborator<'tcx>, ) -> Self { - Self { - tcx, - region_bound_pairs, - implicit_region_bound, - param_env, - elaborator: traits::Elaborator::new(tcx), - } + Self { tcx, region_bound_pairs, implicit_region_bound, param_env, elaborator } } /// Returns a "verify bound" that encodes what we know about @@ -108,7 +101,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { pub fn projection_declared_bounds_from_trait<'a>( &'a mut self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> impl Iterator> + 'a + Captures2<'cx, 'tcx> { + ) -> impl Iterator> + 'a + Captures3<'cx, 'tcx, 'e> { self.declared_projection_bounds_from_trait(projection_ty) } @@ -162,11 +155,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { .filter(|b| !b.must_hold()) .collect::>(); - if bounds.len() == 1 { - bounds.pop().unwrap() - } else { - VerifyBound::AllBounds(bounds) - } + if bounds.len() == 1 { bounds.pop().unwrap() } else { VerifyBound::AllBounds(bounds) } } /// Searches the environment for where-clauses like `G: 'a` where @@ -243,7 +232,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { fn declared_projection_bounds_from_trait<'a>( &'a mut self, projection_ty: ty::ProjectionTy<'tcx>, - ) -> impl Iterator> + 'a + Captures2<'cx, 'tcx> { + ) -> impl Iterator> + 'a + Captures3<'cx, 'tcx, 'e> { debug!("projection_bounds(projection_ty={:?})", projection_ty); let tcx = self.tcx; self.region_bounds_declared_on_associated_item(projection_ty.item_def_id) @@ -283,7 +272,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { fn region_bounds_declared_on_associated_item<'a>( &'a mut self, assoc_item_def_id: DefId, - ) -> impl Iterator> + 'a + Captures2<'cx, 'tcx> { + ) -> impl Iterator> + 'a + Captures3<'cx, 'tcx, 'e> { let tcx = self.tcx; let assoc_item = tcx.associated_item(assoc_item_def_id); let trait_def_id = assoc_item.container.assert_trait(); diff --git a/src/librustc/util/captures.rs b/src/librustc/util/captures.rs index 7e5dd482d6e96..10c59e670ddcb 100644 --- a/src/librustc/util/captures.rs +++ b/src/librustc/util/captures.rs @@ -9,8 +9,12 @@ pub trait Captures<'a> {} impl<'a, T: ?Sized> Captures<'a> for T {} -// FIXME(eddyb) false positive, the lifetime parameter is "phantom" but needed. #[allow(unused_lifetimes)] pub trait Captures2<'a, 'b> {} impl<'a, 'b, T: ?Sized> Captures2<'a, 'b> for T {} + +#[allow(unused_lifetimes)] +pub trait Captures3<'a, 'b, 'c> {} + +impl<'a, 'b, 'c, T: ?Sized> Captures3<'a, 'b, 'c> for T {} diff --git a/src/librustc_mir/borrow_check/type_check/constraint_conversion.rs b/src/librustc_mir/borrow_check/type_check/constraint_conversion.rs index 8f65a0f01c6e0..40b705968fd40 100644 --- a/src/librustc_mir/borrow_check/type_check/constraint_conversion.rs +++ b/src/librustc_mir/borrow_check/type_check/constraint_conversion.rs @@ -5,6 +5,7 @@ use rustc::infer::outlives::obligations::{TypeOutlives, TypeOutlivesDelegate}; use rustc::infer::region_constraints::{GenericKind, VerifyBound}; use rustc::infer::{self, InferCtxt, SubregionOrigin}; use rustc::mir::ConstraintCategory; +use rustc::traits; use rustc::ty::subst::GenericArgKind; use rustc::ty::{self, TyCtxt}; use rustc_span::DUMMY_SP; @@ -108,6 +109,7 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> { region_bound_pairs, implicit_region_bound, param_env, + &mut traits::Elaborator::new(tcx), ) .type_must_outlive(origin, t1, r2); }