Rollup merge of rust-lang#139584 - oli-obk:horrible-experiment-1, r=petrochenkov

Avoid a reverse map that is only used in diagnostics paths

r? `@petrochenkov`

iterating a map until a value matches and returning the key is bad obviously, but it happens very rarely and only on diagnostics paths. It would also be a lot cheaper with rust-lang#138995. Which is actually why I'm trying this out, that PR adds a new entry in `create_def`, which makes `create_def` show up in cachegrind. So I'm trying out if removing adding an entry in `create_def` is a perf improvement

Author: jhpratt

Cargo.lock

@@ -4397,7 +4397,6 @@ dependencies = [
  "rustc_feature",
  "rustc_fluent_macro",
  "rustc_hir",
- "rustc_index",
  "rustc_macros",
  "rustc_metadata",
  "rustc_middle",

compiler/rustc_data_structures/src/unord.rs

@@ -109,6 +109,16 @@ impl<T, I: Iterator<Item = T>> UnordItems<T, I> {
     pub fn collect<C: From<UnordItems<T, I>>>(self) -> C {
         self.into()
     }
+
+    /// If the iterator has only one element, returns it, otherwise returns `None`.
+    #[track_caller]
+    pub fn get_only(mut self) -> Option<T> {
+        let item = self.0.next();
+        if self.0.next().is_some() {
+            return None;
+        }
+        item
+    }
 }
 
 impl<T> UnordItems<T, std::iter::Empty<T>> {

compiler/rustc_resolve/Cargo.toml

@@ -18,7 +18,6 @@ rustc_expand = { path = "../rustc_expand" }
 rustc_feature = { path = "../rustc_feature" }
 rustc_fluent_macro = { path = "../rustc_fluent_macro" }
 rustc_hir = { path = "../rustc_hir" }
-rustc_index = { path = "../rustc_index" }
 rustc_macros = { path = "../rustc_macros" }
 rustc_metadata = { path = "../rustc_metadata" }
 rustc_middle = { path = "../rustc_middle" }

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -1207,7 +1207,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             for (rule_i, rule_span) in &self.r.macro_map[&def_id.to_def_id()].rule_spans {
                 self.r
                     .unused_macro_rules
-                    .entry(def_id)
+                    .entry(node_id)
                     .or_default()
                     .insert(*rule_i, (ident, *rule_span));
             }

compiler/rustc_resolve/src/diagnostics.rs

@@ -170,10 +170,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
     fn report_with_use_injections(&mut self, krate: &Crate) {
         for UseError { mut err, candidates, def_id, instead, suggestion, path, is_call } in
-            self.use_injections.drain(..)
+            std::mem::take(&mut self.use_injections)
         {
             let (span, found_use) = if let Some(def_id) = def_id.as_local() {
-                UsePlacementFinder::check(krate, self.def_id_to_node_id[def_id])
+                UsePlacementFinder::check(krate, self.def_id_to_node_id(def_id))
             } else {
                 (None, FoundUse::No)
             };
@@ -1435,7 +1435,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         let import_suggestions =
             self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
         let (span, found_use) = match parent_scope.module.nearest_parent_mod().as_local() {
-            Some(def_id) => UsePlacementFinder::check(krate, self.def_id_to_node_id[def_id]),
+            Some(def_id) => UsePlacementFinder::check(krate, self.def_id_to_node_id(def_id)),
             None => (None, FoundUse::No),
         };
         show_candidates(

compiler/rustc_resolve/src/imports.rs

@@ -639,38 +639,38 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 }
 
                 if let Some(glob_binding) = resolution.shadowed_glob {
-                    let binding_id = match binding.kind {
-                        NameBindingKind::Res(res) => {
-                            Some(self.def_id_to_node_id[res.def_id().expect_local()])
-                        }
-                        NameBindingKind::Module(module) => {
-                            Some(self.def_id_to_node_id[module.def_id().expect_local()])
-                        }
-                        NameBindingKind::Import { import, .. } => import.id(),
-                    };
-
                     if binding.res() != Res::Err
                         && glob_binding.res() != Res::Err
                         && let NameBindingKind::Import { import: glob_import, .. } =
                             glob_binding.kind
-                        && let Some(binding_id) = binding_id
                         && let Some(glob_import_id) = glob_import.id()
                         && let glob_import_def_id = self.local_def_id(glob_import_id)
                         && self.effective_visibilities.is_exported(glob_import_def_id)
                         && glob_binding.vis.is_public()
                         && !binding.vis.is_public()
                     {
-                        self.lint_buffer.buffer_lint(
-                            HIDDEN_GLOB_REEXPORTS,
-                            binding_id,
-                            binding.span,
-                            BuiltinLintDiag::HiddenGlobReexports {
-                                name: key.ident.name.to_string(),
-                                namespace: key.ns.descr().to_owned(),
-                                glob_reexport_span: glob_binding.span,
-                                private_item_span: binding.span,
-                            },
-                        );
+                        let binding_id = match binding.kind {
+                            NameBindingKind::Res(res) => {
+                                Some(self.def_id_to_node_id(res.def_id().expect_local()))
+                            }
+                            NameBindingKind::Module(module) => {
+                                Some(self.def_id_to_node_id(module.def_id().expect_local()))
+                            }
+                            NameBindingKind::Import { import, .. } => import.id(),
+                        };
+                        if let Some(binding_id) = binding_id {
+                            self.lint_buffer.buffer_lint(
+                                HIDDEN_GLOB_REEXPORTS,
+                                binding_id,
+                                binding.span,
+                                BuiltinLintDiag::HiddenGlobReexports {
+                                    name: key.ident.name.to_string(),
+                                    namespace: key.ns.descr().to_owned(),
+                                    glob_reexport_span: glob_binding.span,
+                                    private_item_span: binding.span,
+                                },
+                            );
+                        }
                     }
                 }
             }

compiler/rustc_resolve/src/late.rs

@@ -5007,8 +5007,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
             return false;
         }
         let Some(local_did) = did.as_local() else { return true };
-        let Some(node_id) = self.r.def_id_to_node_id.get(local_did) else { return true };
-        !self.r.proc_macros.contains(node_id)
+        !self.r.proc_macros.contains(&local_did)
     }
 
     fn resolve_doc_links(&mut self, attrs: &[Attribute], maybe_exported: MaybeExported<'_>) {

compiler/rustc_resolve/src/lib.rs

@@ -56,7 +56,6 @@ use rustc_hir::def::{
 };
 use rustc_hir::def_id::{CRATE_DEF_ID, CrateNum, DefId, LOCAL_CRATE, LocalDefId, LocalDefIdMap};
 use rustc_hir::{PrimTy, TraitCandidate};
-use rustc_index::IndexVec;
 use rustc_metadata::creader::{CStore, CrateLoader};
 use rustc_middle::metadata::ModChild;
 use rustc_middle::middle::privacy::EffectiveVisibilities;
@@ -1141,7 +1140,7 @@ pub struct Resolver<'ra, 'tcx> {
     ast_transform_scopes: FxHashMap<LocalExpnId, Module<'ra>>,
     unused_macros: FxIndexMap<LocalDefId, (NodeId, Ident)>,
     /// A map from the macro to all its potentially unused arms.
-    unused_macro_rules: FxIndexMap<LocalDefId, UnordMap<usize, (Ident, Span)>>,
+    unused_macro_rules: FxIndexMap<NodeId, UnordMap<usize, (Ident, Span)>>,
     proc_macro_stubs: FxHashSet<LocalDefId>,
     /// Traces collected during macro resolution and validated when it's complete.
     single_segment_macro_resolutions:
@@ -1184,7 +1183,6 @@ pub struct Resolver<'ra, 'tcx> {
     next_node_id: NodeId,
 
     node_id_to_def_id: NodeMap<Feed<'tcx, LocalDefId>>,
-    def_id_to_node_id: IndexVec<LocalDefId, ast::NodeId>,
 
     /// Indices of unnamed struct or variant fields with unresolved attributes.
     placeholder_field_indices: FxHashMap<NodeId, usize>,
@@ -1202,7 +1200,7 @@ pub struct Resolver<'ra, 'tcx> {
     trait_impls: FxIndexMap<DefId, Vec<LocalDefId>>,
     /// A list of proc macro LocalDefIds, written out in the order in which
     /// they are declared in the static array generated by proc_macro_harness.
-    proc_macros: Vec<NodeId>,
+    proc_macros: Vec<LocalDefId>,
     confused_type_with_std_module: FxIndexMap<Span, Span>,
     /// Whether lifetime elision was successful.
     lifetime_elision_allowed: FxHashSet<NodeId>,
@@ -1369,7 +1367,6 @@ impl<'tcx> Resolver<'_, 'tcx> {
             debug!("create_def: def_id_to_node_id[{:?}] <-> {:?}", def_id, node_id);
             self.node_id_to_def_id.insert(node_id, feed.downgrade());
         }
-        assert_eq!(self.def_id_to_node_id.push(node_id), def_id);
 
         feed
     }
@@ -1385,6 +1382,19 @@ impl<'tcx> Resolver<'_, 'tcx> {
     pub fn tcx(&self) -> TyCtxt<'tcx> {
         self.tcx
     }
+
+    /// This function is very slow, as it iterates over the entire
+    /// [Resolver::node_id_to_def_id] map just to find the [NodeId]
+    /// that corresponds to the given [LocalDefId]. Only use this in
+    /// diagnostics code paths.
+    fn def_id_to_node_id(&self, def_id: LocalDefId) -> NodeId {
+        self.node_id_to_def_id
+            .items()
+            .filter(|(_, v)| v.key() == def_id)
+            .map(|(k, _)| *k)
+            .get_only()
+            .unwrap()
+    }
 }
 
 impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
@@ -1417,8 +1427,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             &mut Default::default(),
         );
 
