From ef77dd232d7eed9d82b0719d7fc683924a3dc2de Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 27 Apr 2023 18:34:43 +0300 Subject: [PATCH] resolve: One more attempt to simplify `module_children` --- compiler/rustc_metadata/src/rmeta/encoder.rs | 16 ++++++++------- compiler/rustc_metadata/src/rmeta/mod.rs | 6 ++++++ compiler/rustc_middle/src/ty/context.rs | 21 ++++++-------------- compiler/rustc_middle/src/ty/mod.rs | 3 +-- compiler/rustc_privacy/src/lib.rs | 8 +++++--- compiler/rustc_resolve/src/imports.rs | 19 ++++++------------ compiler/rustc_resolve/src/lib.rs | 9 +++------ src/librustdoc/clean/inline.rs | 3 ++- src/librustdoc/clean/mod.rs | 4 ++-- src/librustdoc/visit_ast.rs | 9 +++++---- 10 files changed, 45 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index f5ffdd27cae3f..3253d0a905744 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/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); diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index dd02463e16a07..84f6b7f934dc3 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -357,10 +357,16 @@ define_tables! { associated_types_for_impl_traits_in_associated_fn: Table>, opt_rpitit_info: Table>>, unused_generic_params: Table, + // 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>, - optional: attributes: Table>, + // 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>, associated_item_or_field_def_ids: Table>, opt_def_kind: Table, diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index a309eaf048d2a..bf78b379986d8 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/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[..]) } } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index db6b35026a8e7..8986defacc7ba 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -165,8 +165,7 @@ pub struct ResolverGlobalCtxt { pub effective_visibilities: EffectiveVisibilities, pub extern_crate_map: FxHashMap, pub maybe_unused_trait_imports: FxIndexSet, - pub module_children_non_reexports: LocalDefIdMap>, - pub module_children_reexports: LocalDefIdMap>, + pub module_children: LocalDefIdMap>, pub glob_map: FxHashMap>, pub main_def: Option, pub trait_impls: FxIndexMap>, diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index c607c7fd5f4a7..7e60870fef0ff 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/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); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index d7c518fbdd0dc..1685468715f15 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -1261,14 +1261,11 @@ 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 { @@ -1276,17 +1273,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { 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); } } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 125f5ce761174..e46463579fe45 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/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, - module_children_non_reexports: LocalDefIdMap>, - module_children_reexports: LocalDefIdMap>, + module_children: LocalDefIdMap>, trait_map: NodeMap>, /// 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, diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 3f6a5d6d9017a..951f54e93663c 100644 --- a/src/librustdoc/clean/inline.rs +++ b/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)); diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 1531e7fc7b91d..f9a46e33f9a8c 100644 --- a/src/librustdoc/clean/mod.rs +++ b/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; } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index a6089680fae9d..841c7a78b2d4f 100644 --- a/src/librustdoc/visit_ast.rs +++ b/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)); } }