Rollup merge of rust-lang#110908 - petrochenkov:notagain4, r=compiler-errors

resolve: One more attempt to simplify `module_children`

If the next step is performed and `fn module_children_local` is merged with the `module_children` query, then it causes perf regressions, regardless of whether query result feeding is [used](https://perf.rust-lang.org/compare.html?start=43a78029b4f4d92978b8fde0a677ea300b113c41&end=2eb5bcc5068b9d92f74bcb1797da664865d6981d&stat=instructions:u) or [not](https://perf.rust-lang.org/compare.html?start=2fce2290865f012391b8f3e581c3852a248031fa&end=2a33d6cd99481d1712037a79e7d66a8aefadbf72&stat=instructions:u).

Author: Dylan-DPC

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1364,9 +1364,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         record!(self.tables.params_in_repr[def_id] <- params_in_repr);
 
         if adt_def.is_enum() {
-            let module_children = tcx.module_children_non_reexports(local_def_id);
+            let module_children = tcx.module_children_local(local_def_id);
             record_array!(self.tables.module_children_non_reexports[def_id] <-
-                module_children.iter().map(|def_id| def_id.local_def_index));
+                module_children.iter().map(|child| child.res.def_id().index));
         } else {
             // For non-enum, there is only one variant, and its def_id is the adt's.
             debug_assert_eq!(adt_def.variants().len(), 1);
@@ -1412,12 +1412,14 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             // Encode this here because we don't do it in encode_def_ids.
             record!(self.tables.expn_that_defined[def_id] <- tcx.expn_that_defined(local_def_id));
         } else {
-            let non_reexports = tcx.module_children_non_reexports(local_def_id);
+            let module_children = tcx.module_children_local(local_def_id);
+
             record_array!(self.tables.module_children_non_reexports[def_id] <-
-                non_reexports.iter().map(|def_id| def_id.local_def_index));
+                module_children.iter().filter(|child| child.reexport_chain.is_empty())
+                    .map(|child| child.res.def_id().index));
 
             record_defaulted_array!(self.tables.module_children_reexports[def_id] <-
-                tcx.module_children_reexports(local_def_id));
+                module_children.iter().filter(|child| !child.reexport_chain.is_empty()));
         }
     }
 
@@ -1676,9 +1678,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             hir::ItemKind::Trait(..) => {
                 record!(self.tables.trait_def[def_id] <- self.tcx.trait_def(def_id));
 
-                let module_children = tcx.module_children_non_reexports(item.owner_id.def_id);
+                let module_children = tcx.module_children_local(item.owner_id.def_id);
                 record_array!(self.tables.module_children_non_reexports[def_id] <-
-                    module_children.iter().map(|def_id| def_id.local_def_index));
+                    module_children.iter().map(|child| child.res.def_id().index));
 
                 let associated_item_def_ids = self.tcx.associated_item_def_ids(def_id);
                 record_associated_item_def_ids(self, associated_item_def_ids);

compiler/rustc_metadata/src/rmeta/mod.rs

@@ -357,10 +357,16 @@ define_tables! {
     associated_types_for_impl_traits_in_associated_fn: Table<DefIndex, LazyArray<DefId>>,
     opt_rpitit_info: Table<DefIndex, Option<LazyValue<ty::ImplTraitInTraitData>>>,
     unused_generic_params: Table<DefIndex, UnusedGenericParams>,
+    // Reexported names are not associated with individual `DefId`s,
+    // e.g. a glob import can introduce a lot of names, all with the same `DefId`.
+    // That's why the encoded list needs to contain `ModChild` structures describing all the names
+    // individually instead of `DefId`s.
     module_children_reexports: Table<DefIndex, LazyArray<ModChild>>,
 
 - optional:
     attributes: Table<DefIndex, LazyArray<ast::Attribute>>,
+    // For non-reexported names in a module every name is associated with a separate `DefId`,
+    // so we can take their names, visibilities etc from other encoded tables.
     module_children_non_reexports: Table<DefIndex, LazyArray<DefIndex>>,
     associated_item_or_field_def_ids: Table<DefIndex, LazyArray<DefIndex>>,
     opt_def_kind: Table<DefIndex, DefKind>,

compiler/rustc_middle/src/ty/context.rs

@@ -2414,26 +2414,17 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
-    /// Named module children from all items except `use` and `extern crate` imports.
-    ///
-    /// In addition to regular items this list also includes struct or variant constructors, and
+    /// Named module children from all kinds of items, including imports.
+    /// In addition to regular items this list also includes struct and variant constructors, and
     /// items inside `extern {}` blocks because all of them introduce names into parent module.
-    /// For non-reexported children every such name is associated with a separate `DefId`.
     ///
     /// Module here is understood in name resolution sense - it can be a `mod` item,
     /// or a crate root, or an enum, or a trait.
-    pub fn module_children_non_reexports(self, def_id: LocalDefId) -> &'tcx [LocalDefId] {
-        self.resolutions(()).module_children_non_reexports.get(&def_id).map_or(&[], |v| &v[..])
-    }
-
-    /// Named module children from `use` and `extern crate` imports.
     ///