-        let mut def_id_to_node_id = IndexVec::default();
-        assert_eq!(def_id_to_node_id.push(CRATE_NODE_ID), CRATE_DEF_ID);
         let mut node_id_to_def_id = NodeMap::default();
         let crate_feed = tcx.create_local_crate_def_id(crate_span);
 
@@ -1553,7 +1561,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             lint_buffer: LintBuffer::default(),
             next_node_id: CRATE_NODE_ID,
             node_id_to_def_id,
-            def_id_to_node_id,
             placeholder_field_indices: Default::default(),
             invocation_parents,
             legacy_const_generic_args: Default::default(),
@@ -1633,7 +1640,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     }
 
     pub fn into_outputs(self) -> ResolverOutputs {
-        let proc_macros = self.proc_macros.iter().map(|id| self.local_def_id(*id)).collect();
+        let proc_macros = self.proc_macros;
         let expn_that_defined = self.expn_that_defined;
         let extern_crate_map = self.extern_crate_map;
         let maybe_unused_trait_imports = self.maybe_unused_trait_imports;

compiler/rustc_resolve/src/macros.rs

@@ -323,8 +323,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
     }
 
     fn record_macro_rule_usage(&mut self, id: NodeId, rule_i: usize) {
-        let did = self.local_def_id(id);
-        if let Some(rules) = self.unused_macro_rules.get_mut(&did) {
+        if let Some(rules) = self.unused_macro_rules.get_mut(&id) {
             rules.remove(&rule_i);
         }
     }
@@ -337,15 +336,12 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
                 ident.span,
                 BuiltinLintDiag::UnusedMacroDefinition(ident.name),
             );
