rustc: remove the closure ID from hir::Upvar's parent field.

Author: eddyb

src/librustc/hir/mod.rs

@@ -2497,8 +2497,8 @@ pub struct Upvar<Id = HirId> {
     /// The variable being captured.
     pub var_id: Id,
 
-    /// The parent closure, if this is not a direct capture.
-    pub parent: Option<ast::NodeId>,
+    /// Whether this is not a direct capture (comes from parent closure).
+    pub has_parent: bool,
 
     // First span where it is accessed (there can be multiple).
     pub span: Span
@@ -2508,7 +2508,7 @@ impl<Id: fmt::Debug + Copy> Upvar<Id> {
     pub fn map_id<R>(self, map: impl FnOnce(Id) -> R) -> Upvar<R> {
         Upvar {
             var_id: map(self.var_id),
-            parent: self.parent,
+            has_parent: self.has_parent,
             span: self.span,
         }
     }

src/librustc/middle/expr_use_visitor.rs

@@ -961,12 +961,16 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
                         -> mc::McResult<mc::cmt_<'tcx>> {
         // Create the cmt for the variable being borrowed, from the
         // caller's perspective
-        let var_ty = self.mc.node_ty(upvar.var_id)?;
-        let res = upvar.parent.map_or(
-            Res::Local(upvar.var_id),
-            |closure_node_id| Res::Upvar(upvar.var_id, closure_node_id),
-        );
-        self.mc.cat_res(closure_hir_id, closure_span, var_ty, res)
+        if upvar.has_parent {
+            let closure_def_id = self.tcx().hir().local_def_id_from_hir_id(closure_hir_id);
+            let parent_def_id = self.tcx().parent(closure_def_id).unwrap();
+            assert!(self.tcx().is_closure(parent_def_id));
+            let var_nid = self.tcx().hir().hir_to_node_id(upvar.var_id);
+            self.mc.cat_upvar(closure_hir_id, closure_span, var_nid, parent_def_id)
+        } else {
+            let var_ty = self.mc.node_ty(upvar.var_id)?;
+            self.mc.cat_res(closure_hir_id, closure_span, var_ty, Res::Local(upvar.var_id))
+        }
     }
 }
 

src/librustc/middle/liveness.rs

