From 1cc7621dec567ded8965fd6c01d80104cfec4d68 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 12 Apr 2017 12:51:19 -0400 Subject: [PATCH] simplify code to remove now unused "stack" and fix comments --- src/librustc/infer/combine.rs | 133 ++++++++++------------------ src/librustc/infer/type_variable.rs | 7 +- 2 files changed, 51 insertions(+), 89 deletions(-) diff --git a/src/librustc/infer/combine.rs b/src/librustc/infer/combine.rs index 03ed654e3cce..b73079b02bdd 100644 --- a/src/librustc/infer/combine.rs +++ b/src/librustc/infer/combine.rs @@ -47,7 +47,6 @@ use ty::relate::{RelateResult, TypeRelation}; use traits::PredicateObligations; use syntax::ast; -use syntax::util::small_vector::SmallVector; use syntax_pos::Span; #[derive(Clone)] @@ -174,6 +173,15 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> { Glb::new(self, a_is_expected) } + /// Here dir is either EqTo, SubtypeOf, or SupertypeOf. The + /// idea is that we should ensure that the type `a_ty` is equal + /// to, a subtype of, or a supertype of (respectively) the type + /// to which `b_vid` is bound. + /// + /// Since `b_vid` has not yet been instantiated with a type, we + /// will first instantiate `b_vid` with a *generalized* version + /// of `a_ty`. Generalization introduces other inference + /// variables wherever subtyping could occur. pub fn instantiate(&mut self, a_ty: Ty<'tcx>, dir: RelationDir, @@ -183,92 +191,49 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> { { use self::RelationDir::*; - // We use SmallVector here instead of Vec because this code is hot and - // it's rare that the stack length exceeds 1. - let mut stack = SmallVector::new(); - stack.push((a_ty, dir, b_vid)); - loop { - // For each turn of the loop, we extract a tuple - // - // (a_ty, dir, b_vid) - // - // to relate. Here dir is either SubtypeOf or - // SupertypeOf. The idea is that we should ensure that - // the type `a_ty` is a subtype or supertype (respectively) of the - // type to which `b_vid` is bound. - // - // If `b_vid` has not yet been instantiated with a type - // (which is always true on the first iteration, but not - // necessarily true on later iterations), we will first - // instantiate `b_vid` with a *generalized* version of - // `a_ty`. Generalization introduces other inference - // variables wherever subtyping could occur (at time of - // this writing, this means replacing free regions with - // region variables). - let (a_ty, dir, b_vid) = match stack.pop() { - None => break, - Some(e) => e, - }; - // Get the actual variable that b_vid has been inferred to - let (b_vid, b_ty) = { - let mut variables = self.infcx.type_variables.borrow_mut(); - let b_vid = variables.root_var(b_vid); - (b_vid, variables.probe_root(b_vid)) - }; + // Get the actual variable that b_vid has been inferred to + debug_assert!(self.infcx.type_variables.borrow_mut().probe(b_vid).is_none()); - debug!("instantiate(a_ty={:?} dir={:?} b_vid={:?})", - a_ty, - dir, - b_vid); + debug!("instantiate(a_ty={:?} dir={:?} b_vid={:?})", a_ty, dir, b_vid); - // Check whether `vid` has been instantiated yet. If not, - // make a generalized form of `ty` and instantiate with - // that. - // - // FIXME(#18653) -- we need to generalize nested type - // variables too. - let b_ty = match b_ty { - Some(t) => t, // ...already instantiated. - None => { // ...not yet instantiated: - // Generalize type if necessary. - let generalized_ty = match dir { - EqTo => self.generalize(a_ty, b_vid, false), - SupertypeOf | SubtypeOf => self.generalize(a_ty, b_vid, true), - }?; - debug!("instantiate(a_ty={:?}, dir={:?}, \ - b_vid={:?}, generalized_ty={:?})", - a_ty, dir, b_vid, - generalized_ty); - self.infcx.type_variables - .borrow_mut() - .instantiate(b_vid, generalized_ty); - generalized_ty - } - }; + // Generalize type of `a_ty` appropriately depending on the + // direction. As an example, assume: + // + // - `a_ty == &'x ?1`, where `'x` is some free region and `?1` is an + // inference variable, + // - and `dir` == `SubtypeOf`. + // + // Then the generalized form `b_ty` would be `&'?2 ?3`, where + // `'?2` and `?3` are fresh region/type inference + // variables. (Down below, we will relate `a_ty <: b_ty`, + // adding constraints like `'x: '?2` and `?1 <: ?3`.) + let b_ty = self.generalize(a_ty, b_vid, dir == EqTo)?; + debug!("instantiate(a_ty={:?}, dir={:?}, b_vid={:?}, generalized b_ty={:?})", + a_ty, dir, b_vid, b_ty); + self.infcx.type_variables.borrow_mut().instantiate(b_vid, b_ty); - // The original triple was `(a_ty, dir, b_vid)` -- now we have - // resolved `b_vid` to `b_ty`, so apply `(a_ty, dir, b_ty)`: - // - // FIXME(#16847): This code is non-ideal because all these subtype - // relations wind up attributed to the same spans. We need - // to associate causes/spans with each of the relations in - // the stack to get this right. - match dir { - EqTo => self.equate(a_is_expected).relate(&a_ty, &b_ty), - SubtypeOf => self.sub(a_is_expected).relate(&a_ty, &b_ty), - SupertypeOf => self.sub(a_is_expected).relate_with_variance( - ty::Contravariant, &a_ty, &b_ty), - }?; - } + // Finally, relate `b_ty` to `a_ty`, as described in previous comment. + // + // FIXME(#16847): This code is non-ideal because all these subtype + // relations wind up attributed to the same spans. We need + // to associate causes/spans with each of the relations in + // the stack to get this right. + match dir { + EqTo => self.equate(a_is_expected).relate(&a_ty, &b_ty), + SubtypeOf => self.sub(a_is_expected).relate(&a_ty, &b_ty), + SupertypeOf => self.sub(a_is_expected).relate_with_variance( + ty::Contravariant, &a_ty, &b_ty), + }?; Ok(()) } /// Attempts to generalize `ty` for the type variable `for_vid`. /// This checks for cycle -- that is, whether the type `ty` - /// references `for_vid`. If `make_region_vars` is true, it will - /// also replace all regions with fresh variables. Returns - /// `TyError` in the case of a cycle, `Ok` otherwise. + /// references `for_vid`. If `is_eq_relation` is false, it will + /// also replace all regions/unbound-type-variables with fresh + /// variables. Returns `TyError` in the case of a cycle, `Ok` + /// otherwise. /// /// Preconditions: /// @@ -276,16 +241,14 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> { fn generalize(&self, ty: Ty<'tcx>, for_vid: ty::TyVid, - make_region_vars: bool) + is_eq_relation: bool) -> RelateResult<'tcx, Ty<'tcx>> { - debug_assert!(self.infcx.type_variables.borrow_mut().root_var(for_vid) == for_vid); - let mut generalize = Generalizer { infcx: self.infcx, span: self.trace.cause.span, for_vid_sub_root: self.infcx.type_variables.borrow_mut().sub_root_var(for_vid), - make_region_vars: make_region_vars, + is_eq_relation: is_eq_relation, cycle_detected: false }; let u = ty.fold_with(&mut generalize); @@ -301,7 +264,7 @@ struct Generalizer<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>, span: Span, for_vid_sub_root: ty::TyVid, - make_region_vars: bool, + is_eq_relation: bool, cycle_detected: bool, } @@ -332,7 +295,7 @@ impl<'cx, 'gcx, 'tcx> ty::fold::TypeFolder<'gcx, 'tcx> for Generalizer<'cx, 'gcx self.fold_ty(u) } None => { - if self.make_region_vars { + if !self.is_eq_relation { let origin = variables.origin(vid); let new_var_id = variables.new_var(false, origin, None); let u = self.tcx().mk_var(new_var_id); @@ -379,7 +342,7 @@ impl<'cx, 'gcx, 'tcx> ty::fold::TypeFolder<'gcx, 'tcx> for Generalizer<'cx, 'gcx ty::ReScope(..) | ty::ReVar(..) | ty::ReFree(..) => { - if !self.make_region_vars { + if self.is_eq_relation { return r; } } diff --git a/src/librustc/infer/type_variable.rs b/src/librustc/infer/type_variable.rs index 34bb9feb5c92..4ae2a8026409 100644 --- a/src/librustc/infer/type_variable.rs +++ b/src/librustc/infer/type_variable.rs @@ -151,11 +151,10 @@ impl<'tcx> TypeVariableTable<'tcx> { /// Instantiates `vid` with the type `ty`. /// - /// Precondition: `vid` must be a root in the unification table - /// and has not previously been instantiated. + /// Precondition: `vid` must not have been previously instantiated. pub fn instantiate(&mut self, vid: ty::TyVid, ty: Ty<'tcx>) { - debug_assert!(self.root_var(vid) == vid); - debug_assert!(self.probe(vid).is_none()); + let vid = self.root_var(vid); + debug_assert!(self.probe_root(vid).is_none()); let old_value = { let vid_data = &mut self.values[vid.index as usize];