Rollup merge of #138384 - nnethercote:hir-ItemKind-idents, r=fmease

Move `hir::Item::ident` into `hir::ItemKind`.

 `hir::Item` has an `ident` field.

- It's always non-empty for these item kinds: `ExternCrate`, `Static`, `Const`, `Fn`, `Macro`, `Mod`, `TyAlias`, `Enum`, `Struct`, `Union`, Trait`, TraitAalis`.

- It's always empty for these item kinds: `ForeignMod`, `GlobalAsm`, `Impl`.

- For `Use`, it is non-empty for `UseKind::Single` and empty for `UseKind::{Glob,ListStem}`.

All of this is quite non-obvious; the only documentation is a single comment saying "The name might be a dummy name in case of anonymous items". Some sites that handle items check for an empty ident, some don't. This is a very C-like way of doing things, but this is Rust, we have sum types, we can do this properly and never forget to check for the exceptional case and never YOLO possibly empty identifiers (or possibly dummy spans) around and hope that things will work out.

This is step towards `kw::Empty` elimination (#137978).

r? `@fmease`

6 files changed, 88 insertions, 73 deletions

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 929402d4170..4ecf702d7b6 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -125,7 +125,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
     items.extend(doc.items.values().flat_map(|(item, renamed, _)| {
         // Now we actually lower the imports, skipping everything else.
         if let hir::ItemKind::Use(path, hir::UseKind::Glob) = item.kind {
-            let name = renamed.unwrap_or_else(|| cx.tcx.hir_name(item.hir_id()));
+            let name = renamed.unwrap_or(kw::Empty); // using kw::Empty is a bit of a hack
             clean_use_statement(item, name, path, hir::UseKind::Glob, cx, &mut inserted)
         } else {
             // skip everything else
@@ -1629,8 +1629,7 @@ fn first_non_private<'tcx>(
                 if let Some(use_def_id) = reexp.id()
                     && let Some(local_use_def_id) = use_def_id.as_local()
                     && let hir::Node::Item(item) = cx.tcx.hir_node_by_def_id(local_use_def_id)
-                    && !item.ident.name.is_empty()
-                    && let hir::ItemKind::Use(path, _) = item.kind
+                    && let hir::ItemKind::Use(path, hir::UseKind::Single(_)) = item.kind
                 {
                     for res in &path.res {
                         if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = res {
@@ -1775,7 +1774,7 @@ fn maybe_expand_private_type_alias<'tcx>(
     } else {
         return None;
     };
-    let hir::ItemKind::TyAlias(ty, generics) = alias else { return None };
+    let hir::ItemKind::TyAlias(_, ty, generics) = alias else { return None };
 
     let final_seg = &path.segments.last().expect("segments were empty");
     let mut args = DefIdMap::default();
@@ -2794,20 +2793,24 @@ fn clean_maybe_renamed_item<'tcx>(
     use hir::ItemKind;
 
     let def_id = item.owner_id.to_def_id();
-    let mut name = renamed.unwrap_or_else(|| cx.tcx.hir_name(item.hir_id()));
+    let mut name = renamed.unwrap_or_else(|| {
+        // FIXME: using kw::Empty is a bit of a hack
+        cx.tcx.hir_opt_name(item.hir_id()).unwrap_or(kw::Empty)
+    });
+
     cx.with_param_env(def_id, |cx| {
         let kind = match item.kind {
-            ItemKind::Static(ty, mutability, body_id) => StaticItem(Static {
+            ItemKind::Static(_, ty, mutability, body_id) => StaticItem(Static {
                 type_: Box::new(clean_ty(ty, cx)),
                 mutability,
                 expr: Some(body_id),
             }),
-            ItemKind::Const(ty, generics, body_id) => ConstantItem(Box::new(Constant {
+            ItemKind::Const(_, ty, generics, body_id) => ConstantItem(Box::new(Constant {
                 generics: clean_generics(generics, cx),
                 type_: clean_ty(ty, cx),
                 kind: ConstantKind::Local { body: body_id, def_id },
             })),
-            ItemKind::TyAlias(hir_ty, generics) => {
+            ItemKind::TyAlias(_, hir_ty, generics) => {
                 *cx.current_type_aliases.entry(def_id).or_insert(0) += 1;
                 let rustdoc_ty = clean_ty(hir_ty, cx);
                 let type_ =
@@ -2840,34 +2843,34 @@ fn clean_maybe_renamed_item<'tcx>(
                 ));
                 return ret;
             }
-            ItemKind::Enum(ref def, generics) => EnumItem(Enum {
+            ItemKind::Enum(_, ref def, generics) => EnumItem(Enum {
                 variants: def.variants.iter().map(|v| clean_variant(v, cx)).collect(),
                 generics: clean_generics(generics, cx),
             }),
-            ItemKind::TraitAlias(generics, bounds) => TraitAliasItem(TraitAlias {
+            ItemKind::TraitAlias(_, generics, bounds) => TraitAliasItem(TraitAlias {
                 generics: clean_generics(generics, cx),
                 bounds: bounds.iter().filter_map(|x| clean_generic_bound(x, cx)).collect(),
             }),
-            ItemKind::Union(ref variant_data, generics) => UnionItem(Union {
+            ItemKind::Union(_, ref variant_data, generics) => UnionItem(Union {
                 generics: clean_generics(generics, cx),
                 fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
             }),
-            ItemKind::Struct(ref variant_data, generics) => StructItem(Struct {
+            ItemKind::Struct(_, ref variant_data, generics) => StructItem(Struct {
                 ctor_kind: variant_data.ctor_kind(),
                 generics: clean_generics(generics, cx),
                 fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
             }),
             ItemKind::Impl(impl_) => return clean_impl(impl_, item.owner_id.def_id, cx),
-            ItemKind::Macro(macro_def, MacroKind::Bang) => MacroItem(Macro {
+            ItemKind::Macro(_, macro_def, MacroKind::Bang) => MacroItem(Macro {
                 source: display_macro_source(cx, name, macro_def),
                 macro_rules: macro_def.macro_rules,
             }),
-            ItemKind::Macro(_, macro_kind) => clean_proc_macro(item, &mut name, macro_kind, cx),
+            ItemKind::Macro(_, _, macro_kind) => clean_proc_macro(item, &mut name, macro_kind, cx),
             // proc macros can have a name set by attributes
             ItemKind::Fn { ref sig, generics, body: body_id, .. } => {
                 clean_fn_or_proc_macro(item, sig, generics, body_id, &mut name, cx)
             }
-            ItemKind::Trait(_, _, generics, bounds, item_ids) => {
+            ItemKind::Trait(_, _, _, generics, bounds, item_ids) => {
                 let items = item_ids
                     .iter()
                     .map(|ti| clean_trait_item(cx.tcx.hir_trait_item(ti.id), cx))
@@ -2880,7 +2883,7 @@ fn clean_maybe_renamed_item<'tcx>(
                     bounds: bounds.iter().filter_map(|x| clean_generic_bound(x, cx)).collect(),
                 }))
             }
-            ItemKind::ExternCrate(orig_name) => {
+            ItemKind::ExternCrate(orig_name, _) => {
                 return clean_extern_crate(item, name, orig_name, cx);
             }
             ItemKind::Use(path, kind) => {
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index 3f9023659db..ebfd1338b2e 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -226,7 +226,7 @@ impl ExternalCrate {
                 .filter_map(|&id| {
                     let item = tcx.hir_item(id);
                     match item.kind {
-                        hir::ItemKind::Mod(_) => {
+                        hir::ItemKind::Mod(..) => {
                             as_keyword(Res::Def(DefKind::Mod, id.owner_id.to_def_id()))
                         }
                         _ => None,
@@ -282,7 +282,7 @@ impl ExternalCrate {
                 .filter_map(|&id| {
                     let item = tcx.hir_item(id);
                     match item.kind {
-                        hir::ItemKind::Mod(_) => {
+                        hir::ItemKind::Mod(..) => {
                             as_primitive(Res::Def(DefKind::Mod, id.owner_id.to_def_id()))
                         }
                         _ => None,
diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs
index 18ad442d017..d0072256941 100644
--- a/src/librustdoc/doctest/rust.rs
+++ b/src/librustdoc/doctest/rust.rs
@@ -80,7 +80,7 @@ impl<'tcx> HirCollector<'tcx> {
 
     pub fn collect_crate(mut self) -> Vec<ScrapedDocTest> {
         let tcx = self.tcx;
-        self.visit_testable("".to_string(), CRATE_DEF_ID, tcx.hir().span(CRATE_HIR_ID), |this| {
+        self.visit_testable(None, CRATE_DEF_ID, tcx.hir().span(CRATE_HIR_ID), |this| {
             tcx.hir_walk_toplevel_module(this)
         });
         self.collector.tests
@@ -90,7 +90,7 @@ impl<'tcx> HirCollector<'tcx> {
 impl HirCollector<'_> {
     fn visit_testable<F: FnOnce(&mut Self)>(
         &mut self,
-        name: String,
+        name: Option<String>,
         def_id: LocalDefId,
         sp: Span,
         nested: F,
@@ -103,9 +103,10 @@ impl HirCollector<'_> {
             return;
         }
 
-        let has_name = !name.is_empty();
-        if has_name {
+        let mut has_name = false;
+        if let Some(name) = name {
             self.collector.cur_path.push(name);
+            has_name = true;
         }
 
         // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with
@@ -153,9 +154,9 @@ impl<'tcx> intravisit::Visitor<'tcx> for HirCollector<'tcx> {
     fn visit_item(&mut self, item: &'tcx hir::Item<'_>) {
         let name = match &item.kind {
             hir::ItemKind::Impl(impl_) => {
-                rustc_hir_pretty::id_to_string(&self.tcx, impl_.self_ty.hir_id)
+                Some(rustc_hir_pretty::id_to_string(&self.tcx, impl_.self_ty.hir_id))
             }
-            _ => item.ident.to_string(),
+            _ => item.kind.ident().map(|ident| ident.to_string()),
         };
 
         self.visit_testable(name, item.owner_id.def_id, item.span, |this| {
@@ -164,31 +165,46 @@ impl<'tcx> intravisit::Visitor<'tcx> for HirCollector<'tcx> {
     }
 
     fn visit_trait_item(&mut self, item: &'tcx hir::TraitItem<'_>) {
-        self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| {
-            intravisit::walk_trait_item(this, item);
-        });
+        self.visit_testable(
+            Some(item.ident.to_string()),
+            item.owner_id.def_id,
+            item.span,
+            |this| {
+                intravisit::walk_trait_item(this, item);
+            },
+        );
     }
 
     fn visit_impl_item(&mut self, item: &'tcx hir::ImplItem<'_>) {
-        self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| {
-            intravisit::walk_impl_item(this, item);
-        });
+        self.visit_testable(
+            Some(item.ident.to_string()),
+            item.owner_id.def_id,
+            item.span,
+            |this| {
+                intravisit::walk_impl_item(this, item);
+            },
+        );
     }
 
     fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem<'_>) {
-        self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| {
-            intravisit::walk_foreign_item(this, item);
-        });
+        self.visit_testable(
+            Some(item.ident.to_string()),
+            item.owner_id.def_id,
+            item.span,
+            |this| {
+                intravisit::walk_foreign_item(this, item);
+            },
+        );
     }
 
     fn visit_variant(&mut self, v: &'tcx hir::Variant<'_>) {
-        self.visit_testable(v.ident.to_string(), v.def_id, v.span, |this| {
+        self.visit_testable(Some(v.ident.to_string()), v.def_id, v.span, |this| {
             intravisit::walk_variant(this, v);
         });
     }
 
     fn visit_field_def(&mut self, f: &'tcx hir::FieldDef<'_>) {
-        self.visit_testable(f.ident.to_string(), f.def_id, f.span, |this| {
+        self.visit_testable(Some(f.ident.to_string()), f.def_id, f.span, |this| {
             intravisit::walk_field_def(this, f);
         });
     }
diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs
index 3228f71df07..3676051b1bd 100644
--- a/src/librustdoc/html/render/span_map.rs
+++ b/src/librustdoc/html/render/span_map.rs
@@ -242,10 +242,9 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> {
             // Now that we confirmed it's a file import, we want to get the span for the module
             // name only and not all the "mod foo;".
             if let Node::Item(item) = self.tcx.hir_node(id) {
-                self.matches.insert(
-                    item.ident.span,
-                    LinkFromSrc::Local(clean::Span::new(m.spans.inner_span)),
-                );
+                let (ident, _) = item.expect_mod();
+                self.matches
+                    .insert(ident.span, LinkFromSrc::Local(clean::Span::new(m.spans.inner_span)));
             }
         } else {
             // If it's a "mod foo {}", we want to look to its documentation page.
@@ -273,22 +272,22 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> {
     fn visit_item(&mut self, item: &'tcx Item<'tcx>) {
         match item.kind {
             ItemKind::Static(..)
-            | ItemKind::Const(_, _, _)
+            | ItemKind::Const(..)
             | ItemKind::Fn { .. }
-            | ItemKind::Macro(_, _)
-            | ItemKind::TyAlias(_, _)
-            | ItemKind::Enum(_, _)
-            | ItemKind::Struct(_, _)
-            | ItemKind::Union(_, _)
-            | ItemKind::Trait(_, _, _, _, _)
-            | ItemKind::TraitAlias(_, _) => self.extract_info_from_hir_id(item.hir_id()),
+            | ItemKind::Macro(..)
+            | ItemKind::TyAlias(..)
+            | ItemKind::Enum(..)
+            | ItemKind::Struct(..)
+            | ItemKind::Union(..)
+            | ItemKind::Trait(..)
+            | ItemKind::TraitAlias(..) => self.extract_info_from_hir_id(item.hir_id()),
             ItemKind::Impl(_)
-            | ItemKind::Use(_, _)
-            | ItemKind::ExternCrate(_)
+            | ItemKind::Use(..)
+            | ItemKind::ExternCrate(..)
             | ItemKind::ForeignMod { .. }
             | ItemKind::GlobalAsm { .. }
             // We already have "visit_mod" above so no need to check it here.
-            | ItemKind::Mod(_) => {}
+            | ItemKind::Mod(..) => {}
         }
         intravisit::walk_item(self, item);
     }
diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs
index 45b8dafa907..f8f670f575b 100644
--- a/src/librustdoc/passes/calculate_doc_coverage.rs
+++ b/src/librustdoc/passes/calculate_doc_coverage.rs
@@ -240,7 +240,7 @@ impl DocVisitor<'_> for CoverageCalculator<'_, '_> {
                                 data: hir::VariantData::Tuple(_, _, _),
                                 ..
                             }) | hir::Node::Item(hir::Item {
-                                kind: hir::ItemKind::Struct(hir::VariantData::Tuple(_, _, _), _),
+                                kind: hir::ItemKind::Struct(_, hir::VariantData::Tuple(_, _, _), _),
                                 ..
                             })
                         )
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index 68e381fa3f1..254549e72c6 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -144,9 +144,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 && inserted.insert(def_id)
             {
                 let item = self.cx.tcx.hir_expect_item(local_def_id);
-                top_level_module
-                    .items
-                    .insert((local_def_id, Some(item.ident.name)), (item, None, None));
+                let (ident, _, _) = item.expect_macro();
+                top_level_module.items.insert((local_def_id, Some(ident.name)), (item, None, None));
             }
         }
 
@@ -224,11 +223,11 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         def_id: LocalDefId,
         res: Res,
         renamed: Option<Symbol>,
-        glob: bool,
         please_inline: bool,
     ) -> bool {
         debug!("maybe_inline_local (renamed: {renamed:?}) res: {res:?}");
 
+        let glob = renamed.is_none();
         if renamed == Some(kw::Underscore) {
             // We never inline `_` reexports.
             return false;
@@ -286,7 +285,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 is_hidden &&
                 // If it's a doc hidden module, we need to keep it in case some of its inner items
                 // are re-exported.
-                !matches!(item, Node::Item(&hir::Item { kind: hir::ItemKind::Mod(_), .. }))
+                !matches!(item, Node::Item(&hir::Item { kind: hir::ItemKind::Mod(..), .. }))
             ) ||
                 // The imported item is public and not `doc(hidden)` so no need to inline it.
                 self.reexport_public_and_not_hidden(def_id, res_did)
@@ -297,7 +296,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
 
         let is_bang_macro = matches!(
             item,
-            Node::Item(&hir::Item { kind: hir::ItemKind::Macro(_, MacroKind::Bang), .. })
+            Node::Item(&hir::Item { kind: hir::ItemKind::Macro(_, _, MacroKind::Bang), .. })
         );
 
         if !self.view_item_stack.insert(res_did) && !is_bang_macro {
@@ -311,7 +310,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             Node::Item(_) if is_bang_macro && !please_inline && renamed.is_some() && is_hidden => {
                 return false;
             }
-            Node::Item(&hir::Item { kind: hir::ItemKind::Mod(m), .. }) if glob => {
+            Node::Item(&hir::Item { kind: hir::ItemKind::Mod(_, m), .. }) if glob => {
                 let prev = mem::replace(&mut self.inlining, true);
                 for &i in m.item_ids {
                     let i = tcx.hir_item(i);
@@ -378,7 +377,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             // attribute can still be visible.
             || match item.kind {
                 hir::ItemKind::Impl(..) => true,
-                hir::ItemKind::Macro(_, MacroKind::Bang) => {
+                hir::ItemKind::Macro(_, _, MacroKind::Bang) => {
                     self.cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export)
                 }
                 _ => false,
@@ -419,7 +418,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             }
             return;
         }
-        let name = renamed.unwrap_or(item.ident.name);
+        let get_name = || renamed.unwrap_or(item.kind.ident().unwrap().name);
         let tcx = self.cx.tcx;
 
         let def_id = item.owner_id.to_def_id();
@@ -459,15 +458,13 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                             }
                             _ => false,
                         });
-                        let is_glob = kind == hir::UseKind::Glob;
-                        let ident = if is_glob { None } else { Some(name) };
-                        if self.maybe_inline_local(
-                            item.owner_id.def_id,
-                            res,
-                            ident,
-                            is_glob,
-                            please_inline,
-                        ) {
+                        let ident = match kind {
+                            hir::UseKind::Single(ident) => Some(renamed.unwrap_or(ident.name)),
+                            hir::UseKind::Glob => None,
+                            hir::UseKind::ListStem => unreachable!(),
+                        };
+                        if self.maybe_inline_local(item.owner_id.def_id, res, ident, please_inline)
+                        {
                             debug!("Inlining {:?}", item.owner_id.def_id);
                             continue;
                         }
@@ -475,7 +472,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                     self.add_to_current_mod(item, renamed, import_id);
                 }
             }
-            hir::ItemKind::Macro(macro_def, _) => {
+            hir::ItemKind::Macro(_, macro_def, _) => {
                 // `#[macro_export] macro_rules!` items are handled separately in `visit()`,
                 // above, since they need to be documented at the module top level. Accordingly,
                 // we only want to handle macros if one of three conditions holds:
@@ -495,8 +492,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                     self.add_to_current_mod(item, renamed, import_id);
                 }
             }
-            hir::ItemKind::Mod(m) => {
-                self.enter_mod(item.owner_id.def_id, m, name, renamed, import_id);
+            hir::ItemKind::Mod(_, m) => {
+                self.enter_mod(item.owner_id.def_id, m, get_name(), renamed, import_id);
             }
             hir::ItemKind::Fn { .. }
             | hir::ItemKind::ExternCrate(..)
@@ -512,7 +509,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             hir::ItemKind::Const(..) => {
                 // Underscore constants do not correspond to a nameable item and
                 // so are never useful in documentation.
-                if name != kw::Underscore {
+                if get_name() != kw::Underscore {
                     self.add_to_current_mod(item, renamed, import_id);
                 }
             }