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.

The commit amends a couple of places that do path splitting to ensure no
empty identifiers are created.

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,13 +2157,24 @@ 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)| {
+                let sym = if s.is_empty() {
+                    if i == 0 {
+                        // For a path like `::a::b`, use `kw::PathRoot` as the leading segment.
+                        kw::PathRoot
+                    } else {
+                        return Err(()); // occurs in cases like `String::`
+                    }
+                } 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) {
             PathResult::Module(ModuleOrUniformRoot::Module(module)) => Some(module.res().unwrap()),

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,21 @@ 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`).
-        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() }
-        })?;
+        let (path_root, item_str) = match path_str.rsplit_once("::") {
+            Some(res @ (_path_root, item_str)) if !item_str.is_empty() => res,
+            _ => {
+                // If there's no `::`, or the `::` is at the end (e.g. `String::`) it's not an
+                // associated item. So we can be sure that `rustc_resolve` was accurate when it
+                // said it wasn't resolved.
+                debug!("`::` missing or at end, assuming {path_str} was not in scope");
+                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