Skip to content

Commit ac89e03

Browse files
borsbors
committed
Auto merge of #63149 - petrochenkov:lazypop2, r=<try>
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 f690098 + 2245577 commit ac89e03

File tree

6 files changed

+113
-132
lines changed

6 files changed

+113
-132
lines changed

‎src/librustc_resolve/build_reduced_graph.rs

+29-57
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,6 @@ impl<'a> Resolver<'a> {
365365
self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX })
366366
};
367367

368-
self.populate_module_if_necessary(module);
369368
if let Some(name) = self.session.parse_sess.injected_crate_name.try_get() {
370369
if name.as_str() == ident.name.as_str() {
371370
self.injected_crate = Some(module);
@@ -632,7 +631,7 @@ impl<'a> Resolver<'a> {
632631
}
633632

634633
/// Builds the reduced graph for a single item in an external crate.
635-
fn build_reduced_graph_for_external_crate_res(
634+
crate fn build_reduced_graph_for_external_crate_res(
636635
&mut self,
637636
parent: Module<'a>,
638637
child: Export<ast::NodeId>,
@@ -645,68 +644,53 @@ impl<'a> Resolver<'a> {
645644
let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene
646645
match res {
647646
Res::Def(kind @ DefKind::Mod, def_id)
648-
| Res::Def(kind @ DefKind::Enum, def_id) => {
647+
| Res::Def(kind @ DefKind::Enum, def_id)
648+
| Res::Def(kind @ DefKind::Trait, def_id) => {
649649
let module = self.new_module(parent,
650650
ModuleKind::Def(kind, def_id, ident.name),
651651
def_id,
652652
expansion,
653653
span);
654654
self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion));
655655
}
656-
Res::Def(DefKind::Variant, _)
656+
Res::Def(DefKind::Struct, _)
657+
| Res::Def(DefKind::Union, _)
658+
| Res::Def(DefKind::Variant, _)
657659
| Res::Def(DefKind::TyAlias, _)
658660
| Res::Def(DefKind::ForeignTy, _)
659661
| Res::Def(DefKind::Existential, _)
660662
| Res::Def(DefKind::TraitAlias, _)
663+
| Res::Def(DefKind::AssocTy, _)
664+
| Res::Def(DefKind::AssocExistential, _)
661665
| Res::PrimTy(..)
662666
| Res::ToolMod => {
663667
self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
668+
669+
if let Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) = res {
670+
// Record field names for error reporting.
671+
let field_names = self.cstore.struct_field_names_untracked(def_id);
672+
self.insert_field_names(def_id, field_names);
673+
}
664674
}
665675
Res::Def(DefKind::Fn, _)
676+
| Res::Def(DefKind::Method, _)
666677
| Res::Def(DefKind::Static, _)
667678
| Res::Def(DefKind::Const, _)
668-
| Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => {
669-
self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
670-
}
671-
Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
679+
| Res::Def(DefKind::AssocConst, _)
680+
| Res::Def(DefKind::Ctor(..), _) => {
672681
self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
673682

674-
if let Some(struct_def_id) =
675-
self.cstore.def_key(def_id).parent
676-
.map(|index| DefId { krate: def_id.krate, index: index }) {
677-
self.struct_constructors.insert(struct_def_id, (res, vis));
678-
}
679-
}
680-
Res::Def(DefKind::Trait, def_id) => {
681-
let module_kind = ModuleKind::Def(DefKind::Trait, def_id, ident.name);
682-
let module = self.new_module(parent,
683-
module_kind,
684-
parent.normal_ancestor_id,
685-
expansion,
686-
span);
687-
self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion));
688-
689-
for child in self.cstore.item_children_untracked(def_id, self.session) {
690-
let res = child.res.map_id(|_| panic!("unexpected id"));
691-
let ns = if let Res::Def(DefKind::AssocTy, _) = res {
692-
TypeNS
693-
} else { ValueNS };
694-
self.define(module, child.ident, ns,
695-
(res, ty::Visibility::Public, DUMMY_SP, expansion));
696-
697-
if self.cstore.associated_item_cloned_untracked(child.res.def_id())
698-
.method_has_self_argument {
699-
self.has_self.insert(res.def_id());
683+
if let Res::Def(DefKind::Method, def_id) = res {
684+
let assoc_item = self.cstore.associated_item_cloned_untracked(def_id);
685+
if assoc_item.method_has_self_argument {
686+
self.has_self.insert(def_id);
687+
}
688+
} else if let Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) = res {
689+
let parent = self.cstore.def_key(def_id).parent;
690+
if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) {
691+
self.struct_constructors.insert(struct_def_id, (res, vis));
700692
}
701693
}
702-
module.populated.set(true);
703-
}
704-
Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
705-
self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
706-
707-
// Record field names for error reporting.
708-
let field_names = self.cstore.struct_field_names_untracked(def_id);
709-
self.insert_field_names(def_id, field_names);
710694
}
711695
Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => {
712696
self.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion));
@@ -780,18 +764,6 @@ impl<'a> Resolver<'a> {
780764
Some(ext)
781765
}
782766