+            // Do not report unused individual rules if the entire macro is unused
+            self.unused_macro_rules.swap_remove(&node_id);
         }
 
-        for (&def_id, unused_arms) in self.unused_macro_rules.iter() {
+        for (&node_id, unused_arms) in self.unused_macro_rules.iter() {
             for (&arm_i, &(ident, rule_span)) in unused_arms.to_sorted_stable_ord() {
-                if self.unused_macros.contains_key(&def_id) {
-                    // We already lint the entire macro as unused
-                    continue;
-                }
-                let node_id = self.def_id_to_node_id[def_id];
                 self.lint_buffer.buffer_lint(
                     UNUSED_MACRO_RULES,
                     node_id,
@@ -466,7 +462,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
     }
 
     fn declare_proc_macro(&mut self, id: NodeId) {
-        self.proc_macros.push(id)
+        self.proc_macros.push(self.local_def_id(id))
     }
 
     fn append_stripped_cfg_item(&mut self, parent_node: NodeId, ident: Ident, cfg: ast::MetaItem) {
@@ -932,7 +928,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                             .invocation_parents
                             .get(&parent_scope.expansion)
                             .map_or(ast::CRATE_NODE_ID, |parent| {
-                                self.def_id_to_node_id[parent.parent_def]
+                                self.def_id_to_node_id(parent.parent_def)
                             });
                         self.lint_buffer.buffer_lint(
                             LEGACY_DERIVE_HELPERS,