Remove Ident::empty.

All uses have been removed. And it's nonsensical: an identifier by
definition has at least one char.

The commits adds an is-non-empty assertion to `Ident::new` to enforce
this, and converts some `Ident` constructions to use `Ident::new`.
Adding the assertion requires making `Ident::new` and
`Ident::with_dummy_span` non-const, which is no great loss.

Author: nnethercote

compiler/rustc_ast_pretty/src/pprust/state/expr.rs

@@ -876,6 +876,7 @@ impl<'a> State<'a> {
                 }
             }
         } else {
+            self.end(); // Close the ibox for the pattern.
             self.word(",");
         }
         self.end(); // Close enclosing cbox.

compiler/rustc_hir/src/hir/tests.rs

@@ -50,21 +50,14 @@ fn trait_object_roundtrips() {
 }
 
 fn trait_object_roundtrips_impl(syntax: TraitObjectSyntax) {
-    let unambig = TyKind::TraitObject::<'_, ()>(
-        &[],
-        TaggedRef::new(
-            &const {
-                Lifetime {
-                    hir_id: HirId::INVALID,
-                    ident: Ident::new(sym::name, DUMMY_SP),
-                    kind: LifetimeKind::Static,
-                    source: LifetimeSource::Other,
-                    syntax: LifetimeSyntax::Hidden,
-                }
-            },
-            syntax,
-        ),
-    );
+    let lt = Lifetime {
+        hir_id: HirId::INVALID,
+        ident: Ident::new(sym::name, DUMMY_SP),
+        kind: LifetimeKind::Static,
+        source: LifetimeSource::Other,
+        syntax: LifetimeSyntax::Hidden,
+    };
+    let unambig = TyKind::TraitObject::<'_, ()>(&[], TaggedRef::new(&lt, syntax));
     let unambig_to_ambig = unsafe { std::mem::transmute::<_, TyKind<'_, AmbigArg>>(unambig) };
 
     match unambig_to_ambig {

compiler/rustc_parse/src/parser/expr.rs

@@ -3828,7 +3828,7 @@ impl<'a> Parser<'a> {
         // Convert `label` -> `'label`,
         // so that nameres doesn't complain about non-existing label
         let label = format!("'{}", ident.name);
-        let ident = Ident { name: Symbol::intern(&label), span: ident.span };
+        let ident = Ident::new(Symbol::intern(&label), ident.span);
 
         self.dcx().emit_err(errors::ExpectedLabelFoundIdent {
             span: ident.span,

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -549,7 +549,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                         source = module_path.pop().unwrap();
                         if rename.is_none() {
                             // Keep the span of `self`, but the name of `foo`
-                            ident = Ident { name: source.ident.name, span: self_span };
+                            ident = Ident::new(source.ident.name, self_span);
                         }
                     }
                 } else {
@@ -597,7 +597,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                         if let Some(crate_name) = crate_name {
                             // `crate_name` should not be interpreted as relative.
                             module_path.push(Segment::from_ident_and_id(
-                                Ident { name: kw::PathRoot, span: source.ident.span },
+                                Ident::new(kw::PathRoot, source.ident.span),
                                 self.r.next_node_id(),
                             ));
                             source.ident.name = crate_name;

compiler/rustc_resolve/src/lib.rs

@@ -2157,15 +2157,27 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         ns: Namespace,
         parent_scope: ParentScope<'ra>,
     ) -> Option<Res> {
-        let mut segments =
-            Vec::from_iter(path_str.split("::").map(Ident::from_str).map(Segment::from_ident));
-        if let Some(segment) = segments.first_mut() {
-            if segment.ident.name == kw::Empty {
-                segment.ident.name = kw::PathRoot;
-            }
-        }
+        let segments: Result<Vec<_>, ()> = path_str
+            .split("::")
+            .enumerate()
+            .map(|(i, s)| {
+                // For a path like `::a::b`, use `kw::PathRoot` as the leading segment.
+                // njn: explain `Foo::`
+                let sym = if s.is_empty() {
+                    if i == 0 {
+                        kw::PathRoot
+                    } else {
+                        return Err(());
+                    }
+                } else {
+                    Symbol::intern(s)
+                };
+                Ok(Segment::from_ident(Ident { name: sym, span: DUMMY_SP }))
+            })
+            .collect();
+        let Ok(segments) = segments else { return None };
 
-        match self.maybe_resolve_path(&segments, Some(ns), &parent_scope, None) {
+        let res = match self.maybe_resolve_path(&segments, Some(ns), &parent_scope, None) {
             PathResult::Module(ModuleOrUniformRoot::Module(module)) => Some(module.res().unwrap()),
             PathResult::NonModule(path_res) => {
                 path_res.full_res().filter(|res| !matches!(res, Res::Def(DefKind::Ctor(..), _)))
@@ -2174,7 +2186,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 None
             }
             PathResult::Module(..) | PathResult::Indeterminate => unreachable!(),
-        }
+        };
+        //eprintln!("res = {res:?}");
+        res
     }
 
     /// Retrieves definition span of the given `DefId`.

compiler/rustc_span/src/symbol.rs

@@ -2337,35 +2337,31 @@ pub const STDLIB_STABLE_CRATES: &[Symbol] = &[sym::std, sym::core, sym::alloc, s
 
 #[derive(Copy, Clone, Eq, HashStable_Generic, Encodable, Decodable)]
 pub struct Ident {
+    // `name` should never be the empty symbol. If you are considering that,
+    // you are probably conflating "empty identifer with "no identifier" and
+    // you should use `Option<Ident>` instead.
     pub name: Symbol,
     pub span: Span,
 }
 
 impl Ident {
     #[inline]
     /// Constructs a new identifier from a symbol and a span.
-    pub const fn new(name: Symbol, span: Span) -> Ident {
+    pub fn new(name: Symbol, span: Span) -> Ident {
+        assert_ne!(name, kw::Empty);
         Ident { name, span }
     }
 
     /// Constructs a new identifier with a dummy span.
     #[inline]
-    pub const fn with_dummy_span(name: Symbol) -> Ident {
+    pub fn with_dummy_span(name: Symbol) -> Ident {
         Ident::new(name, DUMMY_SP)
     }
 
-    /// This is best avoided, because it blurs the lines between "empty
-    /// identifier" and "no identifier". Using `Option<Ident>` is preferable,
-    /// where possible, because that is unambiguous.
-    #[inline]
-    pub fn empty() -> Ident {
-        Ident::with_dummy_span(kw::Empty)
-    }
-
     // For dummy identifiers that are never used and absolutely must be
-    // present, it's better to use `Ident::dummy` than `Ident::Empty`, because
-    // it's clearer that it's intended as a dummy value, and more likely to be
-    // detected if it accidentally does get used.
+    // present. Note that this does *not* use the empty symbol; `sym::dummy`
+    // makes it clear that it's intended as a dummy value, and is more likely
+    // to be detected if it accidentally does get used.
     #[inline]
     pub fn dummy() -> Ident {
         Ident::with_dummy_span(sym::dummy)

src/librustdoc/clean/render_macro_matchers.rs

@@ -167,7 +167,7 @@ fn print_tts(printer: &mut Printer<'_>, tts: &TokenStream) {
 }
 
 fn usually_needs_space_between_keyword_and_open_delim(symbol: Symbol, span: Span) -> bool {
-    let ident = Ident { name: symbol, span };
+    let ident = Ident::new(symbol, span);
     let is_keyword = ident.is_used_keyword() || ident.is_unused_keyword();
     if !is_keyword {
         // An identifier that is not a keyword usually does not need a space

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -496,12 +496,22 @@ impl<'tcx> LinkCollector<'_, 'tcx> {
 
         // Try looking for methods and associated items.
         // NB: `path_root` could be empty when resolving in the root namespace (e.g. `::std`).
+        //eprintln!("path_str = `{path_str}`"); // njn: `String::` again
         let (path_root, item_str) = path_str.rsplit_once("::").ok_or_else(|| {
             // If there's no `::`, it's not an associated item.
             // So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved.
             debug!("found no `::`, assuming {path_str} was correctly not in scope");
             UnresolvedPath { item_id, module_id, partial_res: None, unresolved: path_str.into() }
         })?;
+        if item_str.is_empty() {
+            // njn: explain this
+            return Err(UnresolvedPath {
+                item_id,
+                module_id,
+                partial_res: None,
+                unresolved: path_str.into(),
+            });
+        }
         let item_name = Symbol::intern(item_str);
 
         // FIXME(#83862): this arbitrarily gives precedence to primitives over modules to support
@@ -511,6 +521,7 @@ impl<'tcx> LinkCollector<'_, 'tcx> {
         match resolve_primitive(path_root, TypeNS)
             .or_else(|| self.resolve_path(path_root, TypeNS, item_id, module_id))
             .map(|ty_res| {
+                // njn: here 2, item_name is empty
                 self.resolve_associated_item(ty_res, item_name, ns, disambiguator, module_id)
                     .into_iter()
                     .map(|(res, def_id)| (res, Some(def_id)))
@@ -704,6 +715,7 @@ impl<'tcx> LinkCollector<'_, 'tcx> {
                         filter_assoc_items_by_name_and_namespace(
                             tcx,
                             imp,
+                            // njn: tests/rustdoc-ui/intra-doc/empty-associated-items.rs fails here
                             Ident::with_dummy_span(item_name),
                             ns,
                         )