Skip to content

Commit 9b835f3

Browse files
Mark-SimulacrumMark-Simulacrum
authored
Rollup merge of #63149 - petrochenkov:lazypop2, r=eddyb
resolve: Populate external modules in more automatic and lazy way So, resolve had this function `populate_module_if_necessary` for loading module children from other crates from metadata. I never really understood when it should've been called and when not. This PR removes the function and loads the module children automatically on the first access instead. r? @eddyb
2 parents 6381937 + a087df6 commit 9b835f3

9 files changed

+149
-159
lines changed

‎src/librustc_resolve/build_reduced_graph.rs

+41-81
Original file line numberDiff line numberDiff line change
@@ -157,19 +157,6 @@ impl<'a> Resolver<'a> {
157157
self.macro_map.insert(def_id, ext.clone());
158158
Some(ext)
159159
}
160-
161-
/// Ensures that the reduced graph rooted at the given external module
162-
/// is built, building it if it is not.
163-
pub fn populate_module_if_necessary(&mut self, module: Module<'a>) {
164-
if module.populated.get() { return }
165-
let def_id = module.def_id().unwrap();
166-
for child in self.cstore.item_children_untracked(def_id, self.session) {
167-
let child = child.map_id(|_| panic!("unexpected id"));
168-
BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self }
169-
.build_reduced_graph_for_external_crate_res(module, child);
170-
}
171-
module.populated.set(true)
172-
}
173160
}
174161

175162
pub struct BuildReducedGraphVisitor<'a, 'b> {
@@ -595,7 +582,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
595582
self.r.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX })
596583
};
597584

