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.
This commit is contained in:
Nicholas Nethercote 2025-04-24 11:56:44 +10:00
parent 667247db71
commit 0984db553d
7 changed files with 54 additions and 45 deletions

View file

@ -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 {

View file

@ -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,

View file

@ -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;

View file

@ -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::with_dummy_span(sym)))
})
.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()),

View file

@ -2342,6 +2342,9 @@ 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,
}
@ -2349,28 +2352,21 @@ pub struct Ident {
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)

View file

@ -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

View file

@ -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