resolve: Use Interned for NameBinding

Author: petrochenkov

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -9,7 +9,7 @@ use crate::def_collector::collect_definitions;
 use crate::imports::{Import, ImportKind};
 use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
 use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
-use crate::{errors, BindingKey, MacroData};
+use crate::{errors, BindingKey, MacroData, NameBindingData};
 use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
 use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
 use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};
@@ -38,8 +38,8 @@ type Res = def::Res<NodeId>;
 impl<'a, Id: Into<DefId>> ToNameBinding<'a>
     for (Module<'a>, ty::Visibility<Id>, Span, LocalExpnId)
 {
-    fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
-        arenas.alloc_name_binding(NameBinding {
+    fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
+        arenas.alloc_name_binding(NameBindingData {
             kind: NameBindingKind::Module(self.0),
             ambiguity: None,
             vis: self.1.to_def_id(),
@@ -50,8 +50,8 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a>
 }
 
 impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span, LocalExpnId) {
-    fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
-        arenas.alloc_name_binding(NameBinding {
+    fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
+        arenas.alloc_name_binding(NameBindingData {
             kind: NameBindingKind::Res(self.0),
             ambiguity: None,
             vis: self.1.to_def_id(),
@@ -71,7 +71,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         let binding = def.to_name_binding(self.arenas);
         let key = self.new_disambiguated_key(ident, ns);
         if let Err(old_binding) = self.try_define(parent, key, binding) {
-            self.report_conflict(parent, ident, ns, old_binding, &binding);
+            self.report_conflict(parent, ident, ns, old_binding, binding);
         }
     }
 
@@ -996,7 +996,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
     fn add_macro_use_binding(
         &mut self,
         name: Symbol,
-        binding: &'a NameBinding<'a>,
+        binding: NameBinding<'a>,
         span: Span,
         allow_shadowing: bool,
     ) {

compiler/rustc_resolve/src/diagnostics.rs

@@ -182,13 +182,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         }
     }
 
-    pub(crate) fn report_conflict<'b>(
+    pub(crate) fn report_conflict(
         &mut self,
         parent: Module<'_>,
         ident: Ident,
         ns: Namespace,
-        new_binding: &NameBinding<'b>,
-        old_binding: &NameBinding<'b>,
+        new_binding: NameBinding<'a>,
+        old_binding: NameBinding<'a>,
     ) {
         // Error on the second of two conflicting names
         if old_binding.span.lo() > new_binding.span.lo() {
@@ -262,7 +262,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         // See https://github.com/rust-lang/rust/issues/32354
         use NameBindingKind::Import;
-        let can_suggest = |binding: &NameBinding<'_>, import: &self::Import<'_>| {
+        let can_suggest = |binding: NameBinding<'_>, import: &self::Import<'_>| {
             !binding.span.is_dummy()
                 && !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport)
         };
@@ -455,7 +455,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         &mut self,
         finalize: Option<Finalize>,
         path: &[Segment],
-        second_binding: Option<&NameBinding<'_>>,
+        second_binding: Option<NameBinding<'_>>,
     ) {
         let Some(Finalize { node_id, root_span, .. }) = finalize else {
             return;
@@ -1515,7 +1515,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         true
     }
 
-    fn binding_description(&self, b: &NameBinding<'_>, ident: Ident, from_prelude: bool) -> String {
+    fn binding_description(&self, b: NameBinding<'_>, ident: Ident, from_prelude: bool) -> String {
         let res = b.res();
         if b.span.is_dummy() || !self.tcx.sess.source_map().is_span_accessible(b.span) {
             // These already contain the "built-in" prefix or look bad with it.
@@ -1555,7 +1555,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         err.span_label(ident.span, "ambiguous name");
         err.note(format!("ambiguous because of {}", kind.descr()));
 
-        let mut could_refer_to = |b: &NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
+        let mut could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
             let what = self.binding_description(b, ident, misc == AmbiguityErrorMisc::FromPrelude);
             let note_msg = format!("`{ident}` could{also} refer to {what}");
 
@@ -1595,7 +1595,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
     /// If the binding refers to a tuple struct constructor with fields,
     /// returns the span of its fields.
-    fn ctor_fields_span(&self, binding: &NameBinding<'_>) -> Option<Span> {
+    fn ctor_fields_span(&self, binding: NameBinding<'_>) -> Option<Span> {
         if let NameBindingKind::Res(Res::Def(
             DefKind::Ctor(CtorOf::Struct, CtorKind::Fn),
             ctor_def_id,
@@ -1622,7 +1622,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             if ctor_fields_span.is_some() { plain_descr + " constructor" } else { plain_descr };
         let import_descr = nonimport_descr.clone() + " import";
         let get_descr =
-            |b: &NameBinding<'_>| if b.is_import() { &import_descr } else { &nonimport_descr };
+            |b: NameBinding<'_>| if b.is_import() { &import_descr } else { &nonimport_descr };
 
         // Print the primary message.
         let descr = get_descr(binding);
@@ -1702,7 +1702,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 _ => None,
             };
 
-            let first = ptr::eq(binding, first_binding);
+            let first = binding == first_binding;
             let msg = format!(
                 "{and_refers_to}the {item} `{name}`{which} is defined here{dots}",
                 and_refers_to = if first { "" } else { "...and refers to " },
@@ -1762,7 +1762,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         opt_ns: Option<Namespace>, // `None` indicates a module path in import
         parent_scope: &ParentScope<'a>,
         ribs: Option<&PerNS<Vec<Rib<'a>>>>,
-        ignore_binding: Option<&'a NameBinding<'a>>,
+        ignore_binding: Option<NameBinding<'a>>,
         module: Option<ModuleOrUniformRoot<'a>>,
         failed_segment_idx: usize,
         ident: Ident,

compiler/rustc_resolve/src/effective_visibilities.rs

@@ -5,20 +5,17 @@ use rustc_ast::visit::Visitor;
 use rustc_ast::Crate;
 use rustc_ast::EnumDef;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_data_structures::intern::Interned;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::def_id::CRATE_DEF_ID;
 use rustc_middle::middle::privacy::Level;
 use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility};
 use rustc_middle::ty::Visibility;
 use std::mem;
 
-type ImportId<'a> = Interned<'a, NameBinding<'a>>;
-
 #[derive(Clone, Copy)]
 enum ParentId<'a> {
     Def(LocalDefId),
-    Import(ImportId<'a>),
+    Import(NameBinding<'a>),
 }
 
 impl ParentId<'_> {
@@ -36,7 +33,7 @@ pub(crate) struct EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
     /// While walking import chains we need to track effective visibilities per-binding, and def id
     /// keys in `Resolver::effective_visibilities` are not enough for that, because multiple
     /// bindings can correspond to a single def id in imports. So we keep a separate table.
-    import_effective_visibilities: EffectiveVisibilities<ImportId<'a>>,
+    import_effective_visibilities: EffectiveVisibilities<NameBinding<'a>>,
     // It's possible to recalculate this at any point, but it's relatively expensive.
     current_private_vis: Visibility,
     changed: bool,
@@ -47,7 +44,7 @@ impl Resolver<'_, '_> {
         self.get_nearest_non_block_module(def_id.to_def_id()).nearest_parent_mod().expect_local()
     }
 
-    fn private_vis_import(&mut self, binding: ImportId<'_>) -> Visibility {
+    fn private_vis_import(&mut self, binding: NameBinding<'_>) -> Visibility {
         let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() };
         Visibility::Restricted(
             import
@@ -75,7 +72,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
     pub(crate) fn compute_effective_visibilities<'c>(
         r: &'r mut Resolver<'a, 'tcx>,
         krate: &'c Crate,
-    ) -> FxHashSet<Interned<'a, NameBinding<'a>>> {
+    ) -> FxHashSet<NameBinding<'a>> {
         let mut visitor = EffectiveVisibilitiesVisitor {
             r,
             def_effective_visibilities: Default::default(),
@@ -133,8 +130,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
                 // lint. For all bindings added to the table this way `is_ambiguity` returns true.
                 let mut parent_id = ParentId::Def(module_id);
                 while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind {
-                    let binding_id = ImportId::new_unchecked(binding);
-                    self.update_import(binding_id, parent_id);
+                    self.update_import(binding, parent_id);
 
                     if binding.ambiguity.is_some() {
                         // Stop at the root ambiguity, further bindings in the chain should not
@@ -143,7 +139,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
                         break;
                     }
 
-                    parent_id = ParentId::Import(binding_id);
+                    parent_id = ParentId::Import(binding);
                     binding = nested_binding;
                 }
 
@@ -192,7 +188,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
         }
     }
 
-    fn update_import(&mut self, binding: ImportId<'a>, parent_id: ParentId<'a>) {
+    fn update_import(&mut self, binding: NameBinding<'a>, parent_id: ParentId<'a>) {
         let nominal_vis = binding.vis.expect_local();
         let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
         let inherited_eff_vis = self.effective_vis_or_private(parent_id);

compiler/rustc_resolve/src/ident.rs

@@ -284,7 +284,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         parent_scope: &ParentScope<'a>,
         finalize: Option<Finalize>,
         ribs: &[Rib<'a>],
-        ignore_binding: Option<&'a NameBinding<'a>>,
+        ignore_binding: Option<NameBinding<'a>>,
     ) -> Option<LexicalScopeBinding<'a>> {
         assert!(ns == TypeNS || ns == ValueNS);
         let orig_ident = ident;
@@ -378,8 +378,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         parent_scope: &ParentScope<'a>,
         finalize: Option<Finalize>,
         force: bool,
-        ignore_binding: Option<&'a NameBinding<'a>>,
-    ) -> Result<&'a NameBinding<'a>, Determinacy> {
+        ignore_binding: Option<NameBinding<'a>>,
+    ) -> Result<NameBinding<'a>, Determinacy> {
         bitflags::bitflags! {
             struct Flags: u8 {
                 const MACRO_RULES          = 1 << 0;
@@ -415,7 +415,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         // }
         // So we have to save the innermost solution and continue searching in outer scopes
         // to detect potential ambiguities.
-        let mut innermost_result: Option<(&NameBinding<'_>, Flags)> = None;
+        let mut innermost_result: Option<(NameBinding<'_>, Flags)> = None;
         let mut determinacy = Determinacy::Determined;
 
         // Go through all the scopes and try to resolve the name.
@@ -717,7 +717,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         ident: Ident,
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
-    ) -> Result<&'a NameBinding<'a>, Determinacy> {
+    ) -> Result<NameBinding<'a>, Determinacy> {
         self.resolve_ident_in_module_ext(module, ident, ns, parent_scope, None, None)
             .map_err(|(determinacy, _)| determinacy)
     }
@@ -730,8 +730,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
         finalize: Option<Finalize>,
-        ignore_binding: Option<&'a NameBinding<'a>>,
-    ) -> Result<&'a NameBinding<'a>, Determinacy> {
+        ignore_binding: Option<NameBinding<'a>>,
+    ) -> Result<NameBinding<'a>, Determinacy> {
         self.resolve_ident_in_module_ext(module, ident, ns, parent_scope, finalize, ignore_binding)
             .map_err(|(determinacy, _)| determinacy)
     }
@@ -744,8 +744,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
         finalize: Option<Finalize>,
-        ignore_binding: Option<&'a NameBinding<'a>>,
-    ) -> Result<&'a NameBinding<'a>, (Determinacy, Weak)> {
+        ignore_binding: Option<NameBinding<'a>>,
+    ) -> Result<NameBinding<'a>, (Determinacy, Weak)> {
         let tmp_parent_scope;
         let mut adjusted_parent_scope = parent_scope;
         match module {
@@ -782,8 +782,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
         finalize: Option<Finalize>,
-        ignore_binding: Option<&'a NameBinding<'a>>,
-    ) -> Result<&'a NameBinding<'a>, Determinacy> {
+        ignore_binding: Option<NameBinding<'a>>,
+    ) -> Result<NameBinding<'a>, Determinacy> {
         self.resolve_ident_in_module_unadjusted_ext(
             module,
             ident,
@@ -809,8 +809,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         finalize: Option<Finalize>,
         // This binding should be ignored during in-module resolution, so that we don't get
         // "self-confirming" import resolutions during import validation and checking.
-        ignore_binding: Option<&'a NameBinding<'a>>,
-    ) -> Result<&'a NameBinding<'a>, (Determinacy, Weak)> {
+        ignore_binding: Option<NameBinding<'a>>,
+    ) -> Result<NameBinding<'a>, (Determinacy, Weak)> {
         let module = match module {
             ModuleOrUniformRoot::Module(module) => module,
             ModuleOrUniformRoot::CrateRootAndExternPrelude => {
@@ -873,13 +873,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         // binding if it exists. What we really want here is having two separate scopes in
         // a module - one for non-globs and one for globs, but until that's done use this
         // hack to avoid inconsistent resolution ICEs during import validation.
-        let binding =
-            [resolution.binding, resolution.shadowed_glob].into_iter().find_map(|binding| {
-                match (binding, ignore_binding) {
-                    (Some(binding), Some(ignored)) if ptr::eq(binding, ignored) => None,
-                    _ => binding,
-                }
-            });
+        let binding = [resolution.binding, resolution.shadowed_glob]
+            .into_iter()
+            .find_map(|binding| if binding == ignore_binding { None } else { binding });
 
         if let Some(Finalize { path_span, report_private, .. }) = finalize {
             let Some(binding) = binding else {
@@ -930,7 +926,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             return Ok(binding);
         }
 
-        let check_usable = |this: &mut Self, binding: &'a NameBinding<'a>| {
+        let check_usable = |this: &mut Self, binding: NameBinding<'a>| {
             let usable = this.is_accessible_from(binding.vis, parent_scope.module);
             if usable { Ok(binding) } else { Err((Determined, Weak::No)) }
         };
@@ -1352,7 +1348,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         opt_ns: Option<Namespace>, // `None` indicates a module path in import
         parent_scope: &ParentScope<'a>,
         finalize: Option<Finalize>,
-        ignore_binding: Option<&'a NameBinding<'a>>,
+        ignore_binding: Option<NameBinding<'a>>,
     ) -> PathResult<'a> {
         self.resolve_path_with_ribs(path, opt_ns, parent_scope, finalize, None, ignore_binding)
     }
@@ -1364,7 +1360,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         parent_scope: &ParentScope<'a>,
         finalize: Option<Finalize>,
         ribs: Option<&PerNS<Vec<Rib<'a>>>>,
-        ignore_binding: Option<&'a NameBinding<'a>>,
+        ignore_binding: Option<NameBinding<'a>>,
     ) -> PathResult<'a> {
         let mut module = None;
         let mut allow_super = true;