generalize type variables too

When we are generalizing a super/sub-type, we have to replace type
variables with a fresh variable (and not just region variables).  So if
we know that `Box<?T> <: ?U`, for example, we instantiate `?U` with
`Box<?V>` and then relate `Box<?T>` to `Box<?V>` (and hence require that
`?T <: ?V`).

This change has some complex interactions, however:

First, the occurs check must be updated to detect constraints like `?T
<: ?U` and `?U <: Box<?T>`. If we're not careful, we'll create a
never-ending sequence of new variables. To address this, we add a second
unification set into `type_variables` that tracks type variables related
through **either** equality **or** subtyping, and use that during the
occurs-check.

Second, the "fudge regions if ok" code was expecting no new type
variables to be created. It must be updated to create new type variables
outside of the probe. This is relatively straight-forward under the new
scheme, since type variables are now independent from one another, and
any relations are moderated by pending subtype obliations and so forth.
This part would be tricky to backport though.

cc rust-lang#18653
cc rust-lang#40951

Author: nikomatsakis

src/librustc/infer/combine.rs

@@ -264,20 +264,27 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> {
         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.
+    /// 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.
+    ///
+    /// Preconditions:
+    ///
+    /// - `for_vid` is a "root vid"
     fn generalize(&self,
                   ty: Ty<'tcx>,
                   for_vid: ty::TyVid,
                   make_region_vars: 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: for_vid,
+            for_vid_sub_root: self.infcx.type_variables.borrow_mut().sub_root_var(for_vid),
             make_region_vars: make_region_vars,
             cycle_detected: false
         };
@@ -293,7 +300,7 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> {
 struct Generalizer<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
     infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>,
     span: Span,
-    for_vid: ty::TyVid,
+    for_vid_sub_root: ty::TyVid,
     make_region_vars: bool,
     cycle_detected: bool,
 }
@@ -305,17 +312,17 @@ impl<'cx, 'gcx, 'tcx> ty::fold::TypeFolder<'gcx, 'tcx> for Generalizer<'cx, 'gcx
 
     fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
         // Check to see whether the type we are genealizing references
-        // `vid`. At the same time, also update any type variables to
-        // the values that they are bound to. This is needed to truly
-        // check for cycles, but also just makes things readable.
-        //
-        // (In particular, you could have something like `$0 = Box<$1>`
-        //  where `$1` has already been instantiated with `Box<$0>`)
+        // any other type variable related to `vid` via
+        // subtyping. This is basically our "occurs check", preventing
+        // us from creating infinitely sized types.
         match t.sty {
             ty::TyInfer(ty::TyVar(vid)) => {
                 let mut variables = self.infcx.type_variables.borrow_mut();
                 let vid = variables.root_var(vid);
-                if vid == self.for_vid {
+                let sub_vid = variables.sub_root_var(vid);
+                if sub_vid == self.for_vid_sub_root {
+                    // If sub-roots are equal, then `for_vid` and
+                    // `vid` are related via subtyping.
                     self.cycle_detected = true;
                     self.tcx().types.err
                 } else {
@@ -324,7 +331,18 @@ impl<'cx, 'gcx, 'tcx> ty::fold::TypeFolder<'gcx, 'tcx> for Generalizer<'cx, 'gcx
                             drop(variables);
                             self.fold_ty(u)
                         }
-                        None => t,
+                        None => {
+                            if self.make_region_vars {
+                                let origin = variables.origin(vid);
+                                let new_var_id = variables.new_var(false, origin, None);
+                                let u = self.tcx().mk_var(new_var_id);
+                                debug!("generalize: replacing original vid={:?} with new={:?}",
+                                       vid, u);
+                                u
+                            } else {
+                                t
+                            }
+                        }
                     }
                 }
             }

src/librustc/infer/fudge.rs

@@ -8,7 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use ty::{self, TyCtxt};
+use infer::type_variable::TypeVariableMap;
+use ty::{self, Ty, TyCtxt};
 use ty::fold::{TypeFoldable, TypeFolder};
 
 use super::InferCtxt;
@@ -54,57 +55,52 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     /// the actual types (`?T`, `Option<?T`) -- and remember that
     /// after the snapshot is popped, the variable `?T` is no longer
     /// unified.
-    ///
-    /// Assumptions:
-    /// - no new type variables are created during `f()` (asserted
-    ///   below); this simplifies our logic since we don't have to
-    ///   check for escaping type variables
     pub fn fudge_regions_if_ok<T, E, F>(&self,
                                         origin: &RegionVariableOrigin,
                                         f: F) -> Result<T, E> where
         F: FnOnce() -> Result<T, E>,
         T: TypeFoldable<'tcx>,
     {
-        let (region_vars, value) = self.probe(|snapshot| {
-            let vars_at_start = self.type_variables.borrow().num_vars();
+        debug!("fudge_regions_if_ok(origin={:?})", origin);
 
+        let (type_variables, region_vars, value) = self.probe(|snapshot| {
             match f() {
                 Ok(value) => {
                     let value = self.resolve_type_vars_if_possible(&value);
 
                     // At this point, `value` could in principle refer
-                    // to regions that have been created during the
-                    // snapshot (we assert below that `f()` does not
-                    // create any new type variables, so there
-                    // shouldn't be any of those). Once we exit
-                    // `probe()`, those are going to be popped, so we
-                    // will have to eliminate any references to them.
-
-                    assert_eq!(self.type_variables.borrow().num_vars(), vars_at_start,
-                               "type variables were created during fudge_regions_if_ok");
+                    // to types/regions that have been created during
+                    // the snapshot. Once we exit `probe()`, those are
+                    // going to be popped, so we will have to
+                    // eliminate any references to them.
+
+                    let type_variables =
+                        self.type_variables.borrow_mut().types_created_since_snapshot(
+                            &snapshot.type_snapshot);
                     let region_vars =
                         self.region_vars.vars_created_since_snapshot(
                             &snapshot.region_vars_snapshot);
 
-                    Ok((region_vars, value))
+                    Ok((type_variables, region_vars, value))
                 }
                 Err(e) => Err(e),
             }
         })?;
 
         // At this point, we need to replace any of the now-popped
-        // region variables that appear in `value` with a fresh region
-        // variable. We can't do this during the probe because they
-        // would just get popped then too. =)
+        // type/region variables that appear in `value` with a fresh
+        // variable of the appropriate kind. We can't do this during
+        // the probe because they would just get popped then too. =)
 
         // Micro-optimization: if no variables have been created, then
         // `value` can't refer to any of them. =) So we can just return it.
-        if region_vars.is_empty() {
+        if type_variables.is_empty() && region_vars.is_empty() {
             return Ok(value);
         }
 
         let mut fudger = RegionFudger {
             infcx: self,
+            type_variables: &type_variables,
             region_vars: &region_vars,
             origin: origin
         };
@@ -115,6 +111,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
 
 pub struct RegionFudger<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
+    type_variables: &'a TypeVariableMap,
     region_vars: &'a Vec<ty::RegionVid>,
     origin: &'a RegionVariableOrigin,
 }
@@ -124,6 +121,40 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for RegionFudger<'a, 'gcx, 'tcx> {
         self.infcx.tcx
     }
 
+    fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
+        match ty.sty {
+            ty::TyInfer(ty::InferTy::TyVar(vid)) => {
+                match self.type_variables.get(&vid) {
+                    None => {
+                        // This variable was created before the
+                        // "fudging".  Since we refresh all type
+                        // variables to their binding anyhow, we know
+                        // that it is unbound, so we can just return
+                        // it.
+                        debug_assert!(self.infcx.type_variables.borrow_mut().probe(vid).is_none());
+                        ty
+                    }
+
+                    Some(info) => {
+                        // This variable was created during the
+                        // fudging; it was mapped the root
+                        // `root_vid`. There are now two
+                        // possibilities: either the root was creating
+                        // during the fudging too, in which case we
+                        // want a fresh variable, or it was not, in
+                        // which case we can return it.
+                        if self.type_variables.contains_key(&info.root_vid) {
+                            self.infcx.next_ty_var(info.root_origin)
+                        } else {
+                            self.infcx.tcx.mk_var(info.root_vid)
+                        }
+                    }
+                }
+            }
+            _ => ty.super_fold_with(self),
+        }
+    }
+
     fn fold_region(&mut self, r: &'tcx ty::Region) -> &'tcx ty::Region {
         match *r {
             ty::ReVar(v) if self.region_vars.contains(&v) => {

src/librustc/infer/mod.rs

@@ -1036,9 +1036,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         self.probe(|_| {
             let origin = &ObligationCause::dummy();
             let trace = TypeTrace::types(origin, true, a, b);
-            self.sub(true, trace, &a, &b).map(|InferOk { obligations, .. }| {
-                // FIXME(#32730) propagate obligations
-                assert!(obligations.is_empty());
+            self.sub(true, trace, &a, &b).map(|InferOk { obligations: _, .. }| {
+                // Ignore obligations, since we are unrolling
+                // everything anyway.
             })
         })
     }

src/librustc/infer/sub.rs

@@ -80,15 +80,19 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
         let a = infcx.type_variables.borrow_mut().replace_if_possible(a);
         let b = infcx.type_variables.borrow_mut().replace_if_possible(b);
         match (&a.sty, &b.sty) {
-            (&ty::TyInfer(TyVar(_)), &ty::TyInfer(TyVar(_))) => {
+            (&ty::TyInfer(TyVar(a_vid)), &ty::TyInfer(TyVar(b_vid))) => {
                 // Shouldn't have any LBR here, so we can safely put
                 // this under a binder below without fear of accidental
                 // capture.
                 assert!(!a.has_escaping_regions());
                 assert!(!b.has_escaping_regions());
 
                 // can't make progress on `A <: B` if both A and B are
-                // type variables, so record an obligation.
+                // type variables, so record an obligation. We also
+                // have to record in the `type_variables` tracker that
+                // the two variables are equal modulo subtyping, which
+                // is important to the occurs check later on.
+                infcx.type_variables.borrow_mut().sub(a_vid, b_vid);
                 self.fields.obligations.push(
                     Obligation::new(
                         self.fields.trace.cause.clone(),