Auto merge of rust-lang#87234 - cjgillot:lower-mono, r=petrochenkov

Lower only one HIR owner at a time

Based on rust-lang#83723
Additional diff is here: cjgillot/rust@ownernode...lower-mono

Lowering is very tangled and has a tendency to intertwine the transformation of different items. This PR aims at simplifying the logic by:
- moving global analyses to the resolver (item_generics_num_lifetimes, proc_macros, trait_impls);
- removing a few special cases (non-exported macros and use statements);
- restricting the amount of available information at any one time;
- avoiding back-and-forth between different owners: an item must now be lowered all at once, and its parent cannot refer to its nodes.

I also removed the sorting of bodies by span.  The diagnostic ordering changes marginally, since definitions are pretty much sorted already according to the AST. This uncovered a subtlety in thir-unsafeck.

(While these items could logically be in different PRs, the dependency between commits and the amount of conflicts force a monolithic PR.)

compiler/rustc_ast_lowering/src/expr.rs

@@ -55,7 +55,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
                         0,
                         ParenthesizedGenericArgs::Err,
                         ImplTraitContext::disallowed(),
-                        None,
                     ));
                     let args = self.lower_exprs(args);
                     hir::ExprKind::MethodCall(
@@ -328,7 +327,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let mut generic_args = vec![];
         for (idx, arg) in args.into_iter().enumerate() {
             if legacy_args_idx.contains(&idx) {
-                let parent_def_id = self.current_hir_id_owner.0;
+                let parent_def_id = self.current_hir_id_owner;
                 let node_id = self.resolver.next_node_id();
 
                 // Add a definition for the in-band const def.

compiler/rustc_ast_lowering/src/item.rs

@@ -40,12 +40,9 @@ impl ItemLowerer<'_, '_, '_> {
 
 impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
     fn visit_item(&mut self, item: &'a Item) {
-        self.lctx.allocate_hir_id_counter(item.id);
         let hir_id = self.lctx.with_hir_id_owner(item.id, |lctx| {
-            lctx.without_in_scope_lifetime_defs(|lctx| {
-                let hir_item = lctx.lower_item(item);
-                lctx.insert_item(hir_item)
-            })
+            let node = lctx.without_in_scope_lifetime_defs(|lctx| lctx.lower_item(item));
+            hir::OwnerNode::Item(node)
         });
 
         self.lctx.with_parent_item_lifetime_defs(hir_id, |this| {
@@ -72,26 +69,17 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
     }
 
     fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) {
-        self.lctx.allocate_hir_id_counter(item.id);
         self.lctx.with_hir_id_owner(item.id, |lctx| match ctxt {
-            AssocCtxt::Trait => {
-                let hir_item = lctx.lower_trait_item(item);
-                lctx.insert_trait_item(hir_item);
-            }
-            AssocCtxt::Impl => {
-                let hir_item = lctx.lower_impl_item(item);
-                lctx.insert_impl_item(hir_item);
-            }
+            AssocCtxt::Trait => hir::OwnerNode::TraitItem(lctx.lower_trait_item(item)),
+            AssocCtxt::Impl => hir::OwnerNode::ImplItem(lctx.lower_impl_item(item)),
         });
 
         visit::walk_assoc_item(self, item, ctxt);
     }
 
     fn visit_foreign_item(&mut self, item: &'a ForeignItem) {
-        self.lctx.allocate_hir_id_counter(item.id);
         self.lctx.with_hir_id_owner(item.id, |lctx| {
-            let hir_item = lctx.lower_foreign_item(item);
-            lctx.insert_foreign_item(hir_item);
+            hir::OwnerNode::ForeignItem(lctx.lower_foreign_item(item))
         });
 
         visit::walk_foreign_item(self, item);
@@ -106,12 +94,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
     // only used when lowering a child item of a trait or impl.
     fn with_parent_item_lifetime_defs<T>(
         &mut self,
-        parent_hir_id: hir::ItemId,
+        parent_hir_id: LocalDefId,
         f: impl FnOnce(&mut Self) -> T,
     ) -> T {
         let old_len = self.in_scope_lifetimes.len();
 
-        let parent_generics = match self.owners[parent_hir_id.def_id].unwrap().expect_item().kind {
+        let parent_generics = match self.owners[parent_hir_id].unwrap().expect_item().kind {
             hir::ItemKind::Impl(hir::Impl { ref generics, .. })
             | hir::ItemKind::Trait(_, _, ref generics, ..) => generics.params,
             _ => &[],
@@ -186,19 +174,20 @@ impl<'hir> LoweringContext<'_, 'hir> {
         }
     }
 
-    pub fn lower_item(&mut self, i: &Item) -> hir::Item<'hir> {
+    fn lower_item(&mut self, i: &Item) -> &'hir hir::Item<'hir> {
         let mut ident = i.ident;
-        let mut vis = self.lower_visibility(&i.vis, None);
+        let mut vis = self.lower_visibility(&i.vis);
         let hir_id = self.lower_node_id(i.id);
         let attrs = self.lower_attrs(hir_id, &i.attrs);
         let kind = self.lower_item_kind(i.span, i.id, hir_id, &mut ident, attrs, &mut vis, &i.kind);
-        hir::Item {
+        let item = hir::Item {
             def_id: hir_id.expect_owner(),
             ident: self.lower_ident(ident),
             kind,
             vis,
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
     fn lower_item_kind(
@@ -480,10 +469,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 // Essentially a single `use` which imports two names is desugared into
                 // two imports.
                 for new_node_id in [id1, id2] {
-                    // Associate an HirId to both ids even if there is no resolution.
-                    let new_id = self.allocate_hir_id_counter(new_node_id);
-
-                    let res = if let Some(res) = resolutions.next() { res } else { continue };
+                    let new_id = self.resolver.local_def_id(new_node_id);
+                    let res = if let Some(res) = resolutions.next() {
+                        res
+                    } else {
+                        // Associate an HirId to both ids even if there is no resolution.
+                        self.node_id_to_hir_id.ensure_contains_elem(new_node_id, || None);
+                        debug_assert!(self.node_id_to_hir_id[new_node_id].is_none());
+                        self.node_id_to_hir_id[new_node_id] = Some(hir::HirId::make_owner(new_id));
+                        continue;
+                    };
                     let ident = *ident;
                     let mut path = path.clone();
                     for seg in &mut path.segments {
@@ -493,24 +488,25 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                     self.with_hir_id_owner(new_node_id, |this| {
                         let res = this.lower_res(res);
-                        let path = this.lower_path_extra(res, &path, ParamMode::Explicit, None);
+                        let path = this.lower_path_extra(res, &path, ParamMode::Explicit);
                         let kind = hir::ItemKind::Use(path, hir::UseKind::Single);
                         let vis = this.rebuild_vis(&vis);
                         if let Some(attrs) = attrs {
                             this.attrs.insert(hir::HirId::make_owner(new_id), attrs);
                         }
 
-                        this.insert_item(hir::Item {
+                        let item = hir::Item {
                             def_id: new_id,
                             ident: this.lower_ident(ident),
                             kind,
                             vis,
                             span: this.lower_span(span),
-                        });
+                        };
+                        hir::OwnerNode::Item(this.arena.alloc(item))
                     });
                 }
 
-                let path = self.lower_path_extra(ret_res, &path, ParamMode::Explicit, None);
+                let path = self.lower_path_extra(ret_res, &path, ParamMode::Explicit);
                 hir::ItemKind::Use(path, hir::UseKind::Single)
             }
             UseTreeKind::Glob => {
@@ -550,7 +546,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                 // Add all the nested `PathListItem`s to the HIR.
                 for &(ref use_tree, id) in trees {
-                    let new_hir_id = self.allocate_hir_id_counter(id);
+                    let new_hir_id = self.resolver.local_def_id(id);
 
                     let mut prefix = prefix.clone();
 
@@ -574,13 +570,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             this.attrs.insert(hir::HirId::make_owner(new_hir_id), attrs);
                         }
 
-                        this.insert_item(hir::Item {
+                        let item = hir::Item {
                             def_id: new_hir_id,
                             ident: this.lower_ident(ident),
                             kind,
                             vis,
                             span: this.lower_span(use_tree.span),
-                        });
+                        };
+                        hir::OwnerNode::Item(this.arena.alloc(item))
                     });
                 }
 
@@ -610,7 +607,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                 let res = self.expect_full_res_from_use(id).next().unwrap_or(Res::Err);
                 let res = self.lower_res(res);
-                let path = self.lower_path_extra(res, &prefix, ParamMode::Explicit, None);
+                let path = self.lower_path_extra(res, &prefix, ParamMode::Explicit);
                 hir::ItemKind::Use(path, hir::UseKind::ListStem)
             }
         }
@@ -647,11 +644,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
         respan(self.lower_span(vis.span), vis_kind)
     }
 
-    fn lower_foreign_item(&mut self, i: &ForeignItem) -> hir::ForeignItem<'hir> {
+    fn lower_foreign_item(&mut self, i: &ForeignItem) -> &'hir hir::ForeignItem<'hir> {
         let hir_id = self.lower_node_id(i.id);
         let def_id = hir_id.expect_owner();
         self.lower_attrs(hir_id, &i.attrs);
-        hir::ForeignItem {
+        let item = hir::ForeignItem {
             def_id,
             ident: self.lower_ident(i.ident),
             kind: match i.kind {
@@ -679,17 +676,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 ForeignItemKind::TyAlias(..) => hir::ForeignItemKind::Type,
                 ForeignItemKind::MacCall(_) => panic!("macro shouldn't exist here"),
             },
-            vis: self.lower_visibility(&i.vis, None),
+            vis: self.lower_visibility(&i.vis),
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
-    fn lower_foreign_item_ref(&mut self, i: &ForeignItem) -> hir::ForeignItemRef<'hir> {
+    fn lower_foreign_item_ref(&mut self, i: &ForeignItem) -> hir::ForeignItemRef {
         hir::ForeignItemRef {
-            id: hir::ForeignItemId { def_id: self.allocate_hir_id_counter(i.id) },
+            id: hir::ForeignItemId { def_id: self.resolver.local_def_id(i.id) },
             ident: self.lower_ident(i.ident),
             span: self.lower_span(i.span),
-            vis: self.lower_visibility(&i.vis, Some(i.id)),
         }
     }
 
@@ -757,12 +754,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 // FIXME(jseyfried): positional field hygiene.
                 None => Ident::new(sym::integer(index), self.lower_span(f.span)),
             },
-            vis: self.lower_visibility(&f.vis, None),
+            vis: self.lower_visibility(&f.vis),
             ty,
         }
     }
 
-    fn lower_trait_item(&mut self, i: &AssocItem) -> hir::TraitItem<'hir> {
+    fn lower_trait_item(&mut self, i: &AssocItem) -> &'hir hir::TraitItem<'hir> {
         let hir_id = self.lower_node_id(i.id);
         let trait_item_def_id = hir_id.expect_owner();
 
@@ -805,13 +802,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
         };
 
         self.lower_attrs(hir_id, &i.attrs);
-        hir::TraitItem {
+        let item = hir::TraitItem {
             def_id: trait_item_def_id,
             ident: self.lower_ident(i.ident),
             generics,
             kind,
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
     fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef {
@@ -841,7 +839,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         self.expr(span, hir::ExprKind::Err, AttrVec::new())
     }
 
-    fn lower_impl_item(&mut self, i: &AssocItem) -> hir::ImplItem<'hir> {
+    fn lower_impl_item(&mut self, i: &AssocItem) -> &'hir hir::ImplItem<'hir> {
         let impl_item_def_id = self.resolver.local_def_id(i.id);
 
         let (generics, kind) = match &i.kind {
@@ -895,26 +893,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
         let hir_id = self.lower_node_id(i.id);
         self.lower_attrs(hir_id, &i.attrs);
-        hir::ImplItem {
+        let item = hir::ImplItem {
             def_id: hir_id.expect_owner(),
             ident: self.lower_ident(i.ident),
             generics,
-            vis: self.lower_visibility(&i.vis, None),
+            vis: self.lower_visibility(&i.vis),
             defaultness,
             kind,
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
-    fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef<'hir> {
+    fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef {
         // Since `default impl` is not yet implemented, this is always true in impls.
         let has_value = true;
         let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
         hir::ImplItemRef {
-            id: hir::ImplItemId { def_id: self.allocate_hir_id_counter(i.id) },
+            id: hir::ImplItemId { def_id: self.resolver.local_def_id(i.id) },
             ident: self.lower_ident(i.ident),
             span: self.lower_span(i.span),
-            vis: self.lower_visibility(&i.vis, Some(i.id)),
             defaultness,
             kind: match &i.kind {
                 AssocItemKind::Const(..) => hir::AssocItemKind::Const,
@@ -932,25 +930,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
     /// lowered. This can happen during `lower_impl_item_ref()` where we need to
     /// lower a `Visibility` value although we haven't lowered the owning
     /// `ImplItem` in question yet.
-    fn lower_visibility(
-        &mut self,
-        v: &Visibility,
-        explicit_owner: Option<NodeId>,
-    ) -> hir::Visibility<'hir> {
+    fn lower_visibility(&mut self, v: &Visibility) -> hir::Visibility<'hir> {
         let node = match v.kind {
             VisibilityKind::Public => hir::VisibilityKind::Public,
             VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
             VisibilityKind::Restricted { ref path, id } => {
                 debug!("lower_visibility: restricted path id = {:?}", id);
-                let lowered_id = if let Some(owner) = explicit_owner {
-                    self.lower_node_id_with_owner(id, owner)
-                } else {
-                    self.lower_node_id(id)
-                };
-                let res = self.expect_full_res(id);
-                let res = self.lower_res(res);
+                let lowered_id = self.lower_node_id(id);
                 hir::VisibilityKind::Restricted {
-                    path: self.lower_path_extra(res, path, ParamMode::Explicit, explicit_owner),
+                    path: self.lower_path(id, path, ParamMode::Explicit),
                     hir_id: lowered_id,
                 }
             }