Skip to content

resolve: Pre-compute non-reexport module children #110160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 14, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Decoding metadata from a single crate's metadata

use crate::creader::{CStore, CrateMetadataRef};
use crate::rmeta::table::IsDefault;
use crate::rmeta::*;

use rustc_ast as ast;
@@ -995,17 +996,11 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

fn get_mod_child(self, id: DefIndex, sess: &Session) -> ModChild {
let ident = self.item_ident(id, sess);
let kind = self.def_kind(id);
let def_id = self.local_def_id(id);
let res = Res::Def(kind, def_id);
let res = Res::Def(self.def_kind(id), self.local_def_id(id));
let vis = self.get_visibility(id);
let span = self.get_span(id, sess);
let macro_rules = match kind {
DefKind::Macro(..) => self.root.tables.is_macro_rules.get(self, id),
_ => false,
};

ModChild { ident, res, vis, span, macro_rules, reexport_chain: Default::default() }
ModChild { ident, res, vis, span, reexport_chain: Default::default() }
}

/// Iterates over all named children of the given module,
@@ -1029,12 +1024,14 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
} else {
// Iterate over all children.
for child_index in self.root.tables.children.get(self, id).unwrap().decode(self) {
// FIXME: Do not encode RPITITs as a part of this list.
if self.root.tables.opt_rpitit_info.get(self, child_index).is_none() {
yield self.get_mod_child(child_index, sess);
}
}

if let Some(reexports) = self.root.tables.module_reexports.get(self, id) {
let reexports = self.root.tables.module_children_reexports.get(self, id);
if !reexports.is_default() {
for reexport in reexports.decode((self, sess)) {
yield reexport;
}
62 changes: 15 additions & 47 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
@@ -43,7 +43,6 @@ use std::borrow::Borrow;
use std::collections::hash_map::Entry;
use std::hash::Hash;
use std::io::{Read, Seek, Write};
use std::iter;
use std::num::NonZeroUsize;
use std::path::{Path, PathBuf};

@@ -456,7 +455,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}

fn encode_info_for_items(&mut self) {
self.encode_info_for_mod(CRATE_DEF_ID, self.tcx.hir().root_module());
self.encode_info_for_mod(CRATE_DEF_ID);

// Proc-macro crates only export proc-macro items, which are looked
// up using `proc_macro_data`
@@ -1324,7 +1323,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
record!(self.tables.implied_predicates_of[def_id] <- self.tcx.implied_predicates_of(def_id));
}
if let DefKind::Enum | DefKind::Struct | DefKind::Union = def_kind {
self.encode_info_for_adt(def_id);
self.encode_info_for_adt(local_id);
}
if tcx.impl_method_has_trait_impl_trait_tys(def_id)
&& let Ok(table) = self.tcx.collect_return_position_impl_trait_in_trait_tys(def_id)
@@ -1357,7 +1356,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}

#[instrument(level = "trace", skip(self))]
fn encode_info_for_adt(&mut self, def_id: DefId) {
fn encode_info_for_adt(&mut self, local_def_id: LocalDefId) {
let def_id = local_def_id.to_def_id();
let tcx = self.tcx;
let adt_def = tcx.adt_def(def_id);
record!(self.tables.repr_options[def_id] <- adt_def.repr());
@@ -1366,15 +1366,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
record!(self.tables.params_in_repr[def_id] <- params_in_repr);

if adt_def.is_enum() {
record_array!(self.tables.children[def_id] <- iter::from_generator(||
for variant in tcx.adt_def(def_id).variants() {
yield variant.def_id.index;
// Encode constructors which take a separate slot in value namespace.
if let Some(ctor_def_id) = variant.ctor_def_id() {
yield ctor_def_id.index;
}
}
));
let module_children = tcx.module_children_non_reexports(local_def_id);
record_array!(self.tables.children[def_id] <-
module_children.iter().map(|def_id| def_id.local_def_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);
@@ -1406,7 +1400,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}

fn encode_info_for_mod(&mut self, local_def_id: LocalDefId, md: &hir::Mod<'_>) {
fn encode_info_for_mod(&mut self, local_def_id: LocalDefId) {
let tcx = self.tcx;
let def_id = local_def_id.to_def_id();
debug!("EncodeContext::encode_info_for_mod({:?})", def_id);
@@ -1420,38 +1414,12 @@ 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 {
record_array!(self.tables.children[def_id] <- iter::from_generator(|| {
for item_id in md.item_ids {
match tcx.hir().item(*item_id).kind {
// Foreign items are planted into their parent modules
// from name resolution point of view.
hir::ItemKind::ForeignMod { items, .. } => {
for foreign_item in items {
yield foreign_item.id.owner_id.def_id.local_def_index;
}
}
// Only encode named non-reexport children, reexports are encoded
// separately and unnamed items are not used by name resolution.
hir::ItemKind::ExternCrate(..) => continue,
hir::ItemKind::Struct(ref vdata, _) => {
yield item_id.owner_id.def_id.local_def_index;
// Encode constructors which take a separate slot in value namespace.
if let Some(ctor_def_id) = vdata.ctor_def_id() {
yield ctor_def_id.local_def_index;
}
}
_ if tcx.def_key(item_id.owner_id.to_def_id()).get_opt_name().is_some() => {
yield item_id.owner_id.def_id.local_def_index;
}
_ => continue,
}
}
}));
let non_reexports = tcx.module_children_non_reexports(local_def_id);
record_array!(self.tables.children[def_id] <-
non_reexports.iter().map(|def_id| def_id.local_def_index));

let reexports = tcx.module_reexports(local_def_id);
if !reexports.is_empty() {
record_array!(self.tables.module_reexports[def_id] <- reexports);
}
record_defaulted_array!(self.tables.module_children_reexports[def_id] <-
tcx.module_children_reexports(local_def_id));
}
}

@@ -1668,8 +1636,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
self.tables.is_macro_rules.set(def_id.index, macro_def.macro_rules);
record!(self.tables.macro_definition[def_id] <- &*macro_def.body);
}
hir::ItemKind::Mod(ref m) => {
self.encode_info_for_mod(item.owner_id.def_id, m);
hir::ItemKind::Mod(..) => {
self.encode_info_for_mod(item.owner_id.def_id);
}
hir::ItemKind::OpaqueTy(ref opaque) => {
self.encode_explicit_item_bounds(def_id);
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
@@ -357,6 +357,7 @@ 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>,
module_children_reexports: Table<DefIndex, LazyArray<ModChild>>,

- optional:
attributes: Table<DefIndex, LazyArray<ast::Attribute>>,
@@ -414,7 +415,6 @@ define_tables! {
assoc_container: Table<DefIndex, ty::AssocItemContainer>,
macro_definition: Table<DefIndex, LazyValue<ast::DelimArgs>>,
proc_macro: Table<DefIndex, MacroKind>,
module_reexports: Table<DefIndex, LazyArray<ModChild>>,
deduced_param_attrs: Table<DefIndex, LazyArray<DeducedParamAttrs>>,
trait_impl_trait_tys: Table<DefIndex, LazyValue<FxHashMap<DefId, Ty<'static>>>>,
doc_link_resolutions: Table<DefIndex, LazyValue<DocLinkResMap>>,
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -43,8 +43,6 @@ pub struct ModChild {
pub vis: ty::Visibility<DefId>,
/// Span of the item.
pub span: Span,
/// A proper `macro_rules` item (not a reexport).
pub macro_rules: bool,
/// Reexport chain linking this module child to its original reexported item.
/// Empty if the module child is a proper item.
pub reexport_chain: SmallVec<[Reexport; 2]>,
4 changes: 0 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
@@ -1516,10 +1516,6 @@ rustc_queries! {
desc { "getting traits in scope at a block" }
}

query module_reexports(def_id: LocalDefId) -> &'tcx [ModChild] {
desc { |tcx| "looking up reexports of module `{}`", tcx.def_path_str(def_id.to_def_id()) }
}

query impl_defaultness(def_id: DefId) -> hir::Defaultness {
desc { |tcx| "looking up whether `{}` is a default impl", tcx.def_path_str(def_id) }
cache_on_disk_if { def_id.is_local() }
25 changes: 23 additions & 2 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ use crate::arena::Arena;
use crate::dep_graph::{DepGraph, DepKindStruct};
use crate::infer::canonical::CanonicalVarInfo;
use crate::lint::struct_lint_level;
use crate::metadata::ModChild;
use crate::middle::codegen_fn_attrs::CodegenFnAttrs;
use crate::middle::resolve_bound_vars;
use crate::middle::stability;
@@ -2459,6 +2460,28 @@ impl<'tcx> TyCtxt<'tcx> {
self.def_kind(def_id) == DefKind::ImplTraitPlaceholder
}
}

/// 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
/// 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[..])
}
}

impl<'tcx> TyCtxtAt<'tcx> {
@@ -2501,8 +2524,6 @@ pub struct DeducedParamAttrs {
}

pub fn provide(providers: &mut ty::query::Providers) {
providers.module_reexports =
|tcx, id| tcx.resolutions(()).reexport_map.get(&id).map_or(&[], |v| &v[..]);
providers.maybe_unused_trait_imports =
|tcx, ()| &tcx.resolutions(()).maybe_unused_trait_imports;
providers.names_imported_by_glob_use = |tcx, id| {
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -166,7 +166,8 @@ pub struct ResolverGlobalCtxt {
pub effective_visibilities: EffectiveVisibilities,
pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
pub maybe_unused_trait_imports: FxIndexSet<LocalDefId>,
pub reexport_map: FxHashMap<LocalDefId, Vec<ModChild>>,
pub module_children_non_reexports: LocalDefIdMap<Vec<LocalDefId>>,
pub module_children_reexports: LocalDefIdMap<Vec<ModChild>>,
pub glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
pub main_def: Option<MainDefinition>,
pub trait_impls: FxIndexMap<DefId, Vec<LocalDefId>>,
2 changes: 1 addition & 1 deletion compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
@@ -515,7 +515,7 @@ 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_reexports(module_def_id) {
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
&& let Some(def_id) = def_id.as_local() {
6 changes: 2 additions & 4 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
@@ -931,7 +931,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
/// Builds the reduced graph for a single item in an external crate.
fn build_reduced_graph_for_external_crate_res(&mut self, child: ModChild) {
let parent = self.parent_scope.module;
let ModChild { ident, res, vis, span, macro_rules, .. } = child;
let ModChild { ident, res, vis, span, .. } = child;
let res = res.expect_non_local();
let expansion = self.parent_scope.expansion;
// Record primary definitions.
@@ -964,9 +964,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
_,
) => self.r.define(parent, ident, ValueNS, (res, vis, span, expansion)),
Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => {
if !macro_rules {
self.r.define(parent, ident, MacroNS, (res, vis, span, expansion))
}
self.r.define(parent, ident, MacroNS, (res, vis, span, expansion))
}
Res::Def(
DefKind::TyParam
16 changes: 11 additions & 5 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
@@ -1261,10 +1261,14 @@ 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();

module.for_each_child(self, |this, ident, _, binding| {
if let Some(res) = this.is_reexport(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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Res::Err and ambiguities, should this closure just be a no-op?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, those are effectively imports that import nothing, at least from other crates' points of view.
(Although for ambiguities it's an open question - #36837.)

let mut reexport_chain = SmallVec::new();
let mut next_binding = binding;
while let NameBindingKind::Import { binding, import, .. } = next_binding.kind {
@@ -1277,16 +1281,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
res,
vis: binding.vis,
span: binding.span,
macro_rules: false,
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() {
// Call to `expect_local` should be fine because current
// code is only called for local modules.
self.reexport_map.insert(def_id.expect_local(), reexports);
self.module_children_reexports.insert(def_id, reexports);
}
}
}
24 changes: 6 additions & 18 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
@@ -910,7 +910,8 @@ pub struct Resolver<'a, 'tcx> {

/// `CrateNum` resolutions of `extern crate` items.
extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
reexport_map: FxHashMap<LocalDefId, Vec<ModChild>>,
module_children_non_reexports: LocalDefIdMap<Vec<LocalDefId>>,
module_children_reexports: LocalDefIdMap<Vec<ModChild>>,
trait_map: NodeMap<Vec<TraitCandidate>>,

/// A map from nodes to anonymous modules.
@@ -1260,7 +1261,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
lifetimes_res_map: Default::default(),
extra_lifetime_params_map: Default::default(),
extern_crate_map: Default::default(),
reexport_map: FxHashMap::default(),
module_children_non_reexports: Default::default(),
module_children_reexports: Default::default(),
trait_map: NodeMap::default(),
underscore_disambiguator: 0,
empty_module,
@@ -1387,7 +1389,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let visibilities = self.visibilities;
let has_pub_restricted = self.has_pub_restricted;
let extern_crate_map = self.extern_crate_map;
let reexport_map = self.reexport_map;
let maybe_unused_trait_imports = self.maybe_unused_trait_imports;
let glob_map = self.glob_map;
let main_def = self.main_def;
@@ -1399,7 +1400,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
has_pub_restricted,
effective_visibilities,
extern_crate_map,
reexport_map,
module_children_non_reexports: self.module_children_non_reexports,
module_children_reexports: self.module_children_reexports,
glob_map,
maybe_unused_trait_imports,
main_def,
@@ -1950,20 +1952,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
self.main_def = Some(MainDefinition { res, is_import, span });
}

// Items that go to reexport table encoded to metadata and visible through it to other crates.
fn is_reexport(&self, binding: &NameBinding<'a>) -> Option<def::Res<!>> {
if binding.is_import() {
let res = binding.res().expect_non_local();
// Ambiguous imports are treated as errors at this point and are
// not exposed to other crates (see #36837 for more details).
if res != def::Res::Err && !binding.is_ambiguity() {
return Some(res);
}
}

return None;
}
}

fn names_to_string(names: &[Symbol]) -> String {
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
@@ -152,7 +152,7 @@ pub(crate) fn try_inline_glob(
// reexported by the glob, e.g. because they are shadowed by something else.
let reexports = cx
.tcx
.module_reexports(current_mod)
.module_children_reexports(current_mod)
.iter()
.filter_map(|child| child.res.opt_def_id())
.collect();
2 changes: 1 addition & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
@@ -2062,7 +2062,7 @@ pub(crate) fn reexport_chain<'tcx>(
import_def_id: LocalDefId,
target_def_id: LocalDefId,
) -> &'tcx [Reexport] {
for child in tcx.module_reexports(tcx.local_parent(import_def_id)) {
for child in tcx.module_children_reexports(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())
{
Loading