-    /// Reexported names are not associated with individual `DefId`s,
-    /// e.g. a glob import can introduce a lot of names, all with the same `DefId`.
-    /// That's why the list needs to contain `ModChild` structures describing all the names
-    /// individually instead of `DefId`s.
-    pub fn module_children_reexports(self, def_id: LocalDefId) -> &'tcx [ModChild] {
-        self.resolutions(()).module_children_reexports.get(&def_id).map_or(&[], |v| &v[..])
+    /// This is not a query, making it a query causes perf regressions
+    /// (probably due to hashing spans in `ModChild`ren).
+    pub fn module_children_local(self, def_id: LocalDefId) -> &'tcx [ModChild] {
+        self.resolutions(()).module_children.get(&def_id).map_or(&[], |v| &v[..])
     }
 }
 

compiler/rustc_middle/src/ty/mod.rs

@@ -165,8 +165,7 @@ pub struct ResolverGlobalCtxt {
     pub effective_visibilities: EffectiveVisibilities,
     pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
     pub maybe_unused_trait_imports: FxIndexSet<LocalDefId>,
-    pub module_children_non_reexports: LocalDefIdMap<Vec<LocalDefId>>,
-    pub module_children_reexports: LocalDefIdMap<Vec<ModChild>>,
+    pub module_children: LocalDefIdMap<Vec<ModChild>>,
     pub glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
     pub main_def: Option<MainDefinition>,
     pub trait_impls: FxIndexMap<DefId, Vec<LocalDefId>>,

compiler/rustc_privacy/src/lib.rs

@@ -515,9 +515,11 @@ impl<'tcx> EmbargoVisitor<'tcx> {
             let vis = self.tcx.local_visibility(item_id.owner_id.def_id);
             self.update_macro_reachable_def(item_id.owner_id.def_id, def_kind, vis, defining_mod);
         }
-        for export in self.tcx.module_children_reexports(module_def_id) {
-            if export.vis.is_accessible_from(defining_mod, self.tcx)
-                && let Res::Def(def_kind, def_id) = export.res
+        for child in self.tcx.module_children_local(module_def_id) {
+            // FIXME: Use module children for the logic above too.
+            if !child.reexport_chain.is_empty()
+                && child.vis.is_accessible_from(defining_mod, self.tcx)
+                && let Res::Def(def_kind, def_id) = child.res
                 && let Some(def_id) = def_id.as_local() {
                 let vis = self.tcx.local_visibility(def_id);
                 self.update_macro_reachable_def(def_id, def_kind, vis, defining_mod);

compiler/rustc_resolve/src/imports.rs

@@ -1261,32 +1261,25 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         *module.globs.borrow_mut() = Vec::new();
 
         if let Some(def_id) = module.opt_def_id() {
-            let mut non_reexports = Vec::new();
-            let mut reexports = Vec::new();
+            let mut children = Vec::new();
 
             module.for_each_child(self, |this, ident, _, binding| {
                 let res = binding.res().expect_non_local();
-                if !binding.is_import() {
-                    non_reexports.push(res.def_id().expect_local());
-                } else if res != def::Res::Err && !binding.is_ambiguity() {
+                if res != def::Res::Err && !binding.is_ambiguity() {
                     let mut reexport_chain = SmallVec::new();
                     let mut next_binding = binding;
                     while let NameBindingKind::Import { binding, import, .. } = next_binding.kind {
                         reexport_chain.push(import.simplify(this));
                         next_binding = binding;
                     }
 
-                    reexports.push(ModChild { ident, res, vis: binding.vis, reexport_chain });
+                    children.push(ModChild { ident, res, vis: binding.vis, reexport_chain });
                 }
             });
 
-            // Should be fine because this code is only called for local modules.
-            let def_id = def_id.expect_local();
-            if !non_reexports.is_empty() {
-                self.module_children_non_reexports.insert(def_id, non_reexports);
-            }
-            if !reexports.is_empty() {
-                self.module_children_reexports.insert(def_id, reexports);
+            if !children.is_empty() {
+                // Should be fine because this code is only called for local modules.
+                self.module_children.insert(def_id.expect_local(), children);
             }
         }
     }

compiler/rustc_resolve/src/lib.rs

@@ -909,8 +909,7 @@ pub struct Resolver<'a, 'tcx> {
 
     /// `CrateNum` resolutions of `extern crate` items.
     extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
-    module_children_non_reexports: LocalDefIdMap<Vec<LocalDefId>>,
-    module_children_reexports: LocalDefIdMap<Vec<ModChild>>,
+    module_children: LocalDefIdMap<Vec<ModChild>>,
     trait_map: NodeMap<Vec<TraitCandidate>>,
 
     /// A map from nodes to anonymous modules.
@@ -1260,8 +1259,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             lifetimes_res_map: Default::default(),
             extra_lifetime_params_map: Default::default(),
             extern_crate_map: Default::default(),
-            module_children_non_reexports: Default::default(),
-            module_children_reexports: Default::default(),
+            module_children: Default::default(),
             trait_map: NodeMap::default(),
             underscore_disambiguator: 0,
             empty_module,
@@ -1399,8 +1397,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             has_pub_restricted,
             effective_visibilities,
             extern_crate_map,
-            module_children_non_reexports: self.module_children_non_reexports,
-            module_children_reexports: self.module_children_reexports,
+            module_children: self.module_children,
             glob_map,
             maybe_unused_trait_imports,
             main_def,

src/librustdoc/clean/inline.rs

@@ -152,8 +152,9 @@ pub(crate) fn try_inline_glob(
             // reexported by the glob, e.g. because they are shadowed by something else.
             let reexports = cx
                 .tcx
-                .module_children_reexports(current_mod)
+                .module_children_local(current_mod)
                 .iter()
+                .filter(|child| !child.reexport_chain.is_empty())
                 .filter_map(|child| child.res.opt_def_id())
                 .collect();
             let mut items = build_module_items(cx, did, visited, inlined_names, Some(&reexports));

src/librustdoc/clean/mod.rs

@@ -2089,9 +2089,9 @@ pub(crate) fn reexport_chain<'tcx>(
     import_def_id: LocalDefId,
     target_def_id: LocalDefId,
 ) -> &'tcx [Reexport] {
-    for child in tcx.module_children_reexports(tcx.local_parent(import_def_id)) {
+    for child in tcx.module_children_local(tcx.local_parent(import_def_id)) {
         if child.res.opt_def_id() == Some(target_def_id.to_def_id())
-            && child.reexport_chain[0].id() == Some(import_def_id.to_def_id())
+            && child.reexport_chain.first().and_then(|r| r.id()) == Some(import_def_id.to_def_id())
         {
             return &child.reexport_chain;
         }

src/librustdoc/visit_ast.rs

@@ -136,14 +136,15 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         // is declared but also a reexport of itself producing two exports of the same
         // macro in the same module.
         let mut inserted = FxHashSet::default();
-        for export in self.cx.tcx.module_children_reexports(CRATE_DEF_ID) {
-            if let Res::Def(DefKind::Macro(_), def_id) = export.res &&
+        for child in self.cx.tcx.module_children_local(CRATE_DEF_ID) {
+            if !child.reexport_chain.is_empty() &&
+                let Res::Def(DefKind::Macro(_), def_id) = child.res &&
                 let Some(local_def_id) = def_id.as_local() &&
                 self.cx.tcx.has_attr(def_id, sym::macro_export) &&
                 inserted.insert(def_id)
             {
-                    let item = self.cx.tcx.hir().expect_item(local_def_id);
-                    top_level_module.items.insert((local_def_id, Some(item.ident.name)), (item, None, None));
+                let item = self.cx.tcx.hir().expect_item(local_def_id);
+                top_level_module.items.insert((local_def_id, Some(item.ident.name)), (item, None, None));
             }
         }