From 00a578580f9176f41d312b6bfcc06fde99bbba88 Mon Sep 17 00:00:00 2001 From: xizheyin Date: Mon, 4 Aug 2025 11:12:00 +0800 Subject: [PATCH] Introduce ModernIdent type to unify macro 2.0 hygiene handling Signed-off-by: xizheyin --- .../rustc_resolve/src/build_reduced_graph.rs | 4 +- compiler/rustc_resolve/src/check_unused.rs | 4 +- compiler/rustc_resolve/src/diagnostics.rs | 18 +++--- compiler/rustc_resolve/src/imports.rs | 6 +- .../rustc_resolve/src/late/diagnostics.rs | 11 ++-- compiler/rustc_resolve/src/lib.rs | 37 ++++++----- compiler/rustc_resolve/src/macros.rs | 4 +- compiler/rustc_span/src/lib.rs | 3 +- compiler/rustc_span/src/symbol.rs | 61 +++++++++++++++++-- 9 files changed, 108 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 7912345ec56ac..0e055d38cd6fe 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -27,7 +27,7 @@ use rustc_middle::metadata::ModChild; use rustc_middle::ty::{Feed, Visibility}; use rustc_middle::{bug, span_bug}; use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind}; -use rustc_span::{Ident, Span, Symbol, kw, sym}; +use rustc_span::{Ident, Macros20NormalizedIdent, Span, Symbol, kw, sym}; use thin_vec::ThinVec; use tracing::debug; @@ -969,7 +969,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { self.r.potentially_unused_imports.push(import); let imported_binding = self.r.import(binding, import); if parent == self.r.graph_root { - let ident = ident.normalize_to_macros_2_0(); + let ident = Macros20NormalizedIdent::new(ident); if let Some(entry) = self.r.extern_prelude.get(&ident) && expansion != LocalExpnId::ROOT && orig_name.is_some() diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index b85a814776a7f..294a669534dce 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -33,7 +33,7 @@ use rustc_session::lint::BuiltinLintDiag; use rustc_session::lint::builtin::{ MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS, UNUSED_QUALIFICATIONS, }; -use rustc_span::{DUMMY_SP, Ident, Span, kw}; +use rustc_span::{DUMMY_SP, Ident, Macros20NormalizedIdent, Span, kw}; use crate::imports::{Import, ImportKind}; use crate::{LexicalScopeBinding, NameBindingKind, Resolver, module_to_string}; @@ -203,7 +203,7 @@ impl<'a, 'ra, 'tcx> UnusedImportCheckVisitor<'a, 'ra, 'tcx> { if self .r .extern_prelude - .get(&extern_crate.ident) + .get(&unsafe { Macros20NormalizedIdent::new_without_normalize(extern_crate.ident) }) .is_none_or(|entry| entry.introduced_by_item) { continue; diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 3af69b28780d0..554aba8d30070 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -32,7 +32,7 @@ use rustc_span::edit_distance::find_best_match_for_name; use rustc_span::edition::Edition; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, Ident, Span, Symbol, SyntaxContext, kw, sym}; +use rustc_span::{BytePos, Ident, Macros20NormalizedIdent, Span, Symbol, SyntaxContext, kw, sym}; use thin_vec::{ThinVec, thin_vec}; use tracing::{debug, instrument}; @@ -322,8 +322,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Check if the target of the use for both bindings is the same. let duplicate = new_binding.res().opt_def_id() == old_binding.res().opt_def_id(); let has_dummy_span = new_binding.span.is_dummy() || old_binding.span.is_dummy(); - let from_item = - self.extern_prelude.get(&ident).is_none_or(|entry| entry.introduced_by_item); + let from_item = self + .extern_prelude + .get(&unsafe { Macros20NormalizedIdent::new_without_normalize(ident) }) + .is_none_or(|entry| entry.introduced_by_item); // Only suggest removing an import if both bindings are to the same def, if both spans // aren't dummy spans. Further, if both bindings are imports, then the ident must have // been introduced by an item. @@ -532,7 +534,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { module.for_each_child(self, |_this, ident, _ns, binding| { let res = binding.res(); if filter_fn(res) && ctxt.is_none_or(|ctxt| ctxt == ident.span.ctxt()) { - names.push(TypoSuggestion::typo_from_ident(ident, res)); + names.push(TypoSuggestion::typo_from_ident(ident.0, res)); } }); } @@ -1100,7 +1102,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Scope::ExternPrelude => { suggestions.extend(this.extern_prelude.iter().filter_map(|(ident, _)| { let res = Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id()); - filter_fn(res).then_some(TypoSuggestion::typo_from_ident(*ident, res)) + filter_fn(res).then_some(TypoSuggestion::typo_from_ident(ident.0, res)) })); } Scope::ToolPrelude => { @@ -1246,7 +1248,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }; segms.append(&mut path_segments.clone()); - segms.push(ast::PathSegment::from_ident(ident)); + segms.push(ast::PathSegment::from_ident(ident.0)); let path = Path { span: name_binding.span, segments: segms, tokens: None }; if child_accessible @@ -1319,7 +1321,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { if let Some(def_id) = name_binding.res().module_like_def_id() { // form the path let mut path_segments = path_segments.clone(); - path_segments.push(ast::PathSegment::from_ident(ident)); + path_segments.push(ast::PathSegment::from_ident(ident.0)); let alias_import = if let NameBindingKind::Import { import, .. } = name_binding.kind @@ -1455,7 +1457,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { if needs_disambiguation { crate_path.push(ast::PathSegment::path_root(rustc_span::DUMMY_SP)); } - crate_path.push(ast::PathSegment::from_ident(ident)); + crate_path.push(ast::PathSegment::from_ident(ident.0)); suggestions.extend(self.lookup_import_candidates_from_module( lookup_ident, diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 156df45147fd7..c5b618938ccf4 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -498,7 +498,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let imported_binding = self.import(binding, *import); let _ = self.try_define_local( import.parent_scope.module, - ident, + ident.0, key.ns, imported_binding, warn_ambiguity, @@ -1517,7 +1517,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { .is_some_and(|binding| binding.warn_ambiguity_recursive()); let _ = self.try_define_local( import.parent_scope.module, - key.ident, + key.ident.0, key.ns, imported_binding, warn_ambiguity, @@ -1550,7 +1550,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { next_binding = binding; } - children.push(ModChild { ident, res, vis: binding.vis, reexport_chain }); + children.push(ModChild { ident: ident.0, res, vis: binding.vis, reexport_chain }); } }); diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 98e48664e6816..0534909d31867 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1473,7 +1473,10 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { }) .collect(); if let [target] = targets.as_slice() { - return Some(TypoSuggestion::single_item_from_ident(target.0.ident, target.1)); + return Some(TypoSuggestion::single_item_from_ident( + target.0.ident.0, + target.1, + )); } } } @@ -2487,7 +2490,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { Res::Def(DefKind::Mod, crate_id.as_def_id()); filter_fn(crate_mod).then(|| { - TypoSuggestion::typo_from_ident(*ident, crate_mod) + TypoSuggestion::typo_from_ident(ident.0, crate_mod) }) }) })); @@ -2649,7 +2652,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { if let Some(module_def_id) = name_binding.res().module_like_def_id() { // form the path let mut path_segments = path_segments.clone(); - path_segments.push(ast::PathSegment::from_ident(ident)); + path_segments.push(ast::PathSegment::from_ident(ident.0)); let doc_visible = doc_visible && (module_def_id.is_local() || !r.tcx.is_doc_hidden(module_def_id)); if module_def_id == def_id { @@ -2688,7 +2691,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { enum_module.for_each_child(self.r, |_, ident, _, name_binding| { if let Res::Def(DefKind::Ctor(CtorOf::Variant, kind), def_id) = name_binding.res() { let mut segms = enum_import_suggestion.path.segments.clone(); - segms.push(ast::PathSegment::from_ident(ident)); + segms.push(ast::PathSegment::from_ident(ident.0)); let path = Path { span: name_binding.span, segments: segms, tokens: None }; variants.push((path, def_id, kind)); } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 88dfb50b47dd7..ee1ed72a25b37 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -71,7 +71,7 @@ use rustc_query_system::ich::StableHashingContext; use rustc_session::lint::builtin::PRIVATE_MACRO_USE; use rustc_session::lint::{BuiltinLintDiag, LintBuffer}; use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind, SyntaxContext, Transparency}; -use rustc_span::{DUMMY_SP, Ident, Span, Symbol, kw, sym}; +use rustc_span::{DUMMY_SP, Ident, Macros20NormalizedIdent, Span, Symbol, kw, sym}; use smallvec::{SmallVec, smallvec}; use tracing::debug; @@ -531,7 +531,7 @@ impl ModuleKind { struct BindingKey { /// The identifier for the binding, always the `normalize_to_macros_2_0` version of the /// identifier. - ident: Ident, + ident: Macros20NormalizedIdent, ns: Namespace, /// When we add an underscore binding (with ident `_`) to some module, this field has /// a non-zero value that uniquely identifies this binding in that module. @@ -543,7 +543,7 @@ struct BindingKey { impl BindingKey { fn new(ident: Ident, ns: Namespace) -> Self { - BindingKey { ident: ident.normalize_to_macros_2_0(), ns, disambiguator: 0 } + BindingKey { ident: Macros20NormalizedIdent::new(ident), ns, disambiguator: 0 } } fn new_disambiguated( @@ -552,7 +552,7 @@ impl BindingKey { disambiguator: impl FnOnce() -> u32, ) -> BindingKey { let disambiguator = if ident.name == kw::Underscore { disambiguator() } else { 0 }; - BindingKey { ident: ident.normalize_to_macros_2_0(), ns, disambiguator } + BindingKey { ident: Macros20NormalizedIdent::new(ident), ns, disambiguator } } } @@ -659,7 +659,7 @@ impl<'ra> Module<'ra> { fn for_each_child<'tcx, R: AsRef>>( self, resolver: &R, - mut f: impl FnMut(&R, Ident, Namespace, NameBinding<'ra>), + mut f: impl FnMut(&R, Macros20NormalizedIdent, Namespace, NameBinding<'ra>), ) { for (key, name_resolution) in resolver.as_ref().resolutions(self).borrow().iter() { if let Some(binding) = name_resolution.borrow().best_binding() { @@ -671,7 +671,7 @@ impl<'ra> Module<'ra> { fn for_each_child_mut<'tcx, R: AsMut>>( self, resolver: &mut R, - mut f: impl FnMut(&mut R, Ident, Namespace, NameBinding<'ra>), + mut f: impl FnMut(&mut R, Macros20NormalizedIdent, Namespace, NameBinding<'ra>), ) { for (key, name_resolution) in resolver.as_mut().resolutions(self).borrow().iter() { if let Some(binding) = name_resolution.borrow().best_binding() { @@ -690,7 +690,7 @@ impl<'ra> Module<'ra> { return; } if let Res::Def(DefKind::Trait | DefKind::TraitAlias, def_id) = binding.res() { - collected_traits.push((name, binding, r.as_ref().get_module(def_id))) + collected_traits.push((name.0, binding, r.as_ref().get_module(def_id))) } }); *traits = Some(collected_traits.into_boxed_slice()); @@ -1049,7 +1049,7 @@ pub struct Resolver<'ra, 'tcx> { graph_root: Module<'ra>, prelude: Option>, - extern_prelude: FxIndexMap>, + extern_prelude: FxIndexMap>, /// N.B., this is used only for better diagnostics, not name resolution itself. field_names: LocalDefIdMap>, @@ -1482,19 +1482,28 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut invocation_parents = FxHashMap::default(); invocation_parents.insert(LocalExpnId::ROOT, InvocationParent::ROOT); - let mut extern_prelude: FxIndexMap> = tcx + let mut extern_prelude: FxIndexMap> = tcx .sess .opts .externs .iter() .filter(|(_, entry)| entry.add_prelude) - .map(|(name, _)| (Ident::from_str(name), Default::default())) + .map(|(name, _)| { + ( + unsafe { + Macros20NormalizedIdent::new_without_normalize(Ident::from_str(name)) + }, + Default::default(), + ) + }) .collect(); if !attr::contains_name(attrs, sym::no_core) { - extern_prelude.insert(Ident::with_dummy_span(sym::core), Default::default()); + extern_prelude + .insert(Macros20NormalizedIdent::with_dummy_span(sym::core), Default::default()); if !attr::contains_name(attrs, sym::no_std) { - extern_prelude.insert(Ident::with_dummy_span(sym::std), Default::default()); + extern_prelude + .insert(Macros20NormalizedIdent::with_dummy_span(sym::std), Default::default()); } } @@ -2005,7 +2014,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Avoid marking `extern crate` items that refer to a name from extern prelude, // but not introduce it, as used if they are accessed from lexical scope. if used == Used::Scope { - if let Some(entry) = self.extern_prelude.get(&ident.normalize_to_macros_2_0()) { + if let Some(entry) = self.extern_prelude.get(&Macros20NormalizedIdent::new(ident)) { if !entry.introduced_by_item && entry.binding.get() == Some(used_binding) { return; } @@ -2168,7 +2177,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return None; } - let norm_ident = ident.normalize_to_macros_2_0(); + let norm_ident = Macros20NormalizedIdent::new(ident); let binding = self.extern_prelude.get(&norm_ident).cloned().and_then(|entry| { Some(if let Some(binding) = entry.binding.get() { if finalize { diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 20504ea609df3..6e498a3c91d29 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -535,11 +535,11 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { target_trait.for_each_child(self, |this, ident, ns, _binding| { // FIXME: Adjust hygiene for idents from globs, like for glob imports. if let Some(overriding_keys) = this.impl_binding_keys.get(&impl_def_id) - && overriding_keys.contains(&BindingKey::new(ident, ns)) + && overriding_keys.contains(&BindingKey::new(ident.0, ns)) { // The name is overridden, do not produce it from the glob delegation. } else { - idents.push((ident, None)); + idents.push((ident.0, None)); } }); Ok(idents) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 3f72ccd9f89dc..d647ec28aae5c 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -66,7 +66,8 @@ pub use span_encoding::{DUMMY_SP, Span}; pub mod symbol; pub use symbol::{ - ByteSymbol, Ident, MacroRulesNormalizedIdent, STDLIB_STABLE_CRATES, Symbol, kw, sym, + ByteSymbol, Ident, MacroRulesNormalizedIdent, Macros20NormalizedIdent, STDLIB_STABLE_CRATES, + Symbol, kw, sym, }; mod analyze_source_file; diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d54175548e30e..fd106bccedd79 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -3,6 +3,7 @@ //! type, and vice versa. use std::hash::{Hash, Hasher}; +use std::ops::{Deref, DerefMut}; use std::{fmt, str}; use rustc_arena::DroplessArena; @@ -2562,16 +2563,17 @@ impl fmt::Display for IdentPrinter { } /// An newtype around `Ident` that calls [Ident::normalize_to_macro_rules] on -/// construction. -// FIXME(matthewj, petrochenkov) Use this more often, add a similar -// `ModernIdent` struct and use that as well. +/// construction for "local variable hygiene" comparisons. +/// +/// Use this type when you need to compare identifiers according to macro_rules hygiene. +/// This ensures compile-time safety and avoids manual normalization calls. #[derive(Copy, Clone, Eq, PartialEq, Hash)] pub struct MacroRulesNormalizedIdent(Ident); impl MacroRulesNormalizedIdent { #[inline] pub fn new(ident: Ident) -> Self { - Self(ident.normalize_to_macro_rules()) + MacroRulesNormalizedIdent(ident.normalize_to_macro_rules()) } } @@ -2587,6 +2589,57 @@ impl fmt::Display for MacroRulesNormalizedIdent { } } +/// An newtype around `Ident` that calls [Ident::normalize_to_macros_2_0] on +/// construction for "item hygiene" comparisons. +/// +/// Identifiers with same string value become same if they came from the same macro 2.0 macro +/// (e.g., `macro` item, but not `macro_rules` item) and stay different if they came from +/// different macro 2.0 macros. +#[derive(Copy, Clone, Eq, PartialEq, Hash)] +pub struct Macros20NormalizedIdent(pub Ident); + +impl Macros20NormalizedIdent { + #[inline] + pub fn new(ident: Ident) -> Self { + Macros20NormalizedIdent(ident.normalize_to_macros_2_0()) + } + + // For search in index map for `Ident` as a index key + pub unsafe fn new_without_normalize(ident: Ident) -> Self { + Macros20NormalizedIdent(ident) + } + + // dummy_span does not need to be normalized, so we can use `Ident` directly + pub fn with_dummy_span(name: Symbol) -> Self { + Macros20NormalizedIdent(Ident::with_dummy_span(name)) + } +} + +impl fmt::Debug for Macros20NormalizedIdent { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.0, f) + } +} + +impl fmt::Display for Macros20NormalizedIdent { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } +} + +impl Deref for Macros20NormalizedIdent { + type Target = Ident; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for Macros20NormalizedIdent { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + /// An interned UTF-8 string. /// /// Internally, a `Symbol` is implemented as an index, and all operations