Rollup merge of #144439 - xizheyin:symbol-rs, r=petrochenkov

Introduce ModernIdent type to unify macro 2.0 hygiene handling

This pr introduce ModernIdent type to unify macro 2.0 hygiene handling

1. Added ModernIdent type. Wraps Ident and automatically calls `normalize_to_macros_2_0()`
2. Unified identifier normalization. Replaced scattered ident.normalize_to_macros_2_0() calls with ModernIdent::new(ident)

r? ````@petrochenkov````

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,8 +969,8 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
         self.r.potentially_unused_imports.push(import);
         let imported_binding = self.r.import(binding, import);
         if ident.name != kw::Underscore && parent == self.r.graph_root {
-            let ident = ident.normalize_to_macros_2_0();
+            let norm_ident = Macros20NormalizedIdent::new(ident);
-            if let Some(entry) = self.r.extern_prelude.get(&ident)
+            if let Some(entry) = self.r.extern_prelude.get(&norm_ident)
                 && expansion != LocalExpnId::ROOT
                 && orig_name.is_some()
                 && !entry.is_import()
@@ -986,7 +986,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             }
 
             use indexmap::map::Entry;
-            match self.r.extern_prelude.entry(ident) {
+            match self.r.extern_prelude.entry(norm_ident) {
                 Entry::Occupied(mut occupied) => {
                     let entry = occupied.get_mut();
                     if let Some(old_binding) = entry.binding.get()

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(&Macros20NormalizedIdent::new(extern_crate.ident))
                 .is_none_or(|entry| entry.introduced_by_item)
             {
                 continue;

compiler/rustc_resolve/src/diagnostics.rs

@@ -30,7 +30,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};
 
@@ -320,8 +320,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 =
+        let from_item = self
-            self.extern_prelude.get(&ident).is_none_or(|entry| entry.introduced_by_item);
+            .extern_prelude
+            .get(&Macros20NormalizedIdent::new(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.
@@ -530,7 +532,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.keys().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
@@ -1453,7 +1455,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,

compiler/rustc_resolve/src/imports.rs

@@ -489,7 +489,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // Define or update `binding` in `module`s glob importers.
         for import in glob_importers.iter() {
             let mut ident = key.ident;
-            let scope = match ident.span.reverse_glob_adjust(module.expansion, import.span) {
+            let scope = match ident.0.span.reverse_glob_adjust(module.expansion, import.span) {
                 Some(Some(def)) => self.expn_def_scope(def),
                 Some(None) => import.parent_scope.module,
                 None => continue,
@@ -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,
@@ -1504,7 +1504,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             })
             .collect::<Vec<_>>();
         for (mut key, binding) in bindings {
-            let scope = match key.ident.span.reverse_glob_adjust(module.expansion, import.span) {
+            let scope = match key.ident.0.span.reverse_glob_adjust(module.expansion, import.span) {
                 Some(Some(def)) => self.expn_def_scope(def),
                 Some(None) => import.parent_scope.module,
                 None => continue,
@@ -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 });
             }
         });
 

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -1472,7 +1472,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,
+                    ));
                 }
             }
         }
@@ -2479,7 +2482,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
                             names.extend(self.r.extern_prelude.keys().flat_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))
+                                    .then_some(TypoSuggestion::typo_from_ident(ident.0, res))
                             }));
 
                             if let Some(prelude) = self.r.prelude {
@@ -2639,7 +2642,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 {
@@ -2678,7 +2681,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));
                 }

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 }
     }
 }
 
@@ -593,7 +593,8 @@ struct ModuleData<'ra> {
     globs: RefCell<Vec<Import<'ra>>>,
 
     /// Used to memoize the traits in this module for faster searches through all traits in scope.
-    traits: RefCell<Option<Box<[(Ident, NameBinding<'ra>, Option<Module<'ra>>)]>>>,
+    traits:
+        RefCell<Option<Box<[(Macros20NormalizedIdent, NameBinding<'ra>, Option<Module<'ra>>)]>>>,
 
     /// Span of the module itself. Used for error reporting.
     span: Span,
@@ -659,7 +660,7 @@ impl<'ra> Module<'ra> {
     fn for_each_child<'tcx, R: AsRef<Resolver<'ra, 'tcx>>>(
         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 +672,7 @@ impl<'ra> Module<'ra> {
     fn for_each_child_mut<'tcx, R: AsMut<Resolver<'ra, 'tcx>>>(
         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() {
@@ -1054,7 +1055,7 @@ pub struct Resolver<'ra, 'tcx> {
     graph_root: Module<'ra>,
 
     prelude: Option<Module<'ra>>,
-    extern_prelude: FxIndexMap<Ident, ExternPreludeEntry<'ra>>,
+    extern_prelude: FxIndexMap<Macros20NormalizedIdent, ExternPreludeEntry<'ra>>,
 
     /// N.B., this is used only for better diagnostics, not name resolution itself.
     field_names: LocalDefIdMap<Vec<Ident>>,
@@ -1499,7 +1500,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     && let name = Symbol::intern(name)
                     && name.can_be_raw()
                 {
-                    Some((Ident::with_dummy_span(name), Default::default()))
+                    Some((Macros20NormalizedIdent::with_dummy_span(name), Default::default()))
                 } else {
                     None
                 }
@@ -1507,9 +1508,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             .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());
             }
         }
 
@@ -1879,7 +1882,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         for &(trait_name, trait_binding, trait_module) in traits.as_ref().unwrap().iter() {
             if self.trait_may_have_item(trait_module, assoc_item) {
                 let def_id = trait_binding.res().def_id();
-                let import_ids = self.find_transitive_imports(&trait_binding.kind, trait_name);
+                let import_ids = self.find_transitive_imports(&trait_binding.kind, trait_name.0);
                 found_traits.push(TraitCandidate { def_id, import_ids });
             }
         }
@@ -2020,7 +2023,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;
                     }
@@ -2179,7 +2182,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
     fn extern_prelude_get(&mut self, ident: Ident, finalize: bool) -> Option<NameBinding<'ra>> {
         let mut record_use = None;
-        let entry = self.extern_prelude.get(&ident.normalize_to_macros_2_0());
+        let entry = self.extern_prelude.get(&Macros20NormalizedIdent::new(ident));
         let binding = entry.and_then(|entry| match entry.binding.get() {
             Some(binding) if binding.is_import() => {
                 if finalize {

compiler/rustc_resolve/src/macros.rs

@@ -536,11 +536,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)

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;

compiler/rustc_span/src/symbol.rs

@@ -3,6 +3,7 @@
 //! type, and vice versa.
 
 use std::hash::{Hash, Hasher};
+use std::ops::Deref;
 use std::{fmt, str};
 
 use rustc_arena::DroplessArena;
@@ -2563,16 +2564,17 @@ impl fmt::Display for IdentPrinter {
 }
 
 /// An newtype around `Ident` that calls [Ident::normalize_to_macro_rules] on
-/// construction.
+/// construction for "local variable hygiene" comparisons.
-// FIXME(matthewj, petrochenkov) Use this more often, add a similar
+///
-// `ModernIdent` struct and use that as well.
+/// 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())
     }
 }
 
@@ -2588,6 +2590,48 @@ 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())
+    }
+
+    // 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)
+    }
+}
+
+/// By impl Deref, we can access the wrapped Ident as if it were a normal Ident
+/// such as `norm_ident.name` instead of `norm_ident.0.name`.
+impl Deref for Macros20NormalizedIdent {
+    type Target = Ident;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
 /// An interned UTF-8 string.
 ///
 /// Internally, a `Symbol` is implemented as an index, and all operations