Auto merge of #44316 - eddyb:no-local-var-def-id, r=michaelwoerister

Use NodeId/HirId instead of DefId for local variables.

r? @michaelwoerister

src/librustc/hir/def.rs

@@ -48,8 +48,9 @@ pub enum Def {
     VariantCtor(DefId, CtorKind),
     Method(DefId),
     AssociatedConst(DefId),
-    Local(DefId),
-    Upvar(DefId,        // def id of closed over local
+
+    Local(ast::NodeId),
+    Upvar(ast::NodeId,  // node id of closed over local
           usize,        // index in the freevars list of the closure
           ast::NodeId), // expr node that creates the closure
     Label(ast::NodeId),
@@ -150,11 +151,13 @@ impl Def {
             Def::Variant(id) | Def::VariantCtor(id, ..) | Def::Enum(id) | Def::TyAlias(id) |
             Def::AssociatedTy(id) | Def::TyParam(id) | Def::Struct(id) | Def::StructCtor(id, ..) |
             Def::Union(id) | Def::Trait(id) | Def::Method(id) | Def::Const(id) |
-            Def::AssociatedConst(id) | Def::Local(id) | Def::Upvar(id, ..) | Def::Macro(id, ..) |
+            Def::AssociatedConst(id) | Def::Macro(id, ..) |
             Def::GlobalAsm(id) => {
                 id
             }
 
+            Def::Local(..) |
+            Def::Upvar(..) |
             Def::Label(..)  |
             Def::PrimTy(..) |
             Def::SelfTy(..) |

src/librustc/hir/intravisit.rs

@@ -653,8 +653,8 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat) {
         PatKind::Ref(ref subpattern, _) => {
             visitor.visit_pat(subpattern)
         }
-        PatKind::Binding(_, def_id, ref pth1, ref optional_subpattern) => {
-            visitor.visit_def_mention(Def::Local(def_id));
+        PatKind::Binding(_, canonical_id, ref pth1, ref optional_subpattern) => {
+            visitor.visit_def_mention(Def::Local(canonical_id));
             visitor.visit_name(pth1.span, pth1.node);
             walk_list!(visitor, visit_pat, optional_subpattern);
         }

src/librustc/hir/lowering.rs

@@ -41,8 +41,7 @@
 //! in the HIR, especially for multiple identifiers.
 
 use hir;
-use hir::map::{Definitions, DefKey, REGULAR_SPACE};
-use hir::map::definitions::DefPathData;
+use hir::map::{Definitions, DefKey};
 use hir::def_id::{DefIndex, DefId, CRATE_DEF_INDEX};
 use hir::def::{Def, PathResolution};
 use lint::builtin::PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES;
@@ -1738,29 +1737,28 @@ impl<'a> LoweringContext<'a> {
             node: match p.node {
                 PatKind::Wild => hir::PatKind::Wild,
                 PatKind::Ident(ref binding_mode, pth1, ref sub) => {
-                    self.with_parent_def(p.id, |this| {
-                        match this.resolver.get_resolution(p.id).map(|d| d.base_def()) {
-                            // `None` can occur in body-less function signatures
-                            def @ None | def @ Some(Def::Local(_)) => {
-                                let def_id = def.map(|d| d.def_id()).unwrap_or_else(|| {
-                                    this.resolver.definitions().local_def_id(p.id)
-                                });
-                                hir::PatKind::Binding(this.lower_binding_mode(binding_mode),
-                                                      def_id,
-                                                      respan(pth1.span, pth1.node.name),
-                                                      sub.as_ref().map(|x| this.lower_pat(x)))
-                            }
-                            Some(def) => {
-                                hir::PatKind::Path(hir::QPath::Resolved(None, P(hir::Path {
-                                    span: pth1.span,
-                                    def,
-                                    segments: hir_vec![
-                                        hir::PathSegment::from_name(pth1.node.name)
-                                    ],
-                                })))
-                            }
+                    match self.resolver.get_resolution(p.id).map(|d| d.base_def()) {
+                        // `None` can occur in body-less function signatures
+                        def @ None | def @ Some(Def::Local(_)) => {
+                            let canonical_id = match def {
+                                Some(Def::Local(id)) => id,
+                                _ => p.id
+                            };
+                            hir::PatKind::Binding(self.lower_binding_mode(binding_mode),
+                                                  canonical_id,
+                                                  respan(pth1.span, pth1.node.name),
+                                                  sub.as_ref().map(|x| self.lower_pat(x)))
                         }
-                    })
+                        Some(def) => {
+                            hir::PatKind::Path(hir::QPath::Resolved(None, P(hir::Path {
+                                span: pth1.span,
+                                def,
+                                segments: hir_vec![
+                                    hir::PathSegment::from_name(pth1.node.name)
+                                ],
+                            })))
+                        }
+                    }
                 }
                 PatKind::Lit(ref e) => hir::PatKind::Lit(P(self.lower_expr(e))),
                 PatKind::TupleStruct(ref path, ref pats, ddpos) => {
@@ -2715,14 +2713,9 @@ impl<'a> LoweringContext<'a> {
                                         id: Name,
                                         binding: NodeId,
                                         attrs: ThinVec<Attribute>) -> hir::Expr {
-        let def = {
-            let defs = self.resolver.definitions();
-            Def::Local(defs.local_def_id(binding))
-        };
-
         let expr_path = hir::ExprPath(hir::QPath::Resolved(None, P(hir::Path {
             span,
-            def,
+            def: Def::Local(binding),
             segments: hir_vec![hir::PathSegment::from_name(id)],
         })));
 
@@ -2860,23 +2853,12 @@ impl<'a> LoweringContext<'a> {
     fn pat_ident_binding_mode(&mut self, span: Span, name: Name, bm: hir::BindingAnnotation)
                               -> P<hir::Pat> {
         let LoweredNodeId { node_id, hir_id } = self.next_id();
-        let parent_def = self.parent_def.unwrap();
-        let def_id = {
-            let defs = self.resolver.definitions();
-            let def_path_data = DefPathData::Binding(name.as_str());
-            let def_index = defs.create_def_with_parent(parent_def,
-                                                        node_id,
-                                                        def_path_data,
-                                                        REGULAR_SPACE,
-                                                        Mark::root());
-            DefId::local(def_index)
-        };
 
         P(hir::Pat {
             id: node_id,
             hir_id,
             node: hir::PatKind::Binding(bm,
-                                        def_id,
+                                        node_id,
                                         Spanned {
                                             span,
                                             node: name,

src/librustc/hir/map/def_collector.rs

@@ -233,21 +233,10 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
     }
 
     fn visit_pat(&mut self, pat: &'a Pat) {
-        let parent_def = self.parent_def;
-
         match pat.node {
             PatKind::Mac(..) => return self.visit_macro_invoc(pat.id, false),
-            PatKind::Ident(_, id, _) => {
-                let def = self.create_def(pat.id,
-                                          DefPathData::Binding(id.node.name.as_str()),
-                                          REGULAR_SPACE);
-                self.parent_def = Some(def);
-            }
-            _ => {}
+            _ => visit::walk_pat(self, pat),
         }
-
-        visit::walk_pat(self, pat);
-        self.parent_def = parent_def;
     }
 
     fn visit_expr(&mut self, expr: &'a Expr) {

src/librustc/hir/map/definitions.rs

@@ -212,7 +212,6 @@ impl DefKey {
             DefPathData::TypeParam(name) |
             DefPathData::LifetimeDef(name) |
             DefPathData::EnumVariant(name) |
-            DefPathData::Binding(name) |
             DefPathData::Field(name) |
             DefPathData::GlobalMetaData(name) => {
                 name.hash(&mut hasher);
@@ -372,8 +371,6 @@ pub enum DefPathData {
     StructCtor,
     /// Initializer for a const
     Initializer,
-    /// Pattern binding
-    Binding(InternedString),
     /// An `impl Trait` type node.
     ImplTrait,
     /// A `typeof` type node.
@@ -613,7 +610,6 @@ impl DefPathData {
             TypeParam(name) |
             LifetimeDef(name) |
             EnumVariant(name) |
-            Binding(name) |
             Field(name) |
             GlobalMetaData(name) => Some(name),
 
@@ -638,7 +634,6 @@ impl DefPathData {
             TypeParam(name) |
             LifetimeDef(name) |
             EnumVariant(name) |
-            Binding(name) |
             Field(name) |
             GlobalMetaData(name) => {
                 return name

src/librustc/hir/mod.rs

@@ -623,8 +623,10 @@ pub enum PatKind {
     Wild,
 
     /// A fresh binding `ref mut binding @ OPT_SUBPATTERN`.
-    /// The `DefId` is for the definition of the variable being bound.
-    Binding(BindingAnnotation, DefId, Spanned<Name>, Option<P<Pat>>),
+    /// The `NodeId` is the canonical ID for the variable being bound,
+    /// e.g. in `Ok(x) | Err(x)`, both `x` use the same canonical ID,
+    /// which is the pattern ID of the first `x`.
+    Binding(BindingAnnotation, NodeId, Spanned<Name>, Option<P<Pat>>),
 
     /// A struct or struct variant pattern, e.g. `Variant {x, y, ..}`.
     /// The `bool` is `true` in the presence of a `..`.
@@ -1843,6 +1845,15 @@ pub struct Freevar {
     pub span: Span
 }
 
+impl Freevar {
+    pub fn var_id(&self) -> NodeId {
+        match self.def {
+            Def::Local(id) | Def::Upvar(id, ..) => id,
+            _ => bug!("Freevar::var_id: bad def ({:?})", self.def)
+        }
+    }
+}
+
 pub type FreevarMap = NodeMap<Vec<Freevar>>;
 
 pub type CaptureModeMap = NodeMap<CaptureClause>;

src/librustc/infer/error_reporting/mod.rs

@@ -913,8 +913,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                         name)
             }
             infer::UpvarRegion(ref upvar_id, _) => {
-                format!(" for capture of `{}` by closure",
-                        self.tcx.local_var_name_str_def_index(upvar_id.var_id))
+                let var_node_id = self.tcx.hir.hir_to_node_id(upvar_id.var_id);
+                let var_name = self.tcx.hir.name(var_node_id);
+                format!(" for capture of `{}` by closure", var_name)
             }
         };
 

src/librustc/infer/error_reporting/note.rs

@@ -43,10 +43,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                               "...so that reference does not outlive borrowed content");
             }
             infer::ReborrowUpvar(span, ref upvar_id) => {
+                let var_node_id = self.tcx.hir.hir_to_node_id(upvar_id.var_id);
+                let var_name = self.tcx.hir.name(var_node_id);
                 err.span_note(span,
-                              &format!("...so that closure can access `{}`",
-                                       self.tcx
-                                           .local_var_name_str_def_index(upvar_id.var_id)));
+                              &format!("...so that closure can access `{}`", var_name));
             }
             infer::InfStackClosure(span) => {
                 err.span_note(span, "...so that closure does not outlive its stack frame");
@@ -63,7 +63,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                 err.span_note(span,
                               &format!("...so that captured variable `{}` does not outlive the \
                                         enclosing closure",
-                                       self.tcx.local_var_name_str(id)));
+                                       self.tcx.hir.name(id)));
             }
             infer::IndexSlice(span) => {
                 err.span_note(span, "...so that slice is not indexed outside the lifetime");
@@ -176,22 +176,22 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                 err
             }
             infer::ReborrowUpvar(span, ref upvar_id) => {
+                let var_node_id = self.tcx.hir.hir_to_node_id(upvar_id.var_id);
+                let var_name = self.tcx.hir.name(var_node_id);
                 let mut err = struct_span_err!(self.tcx.sess,
                                                span,
                                                E0313,
                                                "lifetime of borrowed pointer outlives lifetime \
                                                 of captured variable `{}`...",
-                                               self.tcx
-                                                   .local_var_name_str_def_index(upvar_id.var_id));
+                                               var_name);
                 self.tcx.note_and_explain_region(region_scope_tree, &mut err,
                                                  "...the borrowed pointer is valid for ",
                                                  sub,
                                                  "...");
                 self.tcx.note_and_explain_region(
                     region_scope_tree,
                     &mut err,
-                    &format!("...but `{}` is only valid for ",
-                        self.tcx.local_var_name_str_def_index(upvar_id.var_id)),
+                    &format!("...but `{}` is only valid for ", var_name),
                     sup,
                     "");
                 err
@@ -234,7 +234,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                                                E0474,
                                                "captured variable `{}` does not outlive the \
                                                 enclosing closure",
-                                               self.tcx.local_var_name_str(id));
+                                               self.tcx.hir.name(id));
                 self.tcx.note_and_explain_region(region_scope_tree, &mut err,
                     "captured variable is valid for ", sup, "");
                 self.tcx.note_and_explain_region(region_scope_tree, &mut err,

src/librustc/middle/dead.rs

@@ -79,7 +79,8 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
                 self.check_def_id(def.def_id());
             }
             _ if self.ignore_non_const_paths => (),
-            Def::PrimTy(..) | Def::SelfTy(..) => (),
+            Def::PrimTy(..) | Def::SelfTy(..) |
+            Def::Local(..) | Def::Upvar(..) => {}
             Def::Variant(variant_id) | Def::VariantCtor(variant_id, ..) => {
                 if let Some(enum_id) = self.tcx.parent_def_id(variant_id) {
                     self.check_def_id(enum_id);

src/librustc/middle/expr_use_visitor.rs

@@ -828,7 +828,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
 
         let ExprUseVisitor { ref mc, ref mut delegate, param_env } = *self;
         return_if_err!(mc.cat_pattern(cmt_discr.clone(), pat, |cmt_pat, pat| {
-            if let PatKind::Binding(_, def_id, ..) = pat.node {
+            if let PatKind::Binding(_, canonical_id, ..) = pat.node {
                 debug!("binding cmt_pat={:?} pat={:?} match_mode={:?}", cmt_pat, pat, match_mode);
                 let bm = *mc.tables.pat_binding_modes().get(pat.hir_id)
                                                      .expect("missing binding mode");
@@ -838,7 +838,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
 
                 // Each match binding is effectively an assignment to the
                 // binding being produced.
-                let def = Def::Local(def_id);
+                let def = Def::Local(canonical_id);
                 if let Ok(binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty, def) {
                     delegate.mutate(pat.id, pat.span, binding_cmt, MutateMode::Init);
                 }
@@ -895,17 +895,16 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
 
         self.tcx().with_freevars(closure_expr.id, |freevars| {
             for freevar in freevars {
-                let var_def_id = freevar.def.def_id();
-                debug_assert!(var_def_id.is_local());
+                let var_hir_id = self.tcx().hir.node_to_hir_id(freevar.var_id());
                 let closure_def_id = self.tcx().hir.local_def_id(closure_expr.id);
                 let upvar_id = ty::UpvarId {
-                    var_id: var_def_id.index,
+                    var_id: var_hir_id,
                     closure_expr_id: closure_def_id.index
                 };
                 let upvar_capture = self.mc.tables.upvar_capture(upvar_id);
                 let cmt_var = return_if_err!(self.cat_captured_var(closure_expr.id,
                                                                    fn_decl_span,
-                                                                   freevar.def));
+                                                                   freevar));
                 match upvar_capture {
                     ty::UpvarCapture::ByValue => {
                         let mode = copy_or_move(&self.mc,
@@ -930,14 +929,13 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
     fn cat_captured_var(&mut self,
                         closure_id: ast::NodeId,
                         closure_span: Span,
-                        upvar_def: Def)
+                        upvar: &hir::Freevar)
                         -> mc::McResult<mc::cmt<'tcx>> {
         // Create the cmt for the variable being borrowed, from the
         // caller's perspective
-        let var_node_id = self.tcx().hir.as_local_node_id(upvar_def.def_id()).unwrap();
-        let var_hir_id = self.tcx().hir.node_to_hir_id(var_node_id);
+        let var_hir_id = self.tcx().hir.node_to_hir_id(upvar.var_id());
         let var_ty = self.mc.node_ty(var_hir_id)?;
-        self.mc.cat_def(closure_id, closure_span, var_ty, upvar_def)
+        self.mc.cat_def(closure_id, closure_span, var_ty, upvar.def)
     }
 }
 

src/librustc/middle/liveness.rs

@@ -429,8 +429,7 @@ fn visit_expr<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, expr: &'tcx Expr) {
         let mut call_caps = Vec::new();
         ir.tcx.with_freevars(expr.id, |freevars| {
             for fv in freevars {
-                if let Def::Local(def_id) = fv.def {
-                    let rv = ir.tcx.hir.as_local_node_id(def_id).unwrap();
+                if let Def::Local(rv) = fv.def {
                     let fv_ln = ir.add_live_node(FreeVarNode(fv.span));
                     call_caps.push(CaptureInfo {ln: fv_ln,
                                                 var_nid: rv});
@@ -1238,8 +1237,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
     fn access_path(&mut self, id: NodeId, path: &hir::Path, succ: LiveNode, acc: u32)
                    -> LiveNode {
         match path.def {
-          Def::Local(def_id) => {
-            let nid = self.ir.tcx.hir.as_local_node_id(def_id).unwrap();
+          Def::Local(nid) => {
             self.access_var(id, nid, succ, acc, path.span)
           }
           _ => succ
@@ -1414,12 +1412,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
     fn check_lvalue(&mut self, expr: &'tcx Expr) {
         match expr.node {
             hir::ExprPath(hir::QPath::Resolved(_, ref path)) => {
-                if let Def::Local(def_id) = path.def {
+                if let Def::Local(nid) = path.def {
                     // Assignment to an immutable variable or argument: only legal
                     // if there is no later assignment. If this local is actually
                     // mutable, then check for a reassignment to flag the mutability
                     // as being used.
-                    let nid = self.ir.tcx.hir.as_local_node_id(def_id).unwrap();
                     let ln = self.live_node(expr.id, expr.span);
                     let var = self.variable(nid, expr.span);
                     self.warn_about_dead_assign(expr.span, expr.id, ln, var);