From 9fe0052e542747ead0d8861349326d54f0c0903d Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 7 May 2019 04:40:36 +0300 Subject: [PATCH] rustc: remove the closure ID from hir::Upvar's parent field. --- src/librustc/hir/mod.rs | 6 +- src/librustc/middle/expr_use_visitor.rs | 16 ++++-- src/librustc/middle/liveness.rs | 2 +- src/librustc/middle/mem_categorization.rs | 25 +++++---- src/librustc_mir/hair/cx/expr.rs | 67 +++++++++++++---------- src/librustc_mir/hair/cx/mod.rs | 4 ++ src/librustc_resolve/lib.rs | 8 +-- 7 files changed, 72 insertions(+), 56 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 547a7a7930c..66535079e1d 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2497,8 +2497,8 @@ pub struct Upvar { /// The variable being captured. pub var_id: Id, - /// The parent closure, if this is not a direct capture. - pub parent: Option, + /// 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 Upvar { pub fn map_id(self, map: impl FnOnce(Id) -> R) -> Upvar { Upvar { var_id: map(self.var_id), - parent: self.parent, + has_parent: self.has_parent, span: self.span, } } diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index b10acc987a2..034f60875e7 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -961,12 +961,16 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { -> mc::McResult> { // 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)) + } } } diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index e19d713d561..22930cc1989 100644 --- a/src/librustc/middle/liveness.rs +++ b/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 { diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index c58ce762773..7ce729bb715 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/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> - { - 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> { // 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 }, diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 870de08264d..4d32f1eec44 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/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> { +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(); + + 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); - match res { - Res::Local(id) => ExprKind::VarRef { id }, + match upvar_index { + None => ExprKind::VarRef { id: var_hir_id }, - Res::Upvar(var_hir_id, closure_expr_id) => { - let index = cx.upvar_indices[&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(), diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index 74382bb1fe2..251bc24bbd2 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/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(), diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 490d5ae83b8..790c4356630 100644 --- a/src/librustc_resolve/lib.rs +++ b/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);