Unify the upvar variables found in closures with the actual types of the

upvars after analysis is done. Remove the `closure_upvars` helper and
just consult this list of type variables directly.

Author: nikomatsakis

src/librustc/middle/free_region.rs

@@ -40,7 +40,6 @@ impl FreeRegionMap {
                     self.relate_free_regions(free_a, free_b);
                 }
                 Implication::RegionSubRegion(..) |
-                Implication::RegionSubClosure(..) |
                 Implication::RegionSubGeneric(..) |
                 Implication::Predicate(..) => {
                 }

src/librustc/middle/implicator.rs

@@ -28,7 +28,6 @@ use util::nodemap::FnvHashSet;
 pub enum Implication<'tcx> {
     RegionSubRegion(Option<Ty<'tcx>>, ty::Region, ty::Region),
     RegionSubGeneric(Option<Ty<'tcx>>, ty::Region, GenericKind<'tcx>),
-    RegionSubClosure(Option<Ty<'tcx>>, ty::Region, ast::DefId, &'tcx ty::ClosureSubsts<'tcx>),
     Predicate(ast::DefId, ty::Predicate<'tcx>),
 }
 
@@ -96,10 +95,47 @@ impl<'a, 'tcx> Implicator<'a, 'tcx> {
                 // No borrowed content reachable here.
             }
 
-            ty::TyClosure(def_id, ref substs) => {
-                // TODO remove RegionSubClosure
-                let &(r_a, opt_ty) = self.stack.last().unwrap();
-                self.out.push(Implication::RegionSubClosure(opt_ty, r_a, def_id, substs));
+            ty::TyClosure(_, ref substs) => {
+                // FIXME(#27086). We do not accumulate from substs, since they
+                // don't represent reachable data. This means that, in
+                // practice, some of the lifetime parameters might not
+                // be in scope when the body runs, so long as there is
+                // no reachable data with that lifetime. For better or
+                // worse, this is consistent with fn types, however,
+                // which can also encapsulate data in this fashion
+                // (though it's somewhat harder, and typically
+                // requires virtual dispatch).
+                //
+                // Note that changing this (in a naive way, at least)
+                // causes regressions for what appears to be perfectly
+                // reasonable code like this:
+                //
+                // ```
+                // fn foo<'a>(p: &Data<'a>) {
+                //    bar(|q: &mut Parser| q.read_addr())
+                // }
+                // fn bar(p: Box<FnMut(&mut Parser)+'static>) {
+                // }
+                // ```
+                //
+                // Note that `p` (and `'a`) are not used in the
+                // closure at all, but to meet the requirement that
+                // the closure type `C: 'static` (so it can be coerce
+                // to the object type), we get the requirement that
+                // `'a: 'static` since `'a` appears in the closure
+                // type `C`.
+                //
+                // A smarter fix might "prune" unused `func_substs` --
+                // this would avoid breaking simple examples like
+                // this, but would still break others (which might
+                // indeed be invalid, depending on your POV). Pruning
+                // would be a subtle process, since we have to see
+                // what func/type parameters are used and unused,
+                // taking into consideration UFCS and so forth.
+
+                for &upvar_ty in &substs.upvar_tys {
+                    self.accumulate_from_ty(upvar_ty);
+                }
             }
 
             ty::TyTrait(ref t) => {
@@ -274,6 +310,21 @@ impl<'a, 'tcx> Implicator<'a, 'tcx> {
         self.out.extend(obligations);
 
         let variances = self.tcx().item_variances(def_id);
+        self.accumulate_from_substs(substs, Some(&variances));
+    }
+
+    fn accumulate_from_substs(&mut self,
+                              substs: &Substs<'tcx>,
+                              variances: Option<&ty::ItemVariances>)
+    {
+        let mut tmp_variances = None;
+        let variances = variances.unwrap_or_else(|| {
+            tmp_variances = Some(ty::ItemVariances {
+                types: substs.types.map(|_| ty::Variance::Invariant),
+                regions: substs.regions().map(|_| ty::Variance::Invariant),
+            });
+            tmp_variances.as_ref().unwrap()
+        });
 
         for (&region, &variance) in substs.regions().iter().zip(&variances.regions) {
             match variance {

src/librustc/middle/infer/mod.rs

@@ -1399,20 +1399,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             closure_ty
         }
     }
-
-    pub fn closure_upvars(&self,
-                          def_id: ast::DefId,
-                          substs: &ty::ClosureSubsts<'tcx>)
-                          -> Option<Vec<ty::ClosureUpvar<'tcx>>>
-    {
-        let result = ty::ctxt::closure_upvars(self, def_id, substs);
-
-        if self.normalize {
-            normalize_associated_type(&self.tcx, &result)
-        } else {
-            result
-        }
-    }
 }
 
 impl<'tcx> TypeTrace<'tcx> {

src/librustc/middle/traits/select.rs

@@ -1284,22 +1284,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     candidates.ambiguous = true;
                 }
                 _ => {
-                    if self.constituent_types_for_ty(self_ty).is_some() {
-                        candidates.vec.push(DefaultImplCandidate(def_id.clone()))
-                    } else {
-                        // We don't yet know what the constituent
-                        // types are. So call it ambiguous for now,
-                        // though this is a bit stronger than
-                        // necessary: that is, we know that the
-                        // defaulted impl applies, but we can't
-                        // process the confirmation step without
-                        // knowing the constituent types. (Anyway, in
-                        // the particular case of defaulted impls, it
-                        // doesn't really matter much either way,
-                        // since we won't be aiding inference by
-                        // processing the confirmation step.)
-                        candidates.ambiguous = true;
-                    }
+                    candidates.vec.push(DefaultImplCandidate(def_id.clone()))
                 }
             }
         }
@@ -1729,14 +1714,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     return ok_if(Vec::new());
                 }
 
-                // TODO
-                match self.infcx.closure_upvars(def_id, substs) {
-                    Some(upvars) => ok_if(upvars.iter().map(|c| c.ty).collect()),
-                    None => {
-                        debug!("assemble_builtin_bound_candidates: no upvar types available yet");
-                        Ok(AmbiguousBuiltin)
-                    }
-                }
+                ok_if(substs.upvar_tys.clone())
             }
 
             ty::TyStruct(def_id, substs) => {
@@ -1819,7 +1797,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     /// Bar<i32> where struct Bar<T> { x: T, y: u32 } -> [i32, u32]
     /// Zed<i32> where enum Zed { A(T), B(u32) } -> [i32, u32]
     /// ```
-    fn constituent_types_for_ty(&self, t: Ty<'tcx>) -> Option<Vec<Ty<'tcx>>> {
+    fn constituent_types_for_ty(&self, t: Ty<'tcx>) -> Vec<Ty<'tcx>> {
         match t.sty {
             ty::TyUint(_) |
             ty::TyInt(_) |
@@ -1831,7 +1809,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             ty::TyInfer(ty::IntVar(_)) |
             ty::TyInfer(ty::FloatVar(_)) |
             ty::TyChar => {
-                Some(Vec::new())
+                Vec::new()
             }
 
             ty::TyTrait(..) |
@@ -1848,56 +1826,56 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             }
 
             ty::TyBox(referent_ty) => {  // Box<T>
-                Some(vec![referent_ty])
+                vec![referent_ty]
             }
 
             ty::TyRawPtr(ty::TypeAndMut { ty: element_ty, ..}) |
             ty::TyRef(_, ty::TypeAndMut { ty: element_ty, ..}) => {
-                Some(vec![element_ty])
+                vec![element_ty]
             },
 
             ty::TyArray(element_ty, _) | ty::TySlice(element_ty) => {
-                Some(vec![element_ty])
+                vec![element_ty]
             }
 
             ty::TyTuple(ref tys) => {
                 // (T1, ..., Tn) -- meets any bound that all of T1...Tn meet
-                Some(tys.clone())
+                tys.clone()
             }
 
             ty::TyClosure(def_id, ref substs) => {
+                // FIXME(#27086). We are invariant w/r/t our
+                // substs.func_substs, but we don't see them as
+                // constituent types; this seems RIGHT but also like
+                // something that a normal type couldn't simulate. Is
+                // this just a gap with the way that PhantomData and
+                // OIBIT interact? That is, there is no way to say
+                // "make me invariant with respect to this TYPE, but
+                // do not act as though I can reach it"
                 assert_eq!(def_id.krate, ast::LOCAL_CRATE);
-
-                // TODO
-                match self.infcx.closure_upvars(def_id, substs) {
-                    Some(upvars) => {
-                        Some(upvars.iter().map(|c| c.ty).collect())
-                    }
-                    None => {
-                        None
-                    }
-                }
+                substs.upvar_tys.clone()
             }
 
             // for `PhantomData<T>`, we pass `T`
             ty::TyStruct(def_id, substs)
                 if Some(def_id) == self.tcx().lang_items.phantom_data() =>
             {
-                Some(substs.types.get_slice(TypeSpace).to_vec())
+                substs.types.get_slice(TypeSpace).to_vec()
             }
 
             ty::TyStruct(def_id, substs) => {
-                Some(self.tcx().struct_fields(def_id, substs).iter()
-                     .map(|f| f.mt.ty)
-                     .collect())
+                self.tcx().struct_fields(def_id, substs)
+                          .iter()
+                          .map(|f| f.mt.ty)
+                          .collect()
             }
 
             ty::TyEnum(def_id, substs) => {
-                Some(self.tcx().substd_enum_variants(def_id, substs)
-                     .iter()
-                     .flat_map(|variant| &variant.args)
-                     .map(|&ty| ty)
-                     .collect())
+                self.tcx().substd_enum_variants(def_id, substs)
+                          .iter()
+                          .flat_map(|variant| &variant.args)
+                          .map(|&ty| ty)
+                          .collect()
             }
         }
     }
@@ -2147,15 +2125,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         // binder is moved below
         let self_ty = self.infcx.shallow_resolve(obligation.predicate.skip_binder().self_ty());
-        match self.constituent_types_for_ty(self_ty) {
-            Some(types) => self.vtable_default_impl(obligation, trait_def_id, ty::Binder(types)),
-            None => {
-                self.tcx().sess.bug(
-                    &format!(
-                        "asked to confirm default implementation for ambiguous type: {:?}",
-                        self_ty));
-            }
-        }
+        let types = self.constituent_types_for_ty(self_ty);
+        self.vtable_default_impl(obligation, trait_def_id, ty::Binder(types))
     }
 
     fn confirm_default_impl_object_candidate(&mut self,

src/librustc/middle/ty.rs

@@ -4247,12 +4247,8 @@ impl<'tcx> TyS<'tcx> {
                     apply_lang_items(cx, did, res)
                 }
 
-                TyClosure(did, ref substs) => {
-                    // TODO
-                    let param_env = cx.empty_parameter_environment();
-                    let infcx = infer::new_infer_ctxt(cx, &cx.tables, Some(param_env), false);
-                    let upvars = infcx.closure_upvars(did, substs).unwrap();
-                    TypeContents::union(&upvars, |f| tc_ty(cx, &f.ty, cache))
+                TyClosure(_, ref substs) => {
+                    TypeContents::union(&substs.upvar_tys, |ty| tc_ty(cx, &ty, cache))
                 }
 
                 TyTuple(ref tys) => {
@@ -6007,62 +6003,6 @@ impl<'tcx> ctxt<'tcx> {
         (a, b)
     }
 
-    // Returns a list of `ClosureUpvar`s for each upvar.
-    pub fn closure_upvars<'a>(typer: &infer::InferCtxt<'a, 'tcx>,
-                              closure_id: ast::DefId,
-                              substs: &ClosureSubsts<'tcx>)
-                              -> Option<Vec<ClosureUpvar<'tcx>>>
-    {
-        // Presently an unboxed closure type cannot "escape" out of a
-        // function, so we will only encounter ones that originated in the
-        // local crate or were inlined into it along with some function.
-        // This may change if abstract return types of some sort are
-        // implemented.
-        assert!(closure_id.krate == ast::LOCAL_CRATE);
-        let tcx = typer.tcx;
-        match tcx.freevars.borrow().get(&closure_id.node) {
-            None => Some(vec![]),
-            Some(ref freevars) => {
-                freevars.iter()
-                        .map(|freevar| {
-                            let freevar_def_id = freevar.def.def_id();
-                            let freevar_ty = match typer.node_ty(freevar_def_id.node) {
-                                Ok(t) => { t }
-                                Err(()) => { return None; }
-                            };
-                            let freevar_ty = freevar_ty.subst(tcx, &substs.func_substs);
-
-                            let upvar_id = ty::UpvarId {
-                                var_id: freevar_def_id.node,
-                                closure_expr_id: closure_id.node
-                            };
-
-                            typer.upvar_capture(upvar_id).map(|capture| {
-                                let freevar_ref_ty = match capture {
-                                    UpvarCapture::ByValue => {
-                                        freevar_ty
-                                    }
-                                    UpvarCapture::ByRef(borrow) => {
-                                        tcx.mk_ref(tcx.mk_region(borrow.region),
-                                            ty::TypeAndMut {
-                                                ty: freevar_ty,
-                                                mutbl: borrow.kind.to_mutbl_lossy(),
-                                            })
-                                    }
-                                };
-
-                                ClosureUpvar {
-                                    def: freevar.def,
-                                    span: freevar.span,
-                                    ty: freevar_ref_ty,
-                                }
-                            })
-                        })
-                        .collect()
-            }
-        }
-    }
-
     // Returns the repeat count for a repeating vector expression.
     pub fn eval_repeat_count(&self, count_expr: &ast::Expr) -> usize {
         let hint = UncheckedExprHint(self.types.usize);

src/librustc/util/ppaux.rs

@@ -665,22 +665,32 @@ impl<'tcx> fmt::Display for ty::TypeVariants<'tcx> {
             TyClosure(ref did, ref substs) => ty::tls::with(|tcx| {
                 try!(write!(f, "[closure"));
 
-                // TODO consider changing this to print out the upvar types instead
-
-                let closure_tys = &tcx.tables.borrow().closure_tys;
-                try!(closure_tys.get(did).map(|cty| &cty.sig).and_then(|sig| {
-                    tcx.lift(&substs.func_substs).map(|substs| sig.subst(tcx, substs))
-                }).map(|sig| {
-                    fn_sig(f, &sig.0.inputs, false, sig.0.output)
-                }).unwrap_or_else(|| {
-                    if did.krate == ast::LOCAL_CRATE {
-                        try!(write!(f, " {:?}", tcx.map.span(did.node)));
+                if did.krate == ast::LOCAL_CRATE {
+                    try!(write!(f, "@{:?}", tcx.map.span(did.node)));
+                    let mut sep = " ";
+                    try!(tcx.with_freevars(did.node, |freevars| {
+                        for (freevar, upvar_ty) in freevars.iter().zip(&substs.upvar_tys) {
+                            let node_id = freevar.def.local_node_id();
+                            try!(write!(f,
+                                        "{}{}:{}",
+                                        sep,
+                                        tcx.local_var_name_str(node_id),
+                                        upvar_ty));
+                            sep = ", ";
+                        }
+                        Ok(())
+                    }))
+                } else {
+                    // cross-crate closure types should only be
+                    // visible in trans bug reports, I imagine.
+                    try!(write!(f, "@{:?}", did));
+                    let mut sep = " ";
+                    for (index, upvar_ty) in substs.upvar_tys.iter().enumerate() {
+                        try!(write!(f, "{}{}:{}", sep, index, upvar_ty));
+                        sep = ", ";
                     }
-                    Ok(())
-                }));
-                if verbose() {
-                    try!(write!(f, " id={:?}", did));
                 }
+
                 write!(f, "]")
             }),
             TyArray(ty, sz) => write!(f, "[{}; {}]",  ty, sz),