From ea81d8cedb68076cb194204f86a0ffb1204be8f9 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 29 Jul 2019 21:19:50 +0300 Subject: [PATCH 1/4] resolve: Populate external modules in more automatic and lazy way The modules are now populated implicitly on the first access --- src/librustc_resolve/build_reduced_graph.rs | 29 +++++----------- src/librustc_resolve/diagnostics.rs | 27 +++++++-------- src/librustc_resolve/late.rs | 2 +- src/librustc_resolve/late/diagnostics.rs | 14 +++----- src/librustc_resolve/lib.rs | 38 +++++++++++++-------- src/librustc_resolve/resolve_imports.rs | 34 +++++++++++------- 6 files changed, 73 insertions(+), 71 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 539e4a301e012..852353b7c1f01 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -159,19 +159,6 @@ impl<'a> Resolver<'a> { Some(ext) } - /// Ensures that the reduced graph rooted at the given external module - /// is built, building it if it is not. - crate fn populate_module_if_necessary(&mut self, module: Module<'a>) { - if module.populated.get() { return } - let def_id = module.def_id().unwrap(); - for child in self.cstore.item_children_untracked(def_id, self.session) { - let child = child.map_id(|_| panic!("unexpected id")); - BuildReducedGraphVisitor { parent_scope: ParentScope::module(module), r: self } - .build_reduced_graph_for_external_crate_res(child); - } - module.populated.set(true) - } - crate fn build_reduced_graph( &mut self, fragment: &AstFragment, parent_scope: ParentScope<'a> ) -> LegacyScope<'a> { @@ -186,6 +173,10 @@ struct BuildReducedGraphVisitor<'a, 'b> { parent_scope: ParentScope<'a>, } +impl<'a> AsMut> for BuildReducedGraphVisitor<'a, '_> { + fn as_mut(&mut self) -> &mut Resolver<'a> { self.r } +} + impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility { let parent_scope = &self.parent_scope; @@ -603,8 +594,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX }) }; - self.r.populate_module_if_necessary(module); - let used = self.process_legacy_macro_imports(item, module); let binding = (module, ty::Visibility::Public, sp, expansion).to_name_binding(self.r.arenas); @@ -922,6 +911,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { span); self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + module.populate_on_access.set(false); for child in self.r.cstore.item_children_untracked(def_id, self.r.session) { let res = child.res.map_id(|_| panic!("unexpected id")); let ns = if let Res::Def(DefKind::AssocTy, _) = res { @@ -935,7 +925,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.has_self.insert(res.def_id()); } } - module.populated.set(true); } Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); @@ -952,7 +941,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } fn legacy_import_macro(&mut self, - name: Name, + name: ast::Name, binding: &'a NameBinding<'a>, span: Span, allow_shadowing: bool) { @@ -1021,9 +1010,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { if let Some(span) = import_all { let directive = macro_use_directive(self, span); self.r.potentially_unused_imports.push(directive); - module.for_each_child(|ident, ns, binding| if ns == MacroNS { - let imported_binding = self.r.import(binding, directive); - self.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing); + module.for_each_child(self, |this, ident, ns, binding| if ns == MacroNS { + let imported_binding = this.r.import(binding, directive); + this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing); }); } else { for ident in single_imports.iter().cloned() { diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index 861b0fd44ac41..80d1322ff3176 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -73,10 +73,13 @@ crate fn add_typo_suggestion( false } -crate fn add_module_candidates( - module: Module<'_>, names: &mut Vec, filter_fn: &impl Fn(Res) -> bool +crate fn add_module_candidates<'a>( + resolver: &mut Resolver<'a>, + module: Module<'a>, + names: &mut Vec, + filter_fn: &impl Fn(Res) -> bool, ) { - for (&(ident, _), resolution) in module.resolutions.borrow().iter() { + for (&(ident, _), resolution) in resolver.resolutions(module).borrow().iter() { if let Some(binding) = resolution.borrow().binding { let res = binding.res(); if filter_fn(res) { @@ -402,10 +405,10 @@ impl<'a> Resolver<'a> { Scope::CrateRoot => { let root_ident = Ident::new(kw::PathRoot, ident.span); let root_module = this.resolve_crate_root(root_ident); - add_module_candidates(root_module, &mut suggestions, filter_fn); + add_module_candidates(this, root_module, &mut suggestions, filter_fn); } Scope::Module(module) => { - add_module_candidates(module, &mut suggestions, filter_fn); + add_module_candidates(this, module, &mut suggestions, filter_fn); } Scope::MacroUsePrelude => { suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| { @@ -453,7 +456,7 @@ impl<'a> Resolver<'a> { Scope::StdLibPrelude => { if let Some(prelude) = this.prelude { let mut tmp_suggestions = Vec::new(); - add_module_candidates(prelude, &mut tmp_suggestions, filter_fn); + add_module_candidates(this, prelude, &mut tmp_suggestions, filter_fn); suggestions.extend(tmp_suggestions.into_iter().filter(|s| { use_prelude || this.is_builtin_macro(s.res) })); @@ -509,11 +512,9 @@ impl<'a> Resolver<'a> { while let Some((in_module, path_segments, in_module_is_extern)) = worklist.pop() { - self.populate_module_if_necessary(in_module); - // We have to visit module children in deterministic order to avoid // instabilities in reported imports (#43552). - in_module.for_each_child_stable(|ident, ns, name_binding| { + in_module.for_each_child_stable(self, |this, ident, ns, name_binding| { // avoid imports entirely if name_binding.is_import() && !name_binding.is_extern_crate() { return; } // avoid non-importable candidates as well @@ -547,7 +548,7 @@ impl<'a> Resolver<'a> { // outside crate private modules => no need to check this) if !in_module_is_extern || name_binding.vis == ty::Visibility::Public { let did = match res { - Res::Def(DefKind::Ctor(..), did) => self.parent(did), + Res::Def(DefKind::Ctor(..), did) => this.parent(did), _ => res.opt_def_id(), }; candidates.push(ImportSuggestion { did, path }); @@ -607,8 +608,6 @@ impl<'a> Resolver<'a> { krate: crate_id, index: CRATE_DEF_INDEX, }); - self.populate_module_if_necessary(&crate_root); - suggestions.extend(self.lookup_import_candidates_from_module( lookup_ident, namespace, crate_root, ident, &filter_fn)); } @@ -805,7 +804,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { /// at the root of the crate instead of the module where it is defined /// ``` pub(crate) fn check_for_module_export_macro( - &self, + &mut self, directive: &'b ImportDirective<'b>, module: ModuleOrUniformRoot<'b>, ident: Ident, @@ -826,7 +825,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { return None; } - let resolutions = crate_module.resolutions.borrow(); + let resolutions = self.r.resolutions(crate_module).borrow(); let resolution = resolutions.get(&(ident, MacroNS))?; let binding = resolution.borrow().binding()?; if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() { diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index ffdfd85002bca..d8bd86699b7af 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -1929,7 +1929,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { let mut traits = module.traits.borrow_mut(); if traits.is_none() { let mut collected_traits = Vec::new(); - module.for_each_child(|name, ns, binding| { + module.for_each_child(self.r, |_, name, ns, binding| { if ns != TypeNS { return } match binding.res() { Res::Def(DefKind::Trait, _) | diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 68f9c1684d6fb..960bde3180915 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -548,7 +548,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // Items in scope if let RibKind::ModuleRibKind(module) = rib.kind { // Items from this module - add_module_candidates(module, &mut names, &filter_fn); + add_module_candidates(self.r, module, &mut names, &filter_fn); if let ModuleKind::Block(..) = module.kind { // We can see through blocks @@ -577,7 +577,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { })); if let Some(prelude) = self.r.prelude { - add_module_candidates(prelude, &mut names, &filter_fn); + add_module_candidates(self.r, prelude, &mut names, &filter_fn); } } break; @@ -599,7 +599,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { mod_path, Some(TypeNS), false, span, CrateLint::No ) { if let ModuleOrUniformRoot::Module(module) = module { - add_module_candidates(module, &mut names, &filter_fn); + add_module_candidates(self.r, module, &mut names, &filter_fn); } } } @@ -717,9 +717,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // abort if the module is already found if result.is_some() { break; } - self.r.populate_module_if_necessary(in_module); - - in_module.for_each_child_stable(|ident, _, name_binding| { + in_module.for_each_child_stable(self.r, |_, ident, _, name_binding| { // abort if the module is already found or if name_binding is private external if result.is_some() || !name_binding.vis.is_visible_locally() { return @@ -750,10 +748,8 @@ impl<'a> LateResolutionVisitor<'a, '_> { fn collect_enum_variants(&mut self, def_id: DefId) -> Option> { self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| { - self.r.populate_module_if_necessary(enum_module); - let mut variants = Vec::new(); - enum_module.for_each_child_stable(|ident, _, name_binding| { + enum_module.for_each_child_stable(self.r, |_, ident, _, name_binding| { if let Res::Def(DefKind::Variant, _) = name_binding.res() { let mut segms = enum_import_suggestion.path.segments.clone(); segms.push(ast::PathSegment::from_ident(ident)); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 9b5eb51eb582d..bf51ba212ff07 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -431,6 +431,8 @@ impl ModuleKind { } } +type Resolutions<'a> = RefCell>>>; + /// One node in the tree of modules. pub struct ModuleData<'a> { parent: Option>, @@ -439,7 +441,11 @@ pub struct ModuleData<'a> { // The def id of the closest normal module (`mod`) ancestor (including this module). normal_ancestor_id: DefId, - resolutions: RefCell>>>, + // Mapping between names and their (possibly in-progress) resolutions in this module. + // Resolutions in modules from other crates are not populated until accessed. + lazy_resolutions: Resolutions<'a>, + // True if this is a module from other crate that needs to be populated on access. + populate_on_access: Cell, // Macro invocations that can expand into items in this module. unresolved_invocations: RefCell>, @@ -452,11 +458,6 @@ pub struct ModuleData<'a> { // Used to memoize the traits in this module for faster searches through all traits in scope. traits: RefCell)]>>>, - // Whether this module is populated. If not populated, any attempt to - // access the children must be preceded with a - // `populate_module_if_necessary` call. - populated: Cell, - /// Span of the module itself. Used for error reporting. span: Span, @@ -475,30 +476,34 @@ impl<'a> ModuleData<'a> { parent, kind, normal_ancestor_id, - resolutions: Default::default(), + lazy_resolutions: Default::default(), + populate_on_access: Cell::new(!normal_ancestor_id.is_local()), unresolved_invocations: Default::default(), no_implicit_prelude: false, glob_importers: RefCell::new(Vec::new()), globs: RefCell::new(Vec::new()), traits: RefCell::new(None), - populated: Cell::new(normal_ancestor_id.is_local()), span, expansion, } } - fn for_each_child)>(&self, mut f: F) { - for (&(ident, ns), name_resolution) in self.resolutions.borrow().iter() { - name_resolution.borrow().binding.map(|binding| f(ident, ns, binding)); + fn for_each_child(&'a self, resolver: &mut R, mut f: F) + where R: AsMut>, F: FnMut(&mut R, Ident, Namespace, &'a NameBinding<'a>) + { + for (&(ident, ns), name_resolution) in resolver.as_mut().resolutions(self).borrow().iter() { + name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding)); } } - fn for_each_child_stable)>(&self, mut f: F) { - let resolutions = self.resolutions.borrow(); + fn for_each_child_stable(&'a self, resolver: &mut R, mut f: F) + where R: AsMut>, F: FnMut(&mut R, Ident, Namespace, &'a NameBinding<'a>) + { + let resolutions = resolver.as_mut().resolutions(self).borrow(); let mut resolutions = resolutions.iter().collect::>(); resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns)); for &(&(ident, ns), &resolution) in resolutions.iter() { - resolution.borrow().binding.map(|binding| f(ident, ns, binding)); + resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding)); } } @@ -983,6 +988,10 @@ impl<'a> ResolverArenas<'a> { } } +impl<'a> AsMut> for Resolver<'a> { + fn as_mut(&mut self) -> &mut Resolver<'a> { self } +} + impl<'a, 'b> ty::DefIdTree for &'a Resolver<'b> { fn parent(self, id: DefId) -> Option { match id.krate { @@ -2634,7 +2643,6 @@ impl<'a> Resolver<'a> { return None; }; let crate_root = self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX }); - self.populate_module_if_necessary(&crate_root); Some((crate_root, ty::Visibility::Public, DUMMY_SP, ExpnId::root()) .to_name_binding(self.arenas)) } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 693893e9ef13b..d68dbb78e9d00 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -7,9 +7,10 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, PerNS, ScopeSet, ParentScope use crate::Determinacy::{self, *}; use crate::Namespace::{self, TypeNS, MacroNS}; use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError}; -use crate::{Resolver, ResolutionError, Segment}; +use crate::{Resolutions, Resolver, ResolutionError, Segment}; use crate::{names_to_string, module_to_string}; use crate::ModuleKind; +use crate::build_reduced_graph::BuildReducedGraphVisitor; use crate::diagnostics::Suggestion; use errors::Applicability; @@ -161,9 +162,22 @@ impl<'a> NameResolution<'a> { } impl<'a> Resolver<'a> { - crate fn resolution(&self, module: Module<'a>, ident: Ident, ns: Namespace) + crate fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> { + if module.populate_on_access.get() { + module.populate_on_access.set(false); + let def_id = module.def_id().expect("unpopulated module without a def-id"); + for child in self.cstore.item_children_untracked(def_id, self.session) { + let child = child.map_id(|_| panic!("unexpected id")); + BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } + .build_reduced_graph_for_external_crate_res(module, child); + } + } + &module.lazy_resolutions + } + + crate fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace) -> &'a RefCell> { - *module.resolutions.borrow_mut().entry((ident.modern(), ns)) + *self.resolutions(module).borrow_mut().entry((ident.modern(), ns)) .or_insert_with(|| self.arenas.alloc_name_resolution()) } @@ -242,8 +256,6 @@ impl<'a> Resolver<'a> { } }; - self.populate_module_if_necessary(module); - let resolution = self.resolution(module, ident, ns) .try_borrow_mut() .map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports. @@ -1027,7 +1039,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { return if all_ns_failed { let resolutions = match module { - ModuleOrUniformRoot::Module(module) => Some(module.resolutions.borrow()), + ModuleOrUniformRoot::Module(module) => Some(self.r.resolutions(module).borrow()), _ => None, }; let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); @@ -1265,8 +1277,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } }; - self.r.populate_module_if_necessary(module); - if module.is_trait() { self.r.session.span_err(directive.span, "items in traits are not importable."); return; @@ -1282,7 +1292,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { // Ensure that `resolutions` isn't borrowed during `try_define`, // since it might get updated via a glob cycle. - let bindings = module.resolutions.borrow().iter().filter_map(|(&ident, resolution)| { + let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(&ident, resolution)| { resolution.borrow().binding().map(|binding| (ident, binding)) }).collect::>(); for ((mut ident, ns), binding) in bindings { @@ -1310,7 +1320,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { let mut reexports = Vec::new(); - for (&(ident, ns), resolution) in module.resolutions.borrow().iter() { + for (&(ident, ns), resolution) in self.r.resolutions(module).borrow().iter() { let resolution = &mut *resolution.borrow_mut(); let binding = match resolution.binding { Some(binding) => binding, @@ -1369,8 +1379,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> { Some(ModuleOrUniformRoot::Module(module)) => module, _ => bug!("module should exist"), }; - let resolutions = imported_module.parent.expect("parent should exist") - .resolutions.borrow(); + let parent_module = imported_module.parent.expect("parent should exist"); + let resolutions = self.r.resolutions(parent_module).borrow(); let enum_path_segment_index = directive.module_path.len() - 1; let enum_ident = directive.module_path[enum_path_segment_index].ident; From fe2524cb9cf09e7a0aa84bbb36a130e65255e1c0 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 16 Aug 2019 01:15:36 +0300 Subject: [PATCH 2/4] resolve: Populate external traits lazily as well --- src/librustc_resolve/build_reduced_graph.rs | 31 ++++++--------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 852353b7c1f01..643a68d1e53de 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -870,7 +870,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene match res { Res::Def(kind @ DefKind::Mod, def_id) - | Res::Def(kind @ DefKind::Enum, def_id) => { + | Res::Def(kind @ DefKind::Enum, def_id) + | Res::Def(kind @ DefKind::Trait, def_id) => { let module = self.r.new_module(parent, ModuleKind::Def(kind, def_id, ident.name), def_id, @@ -883,6 +884,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { | Res::Def(DefKind::ForeignTy, _) | Res::Def(DefKind::OpaqueTy, _) | Res::Def(DefKind::TraitAlias, _) + | Res::Def(DefKind::AssocTy, _) + | Res::Def(DefKind::AssocOpaqueTy, _) | Res::PrimTy(..) | Res::ToolMod => { self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); @@ -890,6 +893,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { Res::Def(DefKind::Fn, _) | Res::Def(DefKind::Static, _) | Res::Def(DefKind::Const, _) + | Res::Def(DefKind::AssocConst, _) | Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => { self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); } @@ -902,28 +906,11 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.struct_constructors.insert(struct_def_id, (res, vis)); } } - Res::Def(DefKind::Trait, def_id) => { - let module_kind = ModuleKind::Def(DefKind::Trait, def_id, ident.name); - let module = self.r.new_module(parent, - module_kind, - parent.normal_ancestor_id, - expansion, - span); - self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + Res::Def(DefKind::Method, def_id) => { + self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - module.populate_on_access.set(false); - for child in self.r.cstore.item_children_untracked(def_id, self.r.session) { - let res = child.res.map_id(|_| panic!("unexpected id")); - let ns = if let Res::Def(DefKind::AssocTy, _) = res { - TypeNS - } else { ValueNS }; - self.r.define(module, child.ident, ns, - (res, ty::Visibility::Public, DUMMY_SP, expansion)); - - if self.r.cstore.associated_item_cloned_untracked(child.res.def_id()) - .method_has_self_argument { - self.r.has_self.insert(res.def_id()); - } + if self.r.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument { + self.r.has_self.insert(def_id); } } Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { From ab4cc2db56c0985ff5a0eb722898d8f45186d5aa Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 16 Aug 2019 01:18:14 +0300 Subject: [PATCH 3/4] resolve: Move some code around --- src/librustc_resolve/build_reduced_graph.rs | 54 ++++++++++----------- src/librustc_resolve/diagnostics.rs | 32 ++++++------ src/librustc_resolve/late/diagnostics.rs | 9 ++-- src/librustc_resolve/lib.rs | 20 ++++++++ src/librustc_resolve/resolve_imports.rs | 32 +++--------- 5 files changed, 73 insertions(+), 74 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 643a68d1e53de..9f4d35525a86a 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -868,6 +868,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // This is only a guess, two equivalent idents may incorrectly get different gensyms here. let ident = ident.gensym_if_underscore(); let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene + // Record primary definitions. match res { Res::Def(kind @ DefKind::Mod, def_id) | Res::Def(kind @ DefKind::Enum, def_id) @@ -879,7 +880,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { span); self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); } - Res::Def(DefKind::Variant, _) + Res::Def(DefKind::Struct, _) + | Res::Def(DefKind::Union, _) + | Res::Def(DefKind::Variant, _) | Res::Def(DefKind::TyAlias, _) | Res::Def(DefKind::ForeignTy, _) | Res::Def(DefKind::OpaqueTy, _) @@ -887,43 +890,40 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { | Res::Def(DefKind::AssocTy, _) | Res::Def(DefKind::AssocOpaqueTy, _) | Res::PrimTy(..) - | Res::ToolMod => { - self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); - } + | Res::ToolMod => + self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)), Res::Def(DefKind::Fn, _) + | Res::Def(DefKind::Method, _) | Res::Def(DefKind::Static, _) | Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) - | Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => { - self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - } - Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => { - self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - - if let Some(struct_def_id) = - self.r.cstore.def_key(def_id).parent - .map(|index| DefId { krate: def_id.krate, index: index }) { - self.r.struct_constructors.insert(struct_def_id, (res, vis)); - } + | Res::Def(DefKind::Ctor(..), _) => + self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)), + Res::Def(DefKind::Macro(..), _) + | Res::NonMacroAttr(..) => + self.r.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion)), + Res::Def(DefKind::TyParam, _) | Res::Def(DefKind::ConstParam, _) + | Res::Local(..) | Res::SelfTy(..) | Res::SelfCtor(..) | Res::Err => + bug!("unexpected resolution: {:?}", res) + } + // Record some extra data for better diagnostics. + match res { + Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { + let field_names = self.r.cstore.struct_field_names_untracked(def_id); + self.insert_field_names(def_id, field_names); } Res::Def(DefKind::Method, def_id) => { - self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - if self.r.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument { self.r.has_self.insert(def_id); } } - Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { - self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); - - // Record field names for error reporting. - let field_names = self.r.cstore.struct_field_names_untracked(def_id); - self.insert_field_names(def_id, field_names); - } - Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => { - self.r.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion)); + Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => { + let parent = self.r.cstore.def_key(def_id).parent; + if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) { + self.r.struct_constructors.insert(struct_def_id, (res, vis)); + } } - _ => bug!("unexpected resolution: {:?}", res) + _ => {} } } diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index 80d1322ff3176..afdcec19d8ef6 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -73,23 +73,23 @@ crate fn add_typo_suggestion( false } -crate fn add_module_candidates<'a>( - resolver: &mut Resolver<'a>, - module: Module<'a>, - names: &mut Vec, - filter_fn: &impl Fn(Res) -> bool, -) { - for (&(ident, _), resolution) in resolver.resolutions(module).borrow().iter() { - if let Some(binding) = resolution.borrow().binding { - let res = binding.res(); - if filter_fn(res) { - names.push(TypoSuggestion::from_res(ident.name, res)); +impl<'a> Resolver<'a> { + crate fn add_module_candidates( + &mut self, + module: Module<'a>, + names: &mut Vec, + filter_fn: &impl Fn(Res) -> bool, + ) { + for (&(ident, _), resolution) in self.resolutions(module).borrow().iter() { + if let Some(binding) = resolution.borrow().binding { + let res = binding.res(); + if filter_fn(res) { + names.push(TypoSuggestion::from_res(ident.name, res)); + } } } } -} -impl<'a> Resolver<'a> { /// Combines an error with provided span and emits it. /// /// This takes the error provided, combines it with the span and any additional spans inside the @@ -405,10 +405,10 @@ impl<'a> Resolver<'a> { Scope::CrateRoot => { let root_ident = Ident::new(kw::PathRoot, ident.span); let root_module = this.resolve_crate_root(root_ident); - add_module_candidates(this, root_module, &mut suggestions, filter_fn); + this.add_module_candidates(root_module, &mut suggestions, filter_fn); } Scope::Module(module) => { - add_module_candidates(this, module, &mut suggestions, filter_fn); + this.add_module_candidates(module, &mut suggestions, filter_fn); } Scope::MacroUsePrelude => { suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| { @@ -456,7 +456,7 @@ impl<'a> Resolver<'a> { Scope::StdLibPrelude => { if let Some(prelude) = this.prelude { let mut tmp_suggestions = Vec::new(); - add_module_candidates(this, prelude, &mut tmp_suggestions, filter_fn); + this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn); suggestions.extend(tmp_suggestions.into_iter().filter(|s| { use_prelude || this.is_builtin_macro(s.res) })); diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 960bde3180915..a822fa049ca1c 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -1,8 +1,7 @@ use crate::{CrateLint, Module, ModuleKind, ModuleOrUniformRoot}; use crate::{PathResult, PathSource, Segment}; use crate::path_names_to_string; -use crate::diagnostics::{add_typo_suggestion, add_module_candidates}; -use crate::diagnostics::{ImportSuggestion, TypoSuggestion}; +use crate::diagnostics::{add_typo_suggestion, ImportSuggestion, TypoSuggestion}; use crate::late::{LateResolutionVisitor, RibKind}; use errors::{Applicability, DiagnosticBuilder, DiagnosticId}; @@ -548,7 +547,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // Items in scope if let RibKind::ModuleRibKind(module) = rib.kind { // Items from this module - add_module_candidates(self.r, module, &mut names, &filter_fn); + self.r.add_module_candidates(module, &mut names, &filter_fn); if let ModuleKind::Block(..) = module.kind { // We can see through blocks @@ -577,7 +576,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { })); if let Some(prelude) = self.r.prelude { - add_module_candidates(self.r, prelude, &mut names, &filter_fn); + self.r.add_module_candidates(prelude, &mut names, &filter_fn); } } break; @@ -599,7 +598,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { mod_path, Some(TypeNS), false, span, CrateLint::No ) { if let ModuleOrUniformRoot::Module(module) = module { - add_module_candidates(self.r, module, &mut names, &filter_fn); + self.r.add_module_candidates(module, &mut names, &filter_fn); } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index bf51ba212ff07..1c1448c9aa123 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -59,6 +59,7 @@ use std::collections::BTreeSet; use rustc_data_structures::ptr_key::PtrKey; use rustc_data_structures::sync::Lrc; +use build_reduced_graph::BuildReducedGraphVisitor; use diagnostics::{Suggestion, ImportSuggestion}; use diagnostics::{find_span_of_binding_until_next_binding, extend_span_to_previous_binding}; use late::{PathSource, Rib, RibKind::*}; @@ -1257,6 +1258,25 @@ impl<'a> Resolver<'a> { self.arenas.alloc_module(module) } + fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> { + if module.populate_on_access.get() { + module.populate_on_access.set(false); + let def_id = module.def_id().expect("unpopulated module without a def-id"); + for child in self.cstore.item_children_untracked(def_id, self.session) { + let child = child.map_id(|_| panic!("unexpected id")); + BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } + .build_reduced_graph_for_external_crate_res(module, child); + } + } + &module.lazy_resolutions + } + + fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace) + -> &'a RefCell> { + *self.resolutions(module).borrow_mut().entry((ident.modern(), ns)) + .or_insert_with(|| self.arenas.alloc_name_resolution()) + } + fn record_use(&mut self, ident: Ident, ns: Namespace, used_binding: &'a NameBinding<'a>, is_lexical_scope: bool) { if let Some((b2, kind)) = used_binding.ambiguity { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index d68dbb78e9d00..b49f186870669 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -7,10 +7,8 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, PerNS, ScopeSet, ParentScope use crate::Determinacy::{self, *}; use crate::Namespace::{self, TypeNS, MacroNS}; use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError}; -use crate::{Resolutions, Resolver, ResolutionError, Segment}; +use crate::{Resolver, ResolutionError, Segment, ModuleKind}; use crate::{names_to_string, module_to_string}; -use crate::ModuleKind; -use crate::build_reduced_graph::BuildReducedGraphVisitor; use crate::diagnostics::Suggestion; use errors::Applicability; @@ -38,7 +36,7 @@ use syntax_pos::{MultiSpan, Span}; use log::*; -use std::cell::{Cell, RefCell}; +use std::cell::Cell; use std::{mem, ptr}; type Res = def::Res; @@ -162,25 +160,6 @@ impl<'a> NameResolution<'a> { } impl<'a> Resolver<'a> { - crate fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> { - if module.populate_on_access.get() { - module.populate_on_access.set(false); - let def_id = module.def_id().expect("unpopulated module without a def-id"); - for child in self.cstore.item_children_untracked(def_id, self.session) { - let child = child.map_id(|_| panic!("unexpected id")); - BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } - .build_reduced_graph_for_external_crate_res(module, child); - } - } - &module.lazy_resolutions - } - - crate fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace) - -> &'a RefCell> { - *self.resolutions(module).borrow_mut().entry((ident.modern(), ns)) - .or_insert_with(|| self.arenas.alloc_name_resolution()) - } - crate fn resolve_ident_in_module_unadjusted( &mut self, module: ModuleOrUniformRoot<'a>, @@ -1039,7 +1018,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> { return if all_ns_failed { let resolutions = match module { - ModuleOrUniformRoot::Module(module) => Some(self.r.resolutions(module).borrow()), + ModuleOrUniformRoot::Module(module) => + Some(self.r.resolutions(module).borrow()), _ => None, }; let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); @@ -1292,8 +1272,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> { // Ensure that `resolutions` isn't borrowed during `try_define`, // since it might get updated via a glob cycle. - let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(&ident, resolution)| { - resolution.borrow().binding().map(|binding| (ident, binding)) + let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(ident, resolution)| { + resolution.borrow().binding().map(|binding| (*ident, binding)) }).collect::>(); for ((mut ident, ns), binding) in bindings { let scope = match ident.span.reverse_glob_adjust(module.expansion, directive.span) { From 5e47cd4a1784bd9d86a3f5a413d606e938679044 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 16 Aug 2019 21:19:43 +0300 Subject: [PATCH 4/4] Fix rebase Move some code into `build_reduced_graph.rs` to keep `BuildReducedGraphVisitor` it private Also move the def collector call to `build_reduced_graph.rs`, it belongs there. --- src/librustc_resolve/build_reduced_graph.rs | 11 +++++++++++ src/librustc_resolve/lib.rs | 8 +------- src/librustc_resolve/macros.rs | 2 -- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 9f4d35525a86a..1510d74babb6d 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -16,6 +16,7 @@ use crate::{ResolutionError, Determinacy, PathResult, CrateLint}; use rustc::bug; use rustc::hir::def::{self, *}; use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId}; +use rustc::hir::map::DefCollector; use rustc::ty; use rustc::middle::cstore::CrateStore; use rustc_metadata::cstore::LoadedMacro; @@ -162,10 +163,20 @@ impl<'a> Resolver<'a> { crate fn build_reduced_graph( &mut self, fragment: &AstFragment, parent_scope: ParentScope<'a> ) -> LegacyScope<'a> { + fragment.visit_with(&mut DefCollector::new(&mut self.definitions, parent_scope.expansion)); let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope }; fragment.visit_with(&mut visitor); visitor.parent_scope.legacy } + + crate fn build_reduced_graph_external(&mut self, module: Module<'a>) { + let def_id = module.def_id().expect("unpopulated module without a def-id"); + for child in self.cstore.item_children_untracked(def_id, self.session) { + let child = child.map_id(|_| panic!("unexpected id")); + BuildReducedGraphVisitor { r: self, parent_scope: ParentScope::module(module) } + .build_reduced_graph_for_external_crate_res(child); + } + } } struct BuildReducedGraphVisitor<'a, 'b> { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1c1448c9aa123..12c4f5bfe8e61 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -59,7 +59,6 @@ use std::collections::BTreeSet; use rustc_data_structures::ptr_key::PtrKey; use rustc_data_structures::sync::Lrc; -use build_reduced_graph::BuildReducedGraphVisitor; use diagnostics::{Suggestion, ImportSuggestion}; use diagnostics::{find_span_of_binding_until_next_binding, extend_span_to_previous_binding}; use late::{PathSource, Rib, RibKind::*}; @@ -1261,12 +1260,7 @@ impl<'a> Resolver<'a> { fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> { if module.populate_on_access.get() { module.populate_on_access.set(false); - let def_id = module.def_id().expect("unpopulated module without a def-id"); - for child in self.cstore.item_children_untracked(def_id, self.session) { - let child = child.map_id(|_| panic!("unexpected id")); - BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } - .build_reduced_graph_for_external_crate_res(module, child); - } + self.build_reduced_graph_external(module); } &module.lazy_resolutions } diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 079145e816cd5..6f49377c187aa 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -8,7 +8,6 @@ use crate::{ModuleOrUniformRoot, KNOWN_TOOLS}; use crate::Namespace::*; use crate::resolve_imports::ImportResolver; use rustc::hir::def::{self, DefKind, NonMacroAttrKind}; -use rustc::hir::map::DefCollector; use rustc::middle::stability; use rustc::{ty, lint, span_bug}; use syntax::ast::{self, Ident}; @@ -131,7 +130,6 @@ impl<'a> base::Resolver for Resolver<'a> { parent_scope.module.unresolved_invocations.borrow_mut().extend(derives); // Integrate the new AST fragment into all the definition and module structures. - fragment.visit_with(&mut DefCollector::new(&mut self.definitions, expansion)); let output_legacy_scope = self.build_reduced_graph(fragment, parent_scope); self.output_legacy_scopes.insert(expansion, output_legacy_scope); }