598-
self.r.populate_module_if_necessary(module);
599585
if let Some(name) = self.r.session.parse_sess.injected_crate_name.try_get() {
600586
if name.as_str() == ident.name.as_str() {
601587
self.r.injected_crate = Some(module);
@@ -868,7 +854,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
868854
}
869855

870856
/// Builds the reduced graph for a single item in an external crate.
871-
fn build_reduced_graph_for_external_crate_res(
857+
crate fn build_reduced_graph_for_external_crate_res(
872858
&mut self,
873859
parent: Module<'a>,
874860
child: Export<ast::NodeId>,
@@ -879,88 +865,62 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
879865
// This is only a guess, two equivalent idents may incorrectly get different gensyms here.
880866
let ident = ident.gensym_if_underscore();
881867
let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene
868+
// Record primary definitions.
882869
match res {
883870
Res::Def(kind @ DefKind::Mod, def_id)
884-
| Res::Def(kind @ DefKind::Enum, def_id) => {
871+
| Res::Def(kind @ DefKind::Enum, def_id)
872+
| Res::Def(kind @ DefKind::Trait, def_id) => {
885873
let module = self.r.new_module(parent,
886874
ModuleKind::Def(kind, def_id, ident.name),
887875
def_id,
888876
expansion,
889877
span);
890-
self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion));
878+
self.r.define(parent, ident, TypeNS, (module, vis, span, expansion));
891879
}
892-
Res::Def(DefKind::Variant, _)
880+
Res::Def(DefKind::Struct, _)
881+
| Res::Def(DefKind::Union, _)
882+
| Res::Def(DefKind::Variant, _)
893883
| Res::Def(DefKind::TyAlias, _)
894884
| Res::Def(DefKind::ForeignTy, _)
895885
| Res::Def(DefKind::OpaqueTy, _)
896886
| Res::Def(DefKind::TraitAlias, _)
887+
| Res::Def(DefKind::AssocTy, _)
888+
| Res::Def(DefKind::AssocOpaqueTy, _)
897889
| Res::PrimTy(..)
898-
| Res::ToolMod => {
899-
self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
900-
}
890+
| Res::ToolMod =>
891+
self.r.define(parent, ident, TypeNS, (res, vis, span, expansion)),
901892
Res::Def(DefKind::Fn, _)
893+
| Res::Def(DefKind::Method, _)
902894
| Res::Def(DefKind::Static, _)
903895
| Res::Def(DefKind::Const, _)
904-
| Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => {
905-
self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
906-
}
907-
Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
908-
self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
909-
910-
if let Some(struct_def_id) =
911-
self.r.cstore.def_key(def_id).parent
912-
.map(|index| DefId { krate: def_id.krate, index: index }) {
913-
self.r.struct_constructors.insert(struct_def_id, (res, vis));
914-
}
915-
}
916-
Res::Def(DefKind::Trait, def_id) => {
917-
let module_kind = ModuleKind::Def(DefKind::Trait, def_id, ident.name);
918-
let module = self.r.new_module(parent,
919-
module_kind,
920-
parent.normal_ancestor_id,
921-
expansion,
922-
span);
923-
self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion));
924-
925-
for child in self.r.cstore.item_children_untracked(def_id, self.r.session) {
926-
let res = child.res.map_id(|_| panic!("unexpected id"));
927-
let ns = if let Res::Def(DefKind::AssocTy, _) = res {
928-
TypeNS
929-
} else { ValueNS };
930-
self.r.define(module, child.ident, ns,
931-
(res, ty::Visibility::Public, DUMMY_SP, expansion));
932-
933-
if self.r.cstore.associated_item_cloned_untracked(child.res.def_id())
934-
.method_has_self_argument {
935-
self.r.has_self.insert(res.def_id());
936-
}
937-
}
938-
module.populated.set(true);
939-
}
896+
| Res::Def(DefKind::AssocConst, _)
897+
| Res::Def(DefKind::Ctor(..), _) =>
898+
self.r.define(parent, ident, ValueNS, (res, vis, span, expansion)),
899+
Res::Def(DefKind::Macro(..), _)
900+
| Res::NonMacroAttr(..) =>
901+
self.r.define(parent, ident, MacroNS, (res, vis, span, expansion)),
902+
Res::Def(DefKind::TyParam, _) | Res::Def(DefKind::ConstParam, _)
903+
| Res::Local(..) | Res::SelfTy(..) | Res::SelfCtor(..) | Res::Err =>
904+
bug!("unexpected resolution: {:?}", res)
905+
}
906+
// Record some extra data for better diagnostics.
907+
match res {
940908
Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
941-
self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
942-
943-
// Record field names for error reporting.
944909
let field_names = self.r.cstore.struct_field_names_untracked(def_id);
945910
self.insert_field_names(def_id, field_names);
946911
}
947-
Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => {
948-
self.r.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion));
912+
Res::Def(DefKind::Method, def_id) => {
913+
if self.r.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument {
914+
self.r.has_self.insert(def_id);
915+
}
916+
}
917+
Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
918+
let parent = self.r.cstore.def_key(def_id).parent;
919+
if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) {
920+
self.r.struct_constructors.insert(struct_def_id, (res, vis));
921+
}
949922
}
950-
_ => bug!("unexpected resolution: {:?}", res)
951-
}
952-
}
953-
954-
fn legacy_import_macro(&mut self,
955-
name: Name,
956-
binding: &'a NameBinding<'a>,
957-
span: Span,
958-
allow_shadowing: bool) {
959-
if self.r.macro_use_prelude.insert(name, binding).is_some() && !allow_shadowing {
960-
let msg = format!("`{}` is already in scope", name);
961-
let note =
962-
"macro-expanded `#[macro_use]`s may not shadow existing macros (see RFC 1560)";
963-
self.r.session.struct_span_err(span, &msg).note(note).emit();
923+
_ => {}
964924
}
965925
}
966926

