From 8e64ba83beac81f38c5b473f9dbee924a03a3ea7 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 9 Dec 2017 06:36:10 -0500 Subject: [PATCH] extract `constrain_anon_types` to the `InferCtxt` No funtional change. --- src/librustc/infer/anon_types/mod.rs | 262 +++++++++++++++++- .../infer/outlives/free_region_map.rs | 37 ++- src/librustc/middle/free_region.rs | 2 +- src/librustc_typeck/check/regionck.rs | 194 +------------ 4 files changed, 289 insertions(+), 206 deletions(-) diff --git a/src/librustc/infer/anon_types/mod.rs b/src/librustc/infer/anon_types/mod.rs index 6af4f13db88..74daeb837c9 100644 --- a/src/librustc/infer/anon_types/mod.rs +++ b/src/librustc/infer/anon_types/mod.rs @@ -9,11 +9,13 @@ // except according to those terms. use hir::def_id::DefId; -use infer::{InferCtxt, InferOk, TypeVariableOrigin}; +use infer::{self, InferCtxt, InferOk, TypeVariableOrigin}; +use infer::outlives::free_region_map::FreeRegionRelations; use syntax::ast; use traits::{self, PredicateObligation}; use ty::{self, Ty}; use ty::fold::{BottomUpFolder, TypeFoldable}; +use ty::outlives::Component; use ty::subst::Substs; use util::nodemap::DefIdMap; @@ -115,6 +117,264 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { obligations: instantiator.obligations, } } + + /// Given the map `anon_types` containing the existential `impl + /// Trait` types whose underlying, hidden types are being + /// inferred, this method adds constraints to the regions + /// appearing in those underlying hidden types to ensure that they + /// at least do not refer to random scopes within the current + /// function. These constraints are not (quite) sufficient to + /// guarantee that the regions are actually legal values; that + /// final condition is imposed after region inference is done. + /// + /// # The Problem + /// + /// Let's work through an example to explain how it works. Assume + /// the current function is as follows: + /// + /// fn foo<'a, 'b>(..) -> (impl Bar<'a>, impl Bar<'b>) + /// + /// Here, we have two `impl Trait` types whose values are being + /// inferred (the `impl Bar<'a>` and the `impl + /// Bar<'b>`). Conceptually, this is sugar for a setup where we + /// define underlying abstract types (`Foo1`, `Foo2`) and then, in + /// the return type of `foo`, we *reference* those definitions: + /// + /// abstract type Foo1<'x>: Bar<'x>; + /// abstract type Foo2<'x>: Bar<'x>; + /// fn foo<'a, 'b>(..) -> (Foo1<'a>, Foo2<'b>) { .. } + /// // ^^^^ ^^ + /// // | | + /// // | substs + /// // def_id + /// + /// As indicating in the comments above, each of those references + /// is (in the compiler) basically a substitution (`substs`) + /// applied to the type of a suitable `def_id` (which identifies + /// `Foo1` or `Foo2`). + /// + /// Now, at this point in compilation, what we have done is to + /// replace each of the references (`Foo1<'a>`, `Foo2<'b>`) with + /// fresh inference variables C1 and C2. We wish to use the values + /// of these variables to infer the underlying types of `Foo1` and + /// `Foo2`. That is, this gives rise to higher-order (pattern) unification + /// constraints like: + /// + /// for<'a> (Foo1<'a> = C1) + /// for<'b> (Foo1<'b> = C2) + /// + /// For these equation to be satisfiable, the types `C1` and `C2` + /// can only refer to a limited set of regions. For example, `C1` + /// can only refer to `'static` and `'a`, and `C2` can only refer + /// to `'static` and `'b`. The job of this function is to impose that + /// constraint. + /// + /// Up to this point, C1 and C2 are basically just random type + /// inference variables, and hence they may contain arbitrary + /// regions. In fact, it is fairly likely that they do! Consider + /// this possible definition of `foo`: + /// + /// fn foo<'a, 'b>(x: &'a i32, y: &'b i32) -> (impl Bar<'a>, impl Bar<'b>) { + /// (&*x, &*y) + /// } + /// + /// Here, the values for the concrete types of the two impl + /// traits will include inference variables: + /// + /// &'0 i32 + /// &'1 i32 + /// + /// Ordinarily, the subtyping rules would ensure that these are + /// sufficiently large. But since `impl Bar<'a>` isn't a specific + /// type per se, we don't get such constraints by default. This + /// is where this function comes into play. It adds extra + /// constraints to ensure that all the regions which appear in the + /// inferred type are regions that could validly appear. + /// + /// This is actually a bit of a tricky constraint in general. We + /// want to say that each variable (e.g., `'0``) can only take on + /// values that were supplied as arguments to the abstract type + /// (e.g., `'a` for `Foo1<'a>`) or `'static`, which is always in + /// scope. We don't have a constraint quite of this kind in the current + /// region checker. + /// + /// # The Solution + /// + /// We make use of the constraint that we *do* have in the `<=` + /// relation. To do that, we find the "minimum" of all the + /// arguments that appear in the substs: that is, some region + /// which is less than all the others. In the case of `Foo1<'a>`, + /// that would be `'a` (it's the only choice, after all). Then we + /// apply that as a least bound to the variables (e.g., `'a <= + /// '0`). + /// + /// In some cases, there is no minimum. Consider this example: + /// + /// fn baz<'a, 'b>() -> impl Trait<'a, 'b> { ... } + /// + /// Here we would report an error, because `'a` and `'b` have no + /// relation to one another. + /// + /// # The `free_region_relations` parameter + /// + /// The `free_region_relations` argument is used to find the + /// "minimum" of the regions supplied to a given abstract type. + /// It must be a relation that can answer whether `'a <= 'b`, + /// where `'a` and `'b` are regions that appear in the "substs" + /// for the abstract type references (the `<'a>` in `Foo1<'a>`). + /// + /// Note that we do not impose the constraints based on the + /// generic regions from the `Foo1` definition (e.g., `'x`). This + /// is because the constraints we are imposing here is basically + /// the concern of the one generating the constraining type C1, + /// which is the current function. It also means that we can + /// take "implied bounds" into account in some cases: + /// + /// trait SomeTrait<'a, 'b> { } + /// fn foo<'a, 'b>(_: &'a &'b u32) -> impl SomeTrait<'a, 'b> { .. } + /// + /// Here, the fact that `'b: 'a` is known only because of the + /// implied bounds from the `&'a &'b u32` parameter, and is not + /// "inherent" to the abstract type definition. + /// + /// # Parameters + /// + /// - `anon_types` -- the map produced by `instantiate_anon_types` + /// - `free_region_relations` -- something that can be used to relate + /// the free regions (`'a`) that appear in the impl trait. + pub fn constrain_anon_types>( + &self, + anon_types: &AnonTypeMap<'tcx>, + free_region_relations: &FRR, + ) { + debug!("constrain_anon_types()"); + + for (&def_id, anon_defn) in anon_types { + self.constrain_anon_type(def_id, anon_defn, free_region_relations); + } + } + + fn constrain_anon_type>( + &self, + def_id: DefId, + anon_defn: &AnonTypeDecl<'tcx>, + free_region_relations: &FRR, + ) { + debug!("constrain_anon_type()"); + debug!("constrain_anon_type: def_id={:?}", def_id); + debug!("constrain_anon_type: anon_defn={:#?}", anon_defn); + + let concrete_ty = self.resolve_type_vars_if_possible(&anon_defn.concrete_ty); + + debug!("constrain_anon_type: concrete_ty={:?}", concrete_ty); + + let abstract_type_generics = self.tcx.generics_of(def_id); + + let span = self.tcx.def_span(def_id); + + // If there are required region bounds, we can just skip + // ahead. There will already be a registered region + // obligation related `concrete_ty` to those regions. + if anon_defn.has_required_region_bounds { + return; + } + + // There were no `required_region_bounds`, + // so we have to search for a `least_region`. + // Go through all the regions used as arguments to the + // abstract type. These are the parameters to the abstract + // type; so in our example above, `substs` would contain + // `['a]` for the first impl trait and `'b` for the + // second. + let mut least_region = None; + for region_def in &abstract_type_generics.regions { + // Find the index of this region in the list of substitutions. + let index = region_def.index as usize; + + // Get the value supplied for this region from the substs. + let subst_arg = anon_defn.substs[index].as_region().unwrap(); + + // Compute the least upper bound of it with the other regions. + debug!("constrain_anon_types: least_region={:?}", least_region); + debug!("constrain_anon_types: subst_arg={:?}", subst_arg); + match least_region { + None => least_region = Some(subst_arg), + Some(lr) => { + if free_region_relations.sub_free_regions(lr, subst_arg) { + // keep the current least region + } else if free_region_relations.sub_free_regions(subst_arg, lr) { + // switch to `subst_arg` + least_region = Some(subst_arg); + } else { + // There are two regions (`lr` and + // `subst_arg`) which are not relatable. We can't + // find a best choice. + self.tcx + .sess + .struct_span_err(span, "ambiguous lifetime bound in `impl Trait`") + .span_label( + span, + format!("neither `{}` nor `{}` outlives the other", lr, subst_arg), + ) + .emit(); + + least_region = Some(self.tcx.mk_region(ty::ReEmpty)); + break; + } + } + } + } + + let least_region = least_region.unwrap_or(self.tcx.types.re_static); + debug!("constrain_anon_types: least_region={:?}", least_region); + + // Require that the type `concrete_ty` outlives + // `least_region`, modulo any type parameters that appear + // in the type, which we ignore. This is because impl + // trait values are assumed to capture all the in-scope + // type parameters. This little loop here just invokes + // `outlives` repeatedly, draining all the nested + // obligations that result. + let mut types = vec![concrete_ty]; + let bound_region = |r| self.sub_regions(infer::CallReturn(span), least_region, r); + while let Some(ty) = types.pop() { + let mut components = self.tcx.outlives_components(ty); + while let Some(component) = components.pop() { + match component { + Component::Region(r) => { + bound_region(r); + } + + Component::Param(_) => { + // ignore type parameters like `T`, they are captured + // implicitly by the `impl Trait` + } + + Component::UnresolvedInferenceVariable(_) => { + // we should get an error that more type + // annotations are needed in this case + self.tcx + .sess + .delay_span_bug(span, "unresolved inf var in anon"); + } + + Component::Projection(ty::ProjectionTy { + substs, + item_def_id: _, + }) => { + for r in substs.regions() { + bound_region(r); + } + types.extend(substs.types()); + } + + Component::EscapingProjection(more_components) => { + components.extend(more_components); + } + } + } + } + } } struct Instantiator<'a, 'gcx: 'tcx, 'tcx: 'a> { diff --git a/src/librustc/infer/outlives/free_region_map.rs b/src/librustc/infer/outlives/free_region_map.rs index 2127c4714ae..6163ec16420 100644 --- a/src/librustc/infer/outlives/free_region_map.rs +++ b/src/librustc/infer/outlives/free_region_map.rs @@ -38,20 +38,6 @@ impl<'tcx> FreeRegionMap<'tcx> { } } - /// Tests whether `r_a <= r_b`. Both must be free regions or - /// `'static`. - pub fn sub_free_regions<'a, 'gcx>(&self, - r_a: Region<'tcx>, - r_b: Region<'tcx>) - -> bool { - assert!(is_free_or_static(r_a) && is_free_or_static(r_b)); - if let ty::ReStatic = r_b { - true // `'a <= 'static` is just always true, and not stored in the relation explicitly - } else { - r_a == r_b || self.relation.contains(&r_a, &r_b) - } - } - /// Compute the least-upper-bound of two free regions. In some /// cases, this is more conservative than necessary, in order to /// avoid making arbitrary choices. See @@ -75,6 +61,29 @@ impl<'tcx> FreeRegionMap<'tcx> { } } +/// The NLL region handling code represents free region relations in a +/// slightly different way; this trait allows functions to be abstract +/// over which version is in use. +pub trait FreeRegionRelations<'tcx> { + /// Tests whether `r_a <= r_b`. Both must be free regions or + /// `'static`. + fn sub_free_regions(&self, shorter: ty::Region<'tcx>, longer: ty::Region<'tcx>) -> bool; +} + +impl<'tcx> FreeRegionRelations<'tcx> for FreeRegionMap<'tcx> { + fn sub_free_regions(&self, + r_a: Region<'tcx>, + r_b: Region<'tcx>) + -> bool { + assert!(is_free_or_static(r_a) && is_free_or_static(r_b)); + if let ty::ReStatic = r_b { + true // `'a <= 'static` is just always true, and not stored in the relation explicitly + } else { + r_a == r_b || self.relation.contains(&r_a, &r_b) + } + } +} + fn is_free(r: Region) -> bool { match *r { ty::ReEarlyBound(_) | ty::ReFree(_) => true, diff --git a/src/librustc/middle/free_region.rs b/src/librustc/middle/free_region.rs index ca6a5dd7f5b..1341e3515d5 100644 --- a/src/librustc/middle/free_region.rs +++ b/src/librustc/middle/free_region.rs @@ -15,7 +15,7 @@ //! `TransitiveRelation` type and use that to decide when one free //! region outlives another and so forth. -use infer::outlives::free_region_map::FreeRegionMap; +use infer::outlives::free_region_map::{FreeRegionMap, FreeRegionRelations}; use hir::def_id::DefId; use middle::region; use ty::{self, TyCtxt, Region}; diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 63c77a893c9..97352783db8 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -93,7 +93,6 @@ use rustc::ty::{self, Ty}; use rustc::infer; use rustc::infer::outlives::env::OutlivesEnvironment; use rustc::ty::adjustment; -use rustc::ty::outlives::Component; use std::mem; use std::ops::Deref; @@ -344,7 +343,10 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { body_hir_id, call_site_region); - self.constrain_anon_types(); + self.constrain_anon_types( + &self.fcx.anon_types.borrow(), + self.outlives_environment.free_region_map(), + ); } fn visit_region_obligations(&mut self, node_id: ast::NodeId) @@ -363,194 +365,6 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { self.body_id); } - /// Go through each of the existential `impl Trait` types that - /// appear in the function signature. For example, if the current - /// function is as follows: - /// - /// fn foo<'a, 'b>(..) -> (impl Bar<'a>, impl Bar<'b>) - /// - /// we would iterate through the `impl Bar<'a>` and the - /// `impl Bar<'b>` here. Remember that each of them has - /// their own "abstract type" definition created for them. As - /// we iterate, we have a `def_id` that corresponds to this - /// definition, and a set of substitutions `substs` that are - /// being supplied to this abstract typed definition in the - /// signature: - /// - /// abstract type Foo1<'x>: Bar<'x>; - /// abstract type Foo2<'x>: Bar<'x>; - /// fn foo<'a, 'b>(..) -> (Foo1<'a>, Foo2<'b>) { .. } - /// ^^^^ ^^ substs - /// def_id - /// - /// In addition, for each of the types we will have a type - /// variable `concrete_ty` containing the concrete type that - /// this function uses for `Foo1` and `Foo2`. That is, - /// conceptually, there is a constraint like: - /// - /// for<'a> (Foo1<'a> = C) - /// - /// where `C` is `concrete_ty`. For this equation to be satisfiable, - /// the type `C` can only refer to two regions: `'static` and `'a`. - /// - /// The problem is that this type `C` may contain arbitrary - /// region variables. In fact, it is fairly likely that it - /// does! Consider this possible definition of `foo`: - /// - /// fn foo<'a, 'b>(x: &'a i32, y: &'b i32) -> (impl Bar<'a>, impl Bar<'b>) { - /// (&*x, &*y) - /// } - /// - /// Here, the values for the concrete types of the two impl - /// traits will include inference variables: - /// - /// &'0 i32 - /// &'1 i32 - /// - /// Ordinarily, the subtyping rules would ensure that these are - /// sufficiently large. But since `impl Bar<'a>` isn't a specific - /// type per se, we don't get such constraints by default. This - /// is where this function comes into play. It adds extra - /// constraints to ensure that all the regions which appear in the - /// inferred type are regions that could validly appear. - /// - /// This is actually a bit of a tricky constraint in general. We - /// want to say that each variable (e.g., `'0``) can only take on - /// values that were supplied as arguments to the abstract type - /// (e.g., `'a` for `Foo1<'a>`) or `'static`, which is always in - /// scope. We don't have a constraint quite of this kind in the current - /// region checker. - /// - /// What we *do* have is the `<=` relation. So what we do is to - /// find the LUB of all the arguments that appear in the substs: - /// in this case, that would be `LUB('a) = 'a`, and then we apply - /// that as a least bound to the variables (e.g., `'a <= '0`). - /// - /// In some cases this is pretty suboptimal. Consider this example: - /// - /// fn baz<'a, 'b>() -> impl Trait<'a, 'b> { ... } - /// - /// Here, the regions `'a` and `'b` appear in the substitutions, - /// so we would generate `LUB('a, 'b)` as a kind of "minimal upper - /// bound", but that turns out be `'static` -- which is clearly - /// too strict! - fn constrain_anon_types(&mut self) { - debug!("constrain_anon_types()"); - - for (&def_id, anon_defn) in self.fcx.anon_types.borrow().iter() { - let concrete_ty = self.resolve_type(anon_defn.concrete_ty); - - debug!("constrain_anon_types: def_id={:?}", def_id); - debug!("constrain_anon_types: anon_defn={:#?}", anon_defn); - debug!("constrain_anon_types: concrete_ty={:?}", concrete_ty); - - let abstract_type_generics = self.tcx.generics_of(def_id); - - let span = self.tcx.def_span(def_id); - - // If there are required region bounds, we can just skip - // ahead. There will already be a registered region - // obligation related `concrete_ty` to those regions. - if anon_defn.has_required_region_bounds { - continue; - } - - // There were no `required_region_bounds`, - // so we have to search for a `least_region`. - // Go through all the regions used as arguments to the - // abstract type. These are the parameters to the abstract - // type; so in our example above, `substs` would contain - // `['a]` for the first impl trait and `'b` for the - // second. - let mut least_region = None; - for region_def in &abstract_type_generics.regions { - // Find the index of this region in the list of substitutions. - let index = region_def.index as usize; - - // Get the value supplied for this region from the substs. - let subst_arg = anon_defn.substs[index].as_region().unwrap(); - - // Compute the least upper bound of it with the other regions. - debug!("constrain_anon_types: least_region={:?}", least_region); - debug!("constrain_anon_types: subst_arg={:?}", subst_arg); - match least_region { - None => least_region = Some(subst_arg), - Some(lr) => { - if self.outlives_environment - .free_region_map() - .sub_free_regions(lr, subst_arg) { - // keep the current least region - } else if self.outlives_environment - .free_region_map() - .sub_free_regions(subst_arg, lr) { - // switch to `subst_arg` - least_region = Some(subst_arg); - } else { - // There are two regions (`lr` and - // `subst_arg`) which are not relatable. We can't - // find a best choice. - self.tcx - .sess - .struct_span_err(span, "ambiguous lifetime bound in `impl Trait`") - .span_label(span, - format!("neither `{}` nor `{}` outlives the other", - lr, subst_arg)) - .emit(); - - least_region = Some(self.tcx.mk_region(ty::ReEmpty)); - break; - } - } - } - } - - let least_region = least_region.unwrap_or(self.tcx.types.re_static); - debug!("constrain_anon_types: least_region={:?}", least_region); - - // Require that the type `concrete_ty` outlives - // `least_region`, modulo any type parameters that appear - // in the type, which we ignore. This is because impl - // trait values are assumed to capture all the in-scope - // type parameters. This little loop here just invokes - // `outlives` repeatedly, draining all the nested - // obligations that result. - let mut types = vec![concrete_ty]; - let bound_region = |r| self.sub_regions(infer::CallReturn(span), least_region, r); - while let Some(ty) = types.pop() { - let mut components = self.tcx.outlives_components(ty); - while let Some(component) = components.pop() { - match component { - Component::Region(r) => { - bound_region(r); - } - - Component::Param(_) => { - // ignore type parameters like `T`, they are captured - // implicitly by the `impl Trait` - } - - Component::UnresolvedInferenceVariable(_) => { - // we should get an error that more type - // annotations are needed in this case - self.tcx.sess.delay_span_bug(span, "unresolved inf var in anon"); - } - - Component::Projection(ty::ProjectionTy { substs, item_def_id: _ }) => { - for r in substs.regions() { - bound_region(r); - } - types.extend(substs.types()); - } - - Component::EscapingProjection(more_components) => { - components.extend(more_components); - } - } - } - } - } - } - fn resolve_regions_and_report_errors(&self) { self.fcx.resolve_regions_and_report_errors(self.subject_def_id, &self.region_scope_tree,