783-
/// Ensures that the reduced graph rooted at the given external module
784-
/// is built, building it if it is not.
785-
pub fn populate_module_if_necessary(&mut self, module: Module<'a>) {
786-
if module.populated.get() { return }
787-
let def_id = module.def_id().unwrap();
788-
for child in self.cstore.item_children_untracked(def_id, self.session) {
789-
let child = child.map_id(|_| panic!("unexpected id"));
790-
self.build_reduced_graph_for_external_crate_res(module, child);
791-
}
792-
module.populated.set(true)
793-
}
794-
795767
fn legacy_import_macro(&mut self,
796768
name: Name,
797769
binding: &'a NameBinding<'a>,
@@ -863,9 +835,9 @@ impl<'a> Resolver<'a> {
863835
if let Some(span) = import_all {
864836
let directive = macro_use_directive(span);
865837
self.potentially_unused_imports.push(directive);
866-
module.for_each_child(|ident, ns, binding| if ns == MacroNS {
867-
let imported_binding = self.import(binding, directive);
868-
self.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
838+
self.for_each_child(module, |this, ident, ns, binding| if ns == MacroNS {
839+
let imported_binding = this.import(binding, directive);
840+
this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
869841
});
870842
} else {
871843
for ident in single_imports.iter().cloned() {

‎src/librustc_resolve/diagnostics.rs

+28-31
Original file line numberDiff line numberDiff line change
@@ -65,20 +65,23 @@ fn add_typo_suggestion(
6565
false
6666
}
6767

68-
fn add_module_candidates(
69-
module: Module<'_>, names: &mut Vec<TypoSuggestion>, filter_fn: &impl Fn(Res) -> bool
70-
) {
71-
for (&(ident, _), resolution) in module.resolutions.borrow().iter() {
72-
if let Some(binding) = resolution.borrow().binding {
73-
let res = binding.res();
74-
if filter_fn(res) {
75-
names.push(TypoSuggestion::from_res(ident.name, res));
68+
impl<'a> Resolver<'a> {
69+
fn add_module_candidates(
70+
&mut self,
71+
module: Module<'a>,
72+
names: &mut Vec<TypoSuggestion>,
73+
filter_fn: &impl Fn(Res) -> bool,
74+
) {
75+
for (&(ident, _), resolution) in self.resolutions(module).borrow().iter() {
76+
if let Some(binding) = resolution.borrow().binding {
77+
let res = binding.res();
78+
if filter_fn(res) {
79+
names.push(TypoSuggestion::from_res(ident.name, res));
80+
}
7681
}
7782
}
7883
}
79-
}
8084

81-
impl<'a> Resolver<'a> {
8285
/// Handles error reporting for `smart_resolve_path_fragment` function.
8386
/// Creates base error and amends it with one short label and possibly some longer helps/notes.
8487
pub(crate) fn smart_resolve_report_errors(
@@ -593,10 +596,10 @@ impl<'a> Resolver<'a> {
593596
Scope::CrateRoot => {
594597
let root_ident = Ident::new(kw::PathRoot, ident.span);
595598
let root_module = this.resolve_crate_root(root_ident);
596-
add_module_candidates(root_module, &mut suggestions, filter_fn);
599+
this.add_module_candidates(root_module, &mut suggestions, filter_fn);
597600
}
598601
Scope::Module(module) => {
599-
add_module_candidates(module, &mut suggestions, filter_fn);
602+
this.add_module_candidates(module, &mut suggestions, filter_fn);
600603
}
601604
Scope::MacroUsePrelude => {
602605
suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| {
@@ -644,7 +647,7 @@ impl<'a> Resolver<'a> {
644647
Scope::StdLibPrelude => {
645648
if let Some(prelude) = this.prelude {
646649
let mut tmp_suggestions = Vec::new();
647-
add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
650+
this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
648651
suggestions.extend(tmp_suggestions.into_iter().filter(|s| {
649652
use_prelude || this.is_builtin_macro(s.res.opt_def_id())
650653
}));
@@ -694,7 +697,9 @@ impl<'a> Resolver<'a> {
694697
if path.len() == 1 {
695698
// Search in lexical scope.
696699
// Walk backwards up the ribs in scope and collect candidates.
697-
for rib in self.ribs[ns].iter().rev() {
700+
// Ribs have to be cloned to avoid borrowing the resolver.
701+
let ribs = self.ribs[ns].clone();
702+
for rib in ribs.iter().rev() {
698703
// Locals and type parameters
699704
for (ident, &res) in &rib.bindings {
700705
if filter_fn(res) {
@@ -704,7 +709,7 @@ impl<'a> Resolver<'a> {
704709
// Items in scope
705710
if let RibKind::ModuleRibKind(module) = rib.kind {
706711
// Items from this module
707-
add_module_candidates(module, &mut names, &filter_fn);
712+
self.add_module_candidates(module, &mut names, &filter_fn);
708713

709714
if let ModuleKind::Block(..) = module.kind {
710715
// We can see through blocks
@@ -732,7 +737,7 @@ impl<'a> Resolver<'a> {
732737
}));
733738

734739
if let Some(prelude) = self.prelude {
735-
add_module_candidates(prelude, &mut names, &filter_fn);
740+
self.add_module_candidates(prelude, &mut names, &filter_fn);
736741
}
737742
}
738743
break;
@@ -754,7 +759,7 @@ impl<'a> Resolver<'a> {
754759
mod_path, Some(TypeNS), false, span, CrateLint::No
755760
) {
756761
if let ModuleOrUniformRoot::Module(module) = module {
757-
add_module_candidates(module, &mut names, &filter_fn);
762+
self.add_module_candidates(module, &mut names, &filter_fn);
758763
}
759764
}
760765
}
@@ -792,11 +797,9 @@ impl<'a> Resolver<'a> {
792797
while let Some((in_module,
793798
path_segments,
794799
in_module_is_extern)) = worklist.pop() {
795-
self.populate_module_if_necessary(in_module);
796-
797800
// We have to visit module children in deterministic order to avoid
798801
// instabilities in reported imports (#43552).
799-
in_module.for_each_child_stable(|ident, ns, name_binding| {
802+
self.for_each_child_stable(in_module, |this, ident, ns, name_binding| {
800803
// avoid imports entirely
801804
if name_binding.is_import() && !name_binding.is_extern_crate() { return; }
802805
// avoid non-importable candidates as well
@@ -830,7 +833,7 @@ impl<'a> Resolver<'a> {
830833
// outside crate private modules => no need to check this)
831834
if !in_module_is_extern || name_binding.vis == ty::Visibility::Public {
832835
let did = match res {
833-
Res::Def(DefKind::Ctor(..), did) => self.parent(did),
836+
Res::Def(DefKind::Ctor(..), did) => this.parent(did),
834837
_ => res.opt_def_id(),
835838
};
836839
candidates.push(ImportSuggestion { did, path });
@@ -890,8 +893,6 @@ impl<'a> Resolver<'a> {
890893
krate: crate_id,
891894
index: CRATE_DEF_INDEX,
892895
});
893-
self.populate_module_if_necessary(&crate_root);
894-
895896
suggestions.extend(self.lookup_import_candidates_from_module(
896897
lookup_ident, namespace, crate_root, ident, &filter_fn));
897898
}
@@ -910,9 +911,7 @@ impl<'a> Resolver<'a> {
910911
// abort if the module is already found
911912
if result.is_some() { break; }
912913

913-
self.populate_module_if_necessary(in_module);
914-
915-
in_module.for_each_child_stable(|ident, _, name_binding| {
914+
self.for_each_child_stable(in_module, |_, ident, _, name_binding| {
916915
// abort if the module is already found or if name_binding is private external
917916
if result.is_some() || !name_binding.vis.is_visible_locally() {
918917
return
@@ -943,10 +942,8 @@ impl<'a> Resolver<'a> {
943942

944943
fn collect_enum_variants(&mut self, def_id: DefId) -> Option<Vec<Path>> {
945944
self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| {
946-
self.populate_module_if_necessary(enum_module);
947-
948945
let mut variants = Vec::new();
949-
enum_module.for_each_child_stable(|ident, _, name_binding| {
946+
self.for_each_child_stable(enum_module, |_, ident, _, name_binding| {
950947
if let Res::Def(DefKind::Variant, _) = name_binding.res() {
951948
let mut segms = enum_import_suggestion.path.segments.clone();
952949
segms.push(ast::PathSegment::from_ident(ident));
@@ -1147,7 +1144,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
11471144
/// at the root of the crate instead of the module where it is defined
11481145
/// ```
11491146
pub(crate) fn check_for_module_export_macro(
1150-
&self,
1147+
&mut self,
11511148
directive: &'b ImportDirective<'b>,
11521149
module: ModuleOrUniformRoot<'b>,
11531150
ident: Ident,
@@ -1168,7 +1165,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
11681165
return None;
11691166
}
11701167

1171-
let resolutions = crate_module.resolutions.borrow();
1168+
let resolutions = self.resolutions(crate_module).borrow();
11721169
let resolution = resolutions.get(&(ident, MacroNS))?;
11731170
let binding = resolution.borrow().binding()?;
11741171
if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() {

0 commit comments

Comments
 (0)