Auto merge of rust-lang#110160 - petrochenkov:notagain2, r=cjgillot

bors · bors · commit 7a78c4ffd56f · 2023-04-14T01:14:36.000Z
resolve: Pre-compute non-reexport module children

Instead of repeating the same logic by walking HIR during metadata encoding.

The only difference is that we are no longer encoding `macro_rules` items, but we never currently need them as a part of this list. They can be encoded separately if this need ever arises.

`module_reexports` is also un-querified, because I don't see any reasons to make it a query, only overhead.
diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs
@@ -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;
                     }
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -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);
diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs
@@ -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>>,
diff --git a/compiler/rustc_middle/src/metadata.rs b/compiler/rustc_middle/src/metadata.rs
@@ -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]>,
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
@@ -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() }
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
@@ -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| {
diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
@@ -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>>,
diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs
@@ -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() {
diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -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
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -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() {
                     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);
             }
         }
     }
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -909,7 +909,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.
@@ -1259,7 +1260,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,
@@ -1386,7 +1388,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;
@@ -1398,7 +1399,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,
@@ -1949,20 +1951,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 {
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
@@ -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();
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -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())
         {
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
diff --git a/src/tools/clippy/tests/ui/macro_use_imports.fixed b/src/tools/clippy/tests/ui/macro_use_imports.fixed
diff --git a/src/tools/clippy/tests/ui/macro_use_imports.stderr b/src/tools/clippy/tests/ui/macro_use_imports.stderr

Original file line number	Diff line number	Diff line change
`@@ -515,7 +515,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {`
`515`	`515`	`let vis = self.tcx.local_visibility(item_id.owner_id.def_id);`
`516`	`516`	`self.update_macro_reachable_def(item_id.owner_id.def_id, def_kind, vis, defining_mod);`
`517`	`517`	`}`
`518`		`- for export in self.tcx.module_reexports(module_def_id) {`
	`518`	`+ for export in self.tcx.module_children_reexports(module_def_id) {`
`519`	`519`	`if export.vis.is_accessible_from(defining_mod, self.tcx)`
`520`	`520`	`&& let Res::Def(def_kind, def_id) = export.res`
`521`	`521`	`&& let Some(def_id) = def_id.as_local() {`