@@ -1021,9 +981,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
1021981
if let Some(span) = import_all {
1022982
let directive = macro_use_directive(self, span);
1023983
self.r.potentially_unused_imports.push(directive);
1024-
module.for_each_child(|ident, ns, binding| if ns == MacroNS {
1025-
let imported_binding = self.r.import(binding, directive);
1026-
self.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
984+
self.r.for_each_child(module, |this, ident, ns, binding| if ns == MacroNS {
985+
let imported_binding = this.import(binding, directive);
986+
this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
1027987
});
1028988
} else {
1029989
for ident in single_imports.iter().cloned() {
@@ -1039,8 +999,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
1039999
let directive = macro_use_directive(self, ident.span);
10401000
self.r.potentially_unused_imports.push(directive);
10411001
let imported_binding = self.r.import(binding, directive);
1042-
self.legacy_import_macro(ident.name, imported_binding,
1043-
ident.span, allow_shadowing);
1002+
self.r.legacy_import_macro(ident.name, imported_binding,
1003+
ident.span, allow_shadowing);
10441004
} else {
10451005
span_err!(self.r.session, ident.span, E0469, "imported macro not found");
10461006
}

‎src/librustc_resolve/diagnostics.rs

+20-21
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,23 @@ crate fn add_typo_suggestion(
7373
false
7474
}
7575

76-
crate fn add_module_candidates(
77-
module: Module<'_>, names: &mut Vec<TypoSuggestion>, filter_fn: &impl Fn(Res) -> bool
78-
) {
79-
for (&(ident, _), resolution) in module.resolutions.borrow().iter() {
80-
if let Some(binding) = resolution.borrow().binding {
81-
let res = binding.res();
82-
if filter_fn(res) {
83-
names.push(TypoSuggestion::from_res(ident.name, res));
76+
impl<'a> Resolver<'a> {
77+
crate fn add_module_candidates(
78+
&mut self,
79+
module: Module<'a>,
80+
names: &mut Vec<TypoSuggestion>,
81+
filter_fn: &impl Fn(Res) -> bool,
82+
) {
83+
for (&(ident, _), resolution) in self.resolutions(module).borrow().iter() {
84+
if let Some(binding) = resolution.borrow().binding {
85+
let res = binding.res();
86+
if filter_fn(res) {
87+
names.push(TypoSuggestion::from_res(ident.name, res));
88+
}
8489
}
8590
}
8691
}
87-
}
8892

89-
impl<'a> Resolver<'a> {
9093
/// Combines an error with provided span and emits it.
9194
///
9295
/// This takes the error provided, combines it with the span and any additional spans inside the
@@ -391,10 +394,10 @@ impl<'a> Resolver<'a> {
391394
Scope::CrateRoot => {
392395
let root_ident = Ident::new(kw::PathRoot, ident.span);
393396
let root_module = this.resolve_crate_root(root_ident);
394-
add_module_candidates(root_module, &mut suggestions, filter_fn);
397+
this.add_module_candidates(root_module, &mut suggestions, filter_fn);
395398
}
396399
Scope::Module(module) => {
397-
add_module_candidates(module, &mut suggestions, filter_fn);
400+
this.add_module_candidates(module, &mut suggestions, filter_fn);
398401
}
399402
Scope::MacroUsePrelude => {
400403
suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| {
@@ -442,7 +445,7 @@ impl<'a> Resolver<'a> {
442445
Scope::StdLibPrelude => {
443446
if let Some(prelude) = this.prelude {
444447
let mut tmp_suggestions = Vec::new();
445-
add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
448+
this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
446449
suggestions.extend(tmp_suggestions.into_iter().filter(|s| {
447450
use_prelude || this.is_builtin_macro(s.res.opt_def_id())
448451
}));
@@ -498,11 +501,9 @@ impl<'a> Resolver<'a> {
498501
while let Some((in_module,
499502
path_segments,
500503
in_module_is_extern)) = worklist.pop() {
501-
self.populate_module_if_necessary(in_module);
502-
503504
// We have to visit module children in deterministic order to avoid
504505
// instabilities in reported imports (#43552).
505-
in_module.for_each_child_stable(|ident, ns, name_binding| {
506+
self.for_each_child_stable(in_module, |this, ident, ns, name_binding| {
506507
// avoid imports entirely
507508
if name_binding.is_import() && !name_binding.is_extern_crate() { return; }
508509
// avoid non-importable candidates as well
@@ -536,7 +537,7 @@ impl<'a> Resolver<'a> {
536537
// outside crate private modules => no need to check this)
537538
if !in_module_is_extern || name_binding.vis == ty::Visibility::Public {
538539
let did = match res {
539-
Res::Def(DefKind::Ctor(..), did) => self.parent(did),
540+
Res::Def(DefKind::Ctor(..), did) => this.parent(did),
540541
_ => res.opt_def_id(),
541542
};
542543
candidates.push(ImportSuggestion { did, path });
@@ -596,8 +597,6 @@ impl<'a> Resolver<'a> {
596597
krate: crate_id,
597598
index: CRATE_DEF_INDEX,
598599
});
599-
self.populate_module_if_necessary(&crate_root);
600-
601600
suggestions.extend(self.lookup_import_candidates_from_module(
602601
lookup_ident, namespace, crate_root, ident, &filter_fn));
603602
}
@@ -794,7 +793,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
794793
/// at the root of the crate instead of the module where it is defined
795794
/// ```
796795
pub(crate) fn check_for_module_export_macro(
797-
&self,
796+
&mut self,
798797
directive: &'b ImportDirective<'b>,
799798
module: ModuleOrUniformRoot<'b>,
800799
ident: Ident,
@@ -815,7 +814,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
815814
return None;
816815
}
817816

818-
let resolutions = crate_module.resolutions.borrow();
817+
let resolutions = self.r.resolutions(crate_module).borrow();
819818
let resolution = resolutions.get(&(ident, MacroNS))?;
820819
let binding = resolution.borrow().binding()?;
821820
if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() {

‎src/librustc_resolve/late.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1934,7 +1934,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
19341934
let mut traits = module.traits.borrow_mut();
19351935
if traits.is_none() {
19361936
let mut collected_traits = Vec::new();
1937-
module.for_each_child(|name, ns, binding| {
1937+
self.r.for_each_child(module, |_, name, ns, binding| {
19381938
if ns != TypeNS { return }
19391939
match binding.res() {
19401940
Res::Def(DefKind::Trait, _) |

‎src/librustc_resolve/late/diagnostics.rs

+6-11
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use crate::{CrateLint, Module, ModuleKind, ModuleOrUniformRoot};
22
use crate::{PathResult, PathSource, Segment};
33
use crate::path_names_to_string;
4-
use crate::diagnostics::{add_typo_suggestion, add_module_candidates};
5-
use crate::diagnostics::{ImportSuggestion, TypoSuggestion};
4+
use crate::diagnostics::{add_typo_suggestion, ImportSuggestion, TypoSuggestion};
65
use crate::late::{LateResolutionVisitor, RibKind};
76

87
use errors::{Applicability, DiagnosticBuilder, DiagnosticId};
@@ -548,7 +547,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
548547
// Items in scope
549548
if let RibKind::ModuleRibKind(module) = rib.kind {
550549
// Items from this module
551-
add_module_candidates(module, &mut names, &filter_fn);
550+
self.r.add_module_candidates(module, &mut names, &filter_fn);
552551

553552
if let ModuleKind::Block(..) = module.kind {
554553
// We can see through blocks
@@ -577,7 +576,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
577576
}));
578577

579578
if let Some(prelude) = self.r.prelude {
580-
add_module_candidates(prelude, &mut names, &filter_fn);
579+
self.r.add_module_candidates(prelude, &mut names, &filter_fn);
581580
}
582581
}
583582
break;
@@ -599,7 +598,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
599598
mod_path, Some(TypeNS), false, span, CrateLint::No
600599
) {
601600
if let ModuleOrUniformRoot::Module(module) = module {
602-
add_module_candidates(module, &mut names, &filter_fn);
601+
self.r.add_module_candidates(module, &mut names, &filter_fn);
603602
}
604603
}
605604
}
@@ -717,9 +716,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
717716
// abort if the module is already found
718717
if result.is_some() { break; }
719718

720-
self.r.populate_module_if_necessary(in_module);
721-
722-
in_module.for_each_child_stable(|ident, _, name_binding| {
719+
self.r.for_each_child_stable(in_module, |_, ident, _, name_binding| {
723720
// abort if the module is already found or if name_binding is private external
724721
if result.is_some() || !name_binding.vis.is_visible_locally() {
725722
return
@@ -750,10 +747,8 @@ impl<'a> LateResolutionVisitor<'a, '_> {
750747

751748
fn collect_enum_variants(&mut self, def_id: DefId) -> Option<Vec<Path>> {
752749
self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| {
753-
self.r.populate_module_if_necessary(enum_module);
754-
755750
let mut variants = Vec::new();
756-
enum_module.for_each_child_stable(|ident, _, name_binding| {
751+
self.r.for_each_child_stable(enum_module, |_, ident, _, name_binding| {
757752
if let Res::Def(DefKind::Variant, _) = name_binding.res() {
758753
let mut segms = enum_import_suggestion.path.segments.clone();
759754
segms.push(ast::PathSegment::from_ident(ident));

0 commit comments

Comments
 (0)