@@ -486,7 +486,7 @@ fn visit_expr<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, expr: &'tcx Expr) {
         let closure_def_id = ir.tcx.hir().local_def_id_from_hir_id(expr.hir_id);
         if let Some(upvars) = ir.tcx.upvars(closure_def_id) {
             call_caps.extend(upvars.iter().filter_map(|upvar| {
-                if upvar.parent.is_none() {
+                if !upvar.has_parent {
                     let upvar_ln = ir.add_live_node(UpvarNode(upvar.span));
                     Some(CaptureInfo { ln: upvar_ln, var_hid: upvar.var_id })
                 } else {

src/librustc/middle/mem_categorization.rs

@@ -737,9 +737,10 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
                 })
             }
 
-            Res::Upvar(var_id, fn_node_id) => {
+            Res::Upvar(var_id, closure_node_id) => {
                 let var_nid = self.tcx.hir().hir_to_node_id(var_id);
-                self.cat_upvar(hir_id, span, var_nid, fn_node_id)
+                let closure_expr_def_id = self.tcx.hir().local_def_id(closure_node_id);
+                self.cat_upvar(hir_id, span, var_nid, closure_expr_def_id)
             }
 
             Res::Local(vid) => {
@@ -760,15 +761,13 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
 
     // Categorize an upvar, complete with invisible derefs of closure
     // environment and upvar reference as appropriate.
-    fn cat_upvar(&self,
-                 hir_id: hir::HirId,
-                 span: Span,
-                 var_id: ast::NodeId,
-                 fn_node_id: ast::NodeId)
-                 -> McResult<cmt_<'tcx>>
-    {
-        let fn_hir_id = self.tcx.hir().node_to_hir_id(fn_node_id);
-
+    pub fn cat_upvar(
+        &self,
+        hir_id: hir::HirId,
+        span: Span,
+        var_id: ast::NodeId,
+        closure_expr_def_id: DefId,
+    ) -> McResult<cmt_<'tcx>> {
         // An upvar can have up to 3 components. We translate first to a
         // `Categorization::Upvar`, which is itself a fiction -- it represents the reference to the
         // field from the environment.
@@ -792,6 +791,9 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
         // FnMut          | copied -> &'env mut  | upvar -> &'env mut -> &'up bk
         // FnOnce         | copied               | upvar -> &'up bk
 
+        let fn_hir_id = self.tcx.hir().local_def_id_to_hir_id(
+            LocalDefId::from_def_id(closure_expr_def_id),
+        );
         let ty = self.node_ty(fn_hir_id)?;
         let kind = match ty.sty {
             ty::Generator(..) => ty::ClosureKind::FnOnce,
@@ -813,7 +815,6 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
             _ => span_bug!(span, "unexpected type for fn in mem_categorization: {:?}", ty),
         };
 
-        let closure_expr_def_id = self.tcx.hir().local_def_id(fn_node_id);
         let var_hir_id = self.tcx.hir().node_to_hir_id(var_id);
         let upvar_id = ty::UpvarId {
             var_path: ty::UpvarPath { hir_id: var_hir_id },

src/librustc_mir/hair/cx/expr.rs

@@ -6,7 +6,7 @@ use crate::hair::util::UserAnnotatedTyHelpers;
 use rustc_data_structures::indexed_vec::Idx;
 use rustc::hir::def::{CtorOf, Res, DefKind, CtorKind};
 use rustc::mir::interpret::{GlobalId, ErrorHandled, ConstValue};
-use rustc::ty::{self, AdtKind, Ty};
+use rustc::ty::{self, AdtKind, DefIdTree, Ty};
 use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability, PointerCast};
 use rustc::ty::subst::{InternalSubsts, SubstsRef};
 use rustc::hir;
@@ -960,38 +960,50 @@ fn convert_path_expr<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
 
         Res::Def(DefKind::Static, id) => ExprKind::StaticRef { id },
 
-        Res::Local(..) | Res::Upvar(..) => convert_var(cx, expr, res),
+        Res::Local(var_hir_id) => convert_var(cx, expr, var_hir_id),
+        Res::Upvar(var_hir_id, closure_node_id) => {
+            let closure_def_id = cx.tcx.hir().local_def_id(closure_node_id);
+            assert_eq!(cx.body_owner, closure_def_id);
+            assert!(cx.upvar_indices.contains_key(&var_hir_id));
+
+            convert_var(cx, expr, var_hir_id)
+        }
 
         _ => span_bug!(expr.span, "res `{:?}` not yet implemented", res),
     }
 }
 
-fn convert_var<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
-                               expr: &'tcx hir::Expr,
-                               res: Res)
-                               -> ExprKind<'tcx> {
-    let temp_lifetime = cx.region_scope_tree.temporary_scope(expr.hir_id.local_id);
+fn convert_var(
+    cx: &mut Cx<'_, '_, 'tcx>,
+    expr: &'tcx hir::Expr,
+    var_hir_id: hir::HirId,
+) -> ExprKind<'tcx> {
+    let upvar_index = cx.upvar_indices.get(&var_hir_id).cloned();
 
-    match res {
-        Res::Local(id) => ExprKind::VarRef { id },
+    debug!("convert_var({:?}): upvar_index={:?}, body_owner={:?}",
+           var_hir_id, upvar_index, cx.body_owner);
+
+    let temp_lifetime = cx.region_scope_tree.temporary_scope(expr.hir_id.local_id);
 
-        Res::Upvar(var_hir_id, closure_expr_id) => {
-            let index = cx.upvar_indices[&var_hir_id];
+    match upvar_index {
+        None => ExprKind::VarRef { id: var_hir_id },
 
-            debug!("convert_var(upvar({:?}, {:?}, {:?}))",
-                   var_hir_id,
-                   index,
-                   closure_expr_id);
+        Some(upvar_index) => {
+            let closure_def_id = cx.body_owner;
+            let upvar_id = ty::UpvarId {
+                var_path: ty::UpvarPath {hir_id: var_hir_id},
+                closure_expr_id: LocalDefId::from_def_id(closure_def_id),
+            };
             let var_ty = cx.tables().node_type(var_hir_id);
 
             // FIXME free regions in closures are not right
-            let closure_ty = cx.tables()
-                               .node_type(cx.tcx.hir().node_to_hir_id(closure_expr_id));
+            let closure_ty = cx.tables().node_type(
+                cx.tcx.hir().local_def_id_to_hir_id(upvar_id.closure_expr_id),
+            );
 
             // FIXME we're just hard-coding the idea that the
             // signature will be &self or &mut self and hence will
             // have a bound region with number 0
-            let closure_def_id = cx.tcx.hir().local_def_id(closure_expr_id);
             let region = ty::ReFree(ty::FreeRegion {
                 scope: closure_def_id,
                 bound_region: ty::BoundRegion::BrAnon(0),
@@ -1062,15 +1074,11 @@ fn convert_var<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
             // at this point we have `self.n`, which loads up the upvar
             let field_kind = ExprKind::Field {
                 lhs: self_expr.to_ref(),
-                name: Field::new(index),
+                name: Field::new(upvar_index),
             };
 
             // ...but the upvar might be an `&T` or `&mut T` capture, at which
             // point we need an implicit deref
-            let upvar_id = ty::UpvarId {
-                var_path: ty::UpvarPath {hir_id: var_hir_id},
-                closure_expr_id: LocalDefId::from_def_id(closure_def_id),
-            };
             match cx.tables().upvar_capture(upvar_id) {
                 ty::UpvarCapture::ByValue => field_kind,
                 ty::UpvarCapture::ByRef(borrow) => {
@@ -1089,8 +1097,6 @@ fn convert_var<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
                 }
             }
         }
-
-        _ => span_bug!(expr.span, "type of & not region"),
     }
 }
 
@@ -1190,15 +1196,16 @@ fn capture_upvar<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
     let upvar_capture = cx.tables().upvar_capture(upvar_id);
     let temp_lifetime = cx.region_scope_tree.temporary_scope(closure_expr.hir_id.local_id);
     let var_ty = cx.tables().node_type(upvar.var_id);
-    let upvar_res = upvar.parent.map_or(
-        Res::Local(upvar.var_id),
-        |closure_node_id| Res::Upvar(upvar.var_id, closure_node_id),
-    );
+    if upvar.has_parent {
+        let closure_def_id = upvar_id.closure_expr_id.to_def_id();
+        assert_eq!(cx.body_owner, cx.tcx.parent(closure_def_id).unwrap());
+    }
+    assert_eq!(upvar.has_parent, cx.upvar_indices.contains_key(&upvar.var_id));
     let captured_var = Expr {
         temp_lifetime,
         ty: var_ty,
         span: closure_expr.span,
-        kind: convert_var(cx, closure_expr, upvar_res),
+        kind: convert_var(cx, closure_expr, upvar.var_id),
     };
     match upvar_capture {
         ty::UpvarCapture::ByValue => captured_var.to_ref(),

src/librustc_mir/hair/cx/mod.rs

@@ -39,6 +39,9 @@ pub struct Cx<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
     /// `const`, or the body of a `const fn`.
     constness: hir::Constness,
 
+    /// The `DefId` of the owner of this body.
+    body_owner: DefId,
+
     /// What kind of body is being compiled.
     pub body_owner_kind: hir::BodyOwnerKind,
 
@@ -97,6 +100,7 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
             region_scope_tree: tcx.region_scope_tree(src_def_id),
             tables,
             constness,
+            body_owner: src_def_id,
             body_owner_kind,
             check_overflow,
             control_flow_destroyed: Vec::new(),

src/librustc_resolve/lib.rs

@@ -4051,9 +4051,9 @@ impl<'a> Resolver<'a> {
                             // Nothing to do. Continue.
                         }
                         ClosureRibKind(function_id) => {
-                            let parent = match res {
-                                Res::Upvar(_, closure) => Some(closure),
-                                _ => None,
+                            let has_parent = match res {
+                                Res::Upvar(..) => true,
+                                _ => false,
                             };
 
                             let seen = self.upvars_seen
@@ -4071,7 +4071,7 @@ impl<'a> Resolver<'a> {
                             if record_used {
                                 vec.push(Upvar {
                                     var_id,
-                                    parent,
+                                    has_parent,
                                     span,
                                 });
                                 seen.insert(var_id);