Stop putting owner's DefId into their own node_id_to_def_id map

Now they unfortunately land in their parent, which is not necessary

Author: oli-obk

compiler/rustc_ast_lowering/src/delegation.rs

@@ -122,8 +122,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let span = self.lower_span(delegation.path.segments.last().unwrap().ident.span);
 
         // Delegation can be unresolved in illegal places such as function bodies in extern blocks (see #151356)
-        let sig_id = if let Some(delegation_info) =
-            self.resolver.delegation_info(self.local_def_id(item_id))
+        let sig_id = if let Some(delegation_info) = self.resolver.delegation_info(self.owner.def_id)
         {
             self.get_sig_id(delegation_info.resolution_node, span)
         } else {
@@ -143,8 +142,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                 let (param_count, c_variadic) = self.param_count(sig_id);
 
-                let mut generics =
-                    self.uplift_delegation_generics(delegation, sig_id, item_id, is_method);
+                let mut generics = self.uplift_delegation_generics(delegation, sig_id, is_method);
 
                 let body_id = self.lower_delegation_body(
                     delegation,

compiler/rustc_ast_lowering/src/delegation/generics.rs

@@ -231,11 +231,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
         &mut self,
         delegation: &Delegation,
         sig_id: DefId,
-        item_id: NodeId,
         is_method: bool,
     ) -> GenericsGenerationResults<'hir> {
-        let delegation_parent_kind =
-            self.tcx.def_kind(self.tcx.local_parent(self.local_def_id(item_id)));
+        let delegation_parent_kind = self.tcx.def_kind(self.tcx.local_parent(self.owner.def_id));
 
         let segments = &delegation.path.segments;
         let len = segments.len();

compiler/rustc_ast_lowering/src/item.rs

@@ -445,7 +445,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             ty,
                             ImplTraitContext::OpaqueTy {
                                 origin: hir::OpaqueTyOrigin::TyAlias {
-                                    parent: this.local_def_id(id),
+                                    parent: this.owner.def_id,
                                     in_assoc_ty: false,
                                 },
                             },
@@ -598,7 +598,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             ItemKind::MacroDef(ident, MacroDef { body, macro_rules, eii_declaration: _ }) => {
                 let ident = self.lower_ident(*ident);
                 let body = Box::new(self.lower_delim_args(body));
-                let def_id = self.local_def_id(id);
+                let def_id = self.owner.def_id;
                 let def_kind = self.tcx.def_kind(def_id);
                 let DefKind::Macro(macro_kinds) = def_kind else {
                     unreachable!(
@@ -1282,7 +1282,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                                     ty,
                                     ImplTraitContext::OpaqueTy {
                                         origin: hir::OpaqueTyOrigin::TyAlias {
-                                            parent: this.local_def_id(i.id),
+                                            parent: this.owner.def_id,
                                             in_assoc_ty: true,
                                         },
                                     },

compiler/rustc_ast_lowering/src/lib.rs

@@ -319,7 +319,7 @@ impl<'tcx> ResolverAstLowering<'tcx> {
     }
 
     fn owner_def_id(&self, id: NodeId) -> LocalDefId {
-        self.owners[&id].node_id_to_def_id[&id]
+        self.owners[&id].def_id
     }
 
     fn lifetime_elision_allowed(&self, id: NodeId) -> bool {
@@ -694,7 +694,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
     /// Given the id of an owner node in the AST, returns the corresponding `OwnerId`.
     fn owner_id(&self, node: NodeId) -> hir::OwnerId {
-        hir::OwnerId { def_id: self.resolver.owners[&node].node_id_to_def_id[&node] }
+        hir::OwnerId { def_id: self.resolver.owners[&node].def_id }
     }
 
     /// Freshen the `LoweringContext` and ready it to lower a nested item.
@@ -1804,27 +1804,27 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
         let output = match coro {
             Some(coro) => {
-                let fn_def_id = self.local_def_id(fn_node_id);
+                let fn_def_id = self.owner.def_id;
                 self.lower_coroutine_fn_ret_ty(&decl.output, fn_def_id, coro, kind)
             }
             None => match &decl.output {
                 FnRetTy::Ty(ty) => {
                     let itctx = match kind {
                         FnDeclKind::Fn | FnDeclKind::Inherent => ImplTraitContext::OpaqueTy {
                             origin: hir::OpaqueTyOrigin::FnReturn {
-                                parent: self.local_def_id(fn_node_id),
+                                parent: self.owner.def_id,
                                 in_trait_or_impl: None,
                             },
                         },
                         FnDeclKind::Trait => ImplTraitContext::OpaqueTy {
                             origin: hir::OpaqueTyOrigin::FnReturn {
-                                parent: self.local_def_id(fn_node_id),
+                                parent: self.owner.def_id,
                                 in_trait_or_impl: Some(hir::RpitContext::Trait),
                             },
                         },
                         FnDeclKind::Impl => ImplTraitContext::OpaqueTy {
                             origin: hir::OpaqueTyOrigin::FnReturn {
-                                parent: self.local_def_id(fn_node_id),
+                                parent: self.owner.def_id,
                                 in_trait_or_impl: Some(hir::RpitContext::TraitImpl),
                             },
                         },

compiler/rustc_middle/src/ty/mod.rs

@@ -203,11 +203,13 @@ pub struct PerOwnerResolverData {
     pub node_id_to_def_id: NodeMap<LocalDefId>,
     /// The id of the owner
     pub id: ast::NodeId,
+    /// The `DefId` of the owner, can't be found in `node_id_to_def_id`.
+    pub def_id: LocalDefId,
 }
 
 impl PerOwnerResolverData {
-    pub fn new(id: ast::NodeId) -> PerOwnerResolverData {
-        PerOwnerResolverData { node_id_to_def_id: Default::default(), id }
+    pub fn new(id: ast::NodeId, def_id: LocalDefId) -> PerOwnerResolverData {
+        PerOwnerResolverData { node_id_to_def_id: Default::default(), id, def_id }
     }
 }
 

compiler/rustc_passes/src/lang_items.rs

@@ -295,7 +295,7 @@ impl<'ast, 'tcx> visit::Visitor<'ast> for LanguageItemCollector<'ast, 'tcx> {
 
         self.check_for_lang(
             target,
-            self.resolver.owners[&i.id].node_id_to_def_id[&i.id],
+            self.resolver.owners[&i.id].def_id,
             &i.attrs,
             i.span,
             i.opt_generics(),
@@ -346,13 +346,7 @@ impl<'ast, 'tcx> visit::Visitor<'ast> for LanguageItemCollector<'ast, 'tcx> {
             }
         };
 
-        self.check_for_lang(
-            target,
-            self.resolver.owners[&i.id].node_id_to_def_id[&i.id],
-            &i.attrs,
-            i.span,
-            generics,
-        );
+        self.check_for_lang(target, self.resolver.owners[&i.id].def_id, &i.attrs, i.span, generics);
 
         visit::walk_assoc_item(self, i, ctxt);
     }

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -743,8 +743,7 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
             }
             ast::UseTreeKind::Nested { ref items, .. } => {
                 for &(ref tree, id) in items {
-                    self.with_owner(id, |this| {
-                        let feed = this.create_def(id, None, DefKind::Use, use_tree.span());
+                    self.with_owner(id, None, DefKind::Use, use_tree.span(), |this, feed| {
                         this.build_reduced_graph_for_use_tree(
                             // This particular use tree
                             tree, id, &prefix, true, false, // The whole `use` item

compiler/rustc_resolve/src/def_collector.rs

@@ -32,7 +32,6 @@ pub(crate) fn collect_definitions<'ra>(
     with_owner(resolver, invocation_parent.owner, |r| {
         let mut visitor = DefCollector { r, invocation_parent, parent_scope };
         fragment.visit_with(&mut visitor);
-
         visitor.parent_scope.macro_rules
     })
 }
@@ -64,6 +63,7 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
             def_kind,
             self.parent_scope.expansion.to_expn_id(),
             span.with_parent(None),
+            false,
         )
     }
 
@@ -73,23 +73,33 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
         self.invocation_parent.parent_def = orig_parent_def;
     }
 
-    pub(super) fn with_owner<F: FnOnce(&mut Self)>(&mut self, owner: NodeId, f: F) {
+    pub(super) fn with_owner<F: FnOnce(&mut Self, TyCtxtFeed<'tcx, LocalDefId>)>(
+        &mut self,
+        owner: NodeId,
+        name: Option<Symbol>,
+        def_kind: DefKind,
+        span: Span,
+        f: F,
+    ) {
         debug_assert_ne!(owner, DUMMY_NODE_ID);
-        if owner == CRATE_NODE_ID {
-            // Special case: we always have an invocation parent (set in `collect_definitions`)
-            // of at least the crate root, even for visiting the crate root,
-            // which would then remove the crate root from the tables
-            // list twice and try to insert it twice afterwards.
-            debug_assert_eq!(self.r.current_owner.id, CRATE_NODE_ID);
-            return f(self);
-        }
+        debug_assert_ne!(owner, CRATE_NODE_ID);
         // We only get here if the owner didn't exist yet. After the owner has been created,
         // future invocations of `collect_definitions` will get the owner out of the `owners`
         // table.
-        let tables = PerOwnerResolverData::new(owner);
+        let parent_def = self.invocation_parent.parent_def;
+        let feed = self.r.create_def(
+            parent_def,
+            owner,
+            name,
+            def_kind,
+            self.parent_scope.expansion.to_expn_id(),
+            span.with_parent(None),
+            true,
+        );
+        let tables = PerOwnerResolverData::new(owner, feed.key());
 
         let orig_invoc_owner = mem::replace(&mut self.invocation_parent.owner, owner);
-        with_owner_tables(self, owner, tables, f);
+        with_owner_tables(self, owner, tables, |this| f(this, feed));
         let old = mem::replace(&mut self.invocation_parent.owner, orig_invoc_owner);
         assert_eq!(old, owner);
     }
@@ -200,8 +210,7 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
             }
             ItemKind::GlobalAsm(..) => DefKind::GlobalAsm,
             ItemKind::Use(_) => {
-                return self.with_owner(i.id, |this| {
-                    let feed = this.create_def(i.id, None, DefKind::Use, i.span);
+                return self.with_owner(i.id, None, DefKind::Use, i.span, |this, feed| {
                     this.brg_visit_item(i, feed);
                 });
             }
@@ -212,20 +221,23 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
             }
             ItemKind::DelegationMac(..) => unreachable!(),
         };
-        self.with_owner(i.id, |this| {
-            let feed =
-                this.create_def(i.id, i.kind.ident().map(|ident| ident.name), def_kind, i.span);
-
-            if let Some(ext) = opt_syn_ext {
-                this.r.local_macro_map.insert(feed.def_id(), self.r.arenas.alloc_macro(ext));
-            }
+        self.with_owner(
+            i.id,
+            i.kind.ident().map(|ident| ident.name),
+            def_kind,
+            i.span,
+            |this, feed| {
+                if let Some(ext) = opt_syn_ext {
+                    this.r.local_macro_map.insert(feed.def_id(), self.r.arenas.alloc_macro(ext));
+                }
 
-            this.with_parent(feed.def_id(), |this| {
-                this.with_impl_trait(ImplTraitContext::Existential, |this| {
-                    this.brg_visit_item(i, feed)
-                })
-            });
-        });
+                this.with_parent(feed.def_id(), |this| {
+                    this.with_impl_trait(ImplTraitContext::Existential, |this| {
+                        this.brg_visit_item(i, feed)
+                    })
+                });
+            },
+        );
     }
 
     fn visit_block(&mut self, block: &'a Block) {
@@ -316,9 +328,7 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
             }
         };
 
-        self.with_owner(fi.id, |this| {
-            let def = this.create_def(fi.id, Some(ident.name), def_kind, fi.span);
-
+        self.with_owner(fi.id, Some(ident.name), def_kind, fi.span, |this, def| {
             this.with_parent(def.def_id(), |this| {
                 this.build_reduced_graph_for_foreign_item(fi, ident, def);
                 visit::walk_item(this, fi)
@@ -402,8 +412,7 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
             }
         };
 
-        self.with_owner(i.id, |this| {
-            let feed = this.create_def(i.id, Some(ident.name), def_kind, i.span);
+        self.with_owner(i.id, Some(ident.name), def_kind, i.span, |this, feed| {
             this.with_parent(feed.def_id(), |this| {
                 this.brg_visit_assoc_item(i, ctxt, ident, ns, feed)
             });
@@ -604,11 +613,16 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
         } else {
             // Visit attributes after items for backward compatibility.
             // This way they can use `macro_rules` defined later.
-            self.with_owner(CRATE_NODE_ID, |this| {
-                visit::walk_list!(this, visit_item, &krate.items);
-                visit::walk_list!(this, visit_attribute, &krate.attrs);
-                this.contains_macro_use(&krate.attrs);
-            })
+
+            // We always have an invocation parent (set in `collect_definitions`)
+            // of at least the crate root, even for visiting the crate root,
+            // which would then remove the crate root from the tables
+            // list twice and try to insert it twice afterwards.
+            debug_assert_eq!(self.r.current_owner.id, CRATE_NODE_ID);
+
+            visit::walk_list!(self, visit_item, &krate.items);
+            visit::walk_list!(self, visit_attribute, &krate.attrs);
+            self.contains_macro_use(&krate.attrs);
         }
     }