From 0ef7c288f6c47e99d336d232dfebde2ad6e3a845 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 29 Jul 2019 21:19:50 +0300 Subject: [PATCH 1/3] 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 | 23 +++-------- src/librustc_resolve/diagnostics.rs | 45 ++++++++++----------- src/librustc_resolve/lib.rs | 38 +++++++++-------- src/librustc_resolve/resolve_imports.rs | 32 +++++++++------ 4 files changed, 67 insertions(+), 71 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 9d01f33002940..7b66c4c006d3e 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -365,7 +365,6 @@ impl<'a> Resolver<'a> { self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX }) }; - self.populate_module_if_necessary(module); if let Some(name) = self.session.parse_sess.injected_crate_name.try_get() { if name.as_str() == ident.name.as_str() { self.injected_crate = Some(module); @@ -632,7 +631,7 @@ impl<'a> Resolver<'a> { } /// Builds the reduced graph for a single item in an external crate. - fn build_reduced_graph_for_external_crate_res( + crate fn build_reduced_graph_for_external_crate_res( &mut self, parent: Module<'a>, child: Export, @@ -686,6 +685,7 @@ impl<'a> Resolver<'a> { span); self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + module.populate_on_access.set(false); for child in self.cstore.item_children_untracked(def_id, self.session) { let res = child.res.map_id(|_| panic!("unexpected id")); let ns = if let Res::Def(DefKind::AssocTy, _) = res { @@ -699,7 +699,6 @@ impl<'a> Resolver<'a> { self.has_self.insert(res.def_id()); } } - module.populated.set(true); } Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); @@ -780,18 +779,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. - pub 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")); - self.build_reduced_graph_for_external_crate_res(module, child); - } - module.populated.set(true) - } - fn legacy_import_macro(&mut self, name: Name, binding: &'a NameBinding<'a>, @@ -863,9 +850,9 @@ impl<'a> Resolver<'a> { if let Some(span) = import_all { let directive = macro_use_directive(span); self.potentially_unused_imports.push(directive); - module.for_each_child(|ident, ns, binding| if ns == MacroNS { - let imported_binding = self.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.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 aeb6f23da5aa6..177c51cdb1ebd 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -65,10 +65,13 @@ fn add_typo_suggestion( false } -fn add_module_candidates( - module: Module<'_>, names: &mut Vec, filter_fn: &impl Fn(Res) -> bool +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) { @@ -593,10 +596,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)| { @@ -644,7 +647,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.opt_def_id()) })); @@ -694,7 +697,9 @@ impl<'a> Resolver<'a> { if path.len() == 1 { // Search in lexical scope. // Walk backwards up the ribs in scope and collect candidates. - for rib in self.ribs[ns].iter().rev() { + // Ribs have to be cloned to avoid borrowing the resolver. + let ribs = self.ribs[ns].clone(); + for rib in ribs.iter().rev() { // Locals and type parameters for (ident, &res) in &rib.bindings { if filter_fn(res) { @@ -704,7 +709,7 @@ impl<'a> Resolver<'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, module, &mut names, &filter_fn); if let ModuleKind::Block(..) = module.kind { // We can see through blocks @@ -732,7 +737,7 @@ impl<'a> Resolver<'a> { })); if let Some(prelude) = self.prelude { - add_module_candidates(prelude, &mut names, &filter_fn); + add_module_candidates(self, prelude, &mut names, &filter_fn); } } break; @@ -754,7 +759,7 @@ impl<'a> Resolver<'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, module, &mut names, &filter_fn); } } } @@ -792,11 +797,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 @@ -830,7 +833,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 }); @@ -890,8 +893,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)); } @@ -910,9 +911,7 @@ impl<'a> Resolver<'a> { // abort if the module is already found if result.is_some() { break; } - self.populate_module_if_necessary(in_module); - - in_module.for_each_child_stable(|ident, _, name_binding| { + in_module.for_each_child_stable(self, |_, 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 @@ -943,10 +942,8 @@ impl<'a> Resolver<'a> { fn collect_enum_variants(&mut self, def_id: DefId) -> Option> { self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| { - self.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, |_, 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)); @@ -1147,7 +1144,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, @@ -1168,7 +1165,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { return None; } - let resolutions = crate_module.resolutions.borrow(); + let resolutions = self.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/lib.rs b/src/librustc_resolve/lib.rs index e7feccf506957..a70f7b11ef527 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1035,7 +1035,7 @@ enum RibKind<'a> { /// /// The resolution keeps a separate stack of ribs as it traverses the AST for each namespace. When /// resolving, the name is looked up from inside out. -#[derive(Debug)] +#[derive(Clone, Debug)] struct Rib<'a, R = Res> { bindings: FxHashMap, kind: RibKind<'a>, @@ -1156,6 +1156,8 @@ impl ModuleKind { } } +type Resolutions<'a> = RefCell>>>; + /// One node in the tree of modules. pub struct ModuleData<'a> { parent: Option>, @@ -1164,7 +1166,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, single_segment_macro_resolutions: RefCell, Option<&'a NameBinding<'a>>)>>, multi_segment_macro_resolutions: RefCell, Span, MacroKind, ParentScope<'a>, @@ -1182,11 +1188,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, @@ -1205,7 +1206,8 @@ 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()), single_segment_macro_resolutions: RefCell::new(Vec::new()), multi_segment_macro_resolutions: RefCell::new(Vec::new()), builtin_attrs: RefCell::new(Vec::new()), @@ -1214,24 +1216,27 @@ impl<'a> ModuleData<'a> { 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 Resolver<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + for (&(ident, ns), name_resolution) in resolver.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 Resolver<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + let resolutions = resolver.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)); } } @@ -4526,7 +4531,7 @@ impl<'a> Resolver<'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, |_, name, ns, binding| { if ns != TypeNS { return } match binding.res() { Res::Def(DefKind::Trait, _) | @@ -5080,7 +5085,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 59438883d60bc..70bd347dbbca9 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -5,7 +5,7 @@ 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, Segment}; +use crate::{Resolutions, Resolver, Segment}; use crate::{names_to_string, module_to_string}; use crate::{resolve_error, ResolutionError}; use crate::ModuleKind; @@ -156,9 +156,21 @@ impl<'a> NameResolution<'a> { } impl<'a> Resolver<'a> { - 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")); + 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> { - *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()) } @@ -239,8 +251,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. @@ -1079,7 +1089,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.resolutions(module).borrow()), _ => None, }; let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); @@ -1317,8 +1327,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } }; - self.populate_module_if_necessary(module); - if module.is_trait() { self.session.span_err(directive.span, "items in traits are not importable."); return; @@ -1334,7 +1342,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.resolutions(module).borrow().iter().filter_map(|(&ident, resolution)| { resolution.borrow().binding().map(|binding| (ident, binding)) }).collect::>(); for ((mut ident, ns), binding) in bindings { @@ -1361,7 +1369,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.resolutions(module).borrow().iter() { let resolution = &mut *resolution.borrow_mut(); let binding = match resolution.binding { Some(binding) => binding, @@ -1420,8 +1428,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.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 3ee614bbcd0cb0d6e5fcfa74f572679c0d04ed2f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 30 Jul 2019 23:07:18 +0300 Subject: [PATCH 2/3] 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 7b66c4c006d3e..5440ae85a59b8 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -644,7 +644,8 @@ impl<'a> Resolver<'a> { 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.new_module(parent, ModuleKind::Def(kind, def_id, ident.name), def_id, @@ -657,6 +658,8 @@ impl<'a> Resolver<'a> { | Res::Def(DefKind::ForeignTy, _) | Res::Def(DefKind::OpaqueTy, _) | Res::Def(DefKind::TraitAlias, _) + | Res::Def(DefKind::AssocTy, _) + | Res::Def(DefKind::AssocExistential, _) | Res::PrimTy(..) | Res::ToolMod => { self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); @@ -664,6 +667,7 @@ impl<'a> Resolver<'a> { Res::Def(DefKind::Fn, _) | Res::Def(DefKind::Static, _) | Res::Def(DefKind::Const, _) + | Res::Def(DefKind::AssocConst, _) | Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => { self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); } @@ -676,28 +680,11 @@ impl<'a> Resolver<'a> { self.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.new_module(parent, - module_kind, - parent.normal_ancestor_id, - expansion, - span); - self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + Res::Def(DefKind::Method, def_id) => { + self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - module.populate_on_access.set(false); - for child in self.cstore.item_children_untracked(def_id, self.session) { - let res = child.res.map_id(|_| panic!("unexpected id")); - let ns = if let Res::Def(DefKind::AssocTy, _) = res { - TypeNS - } else { ValueNS }; - self.define(module, child.ident, ns, - (res, ty::Visibility::Public, DUMMY_SP, expansion)); - - if self.cstore.associated_item_cloned_untracked(child.res.def_id()) - .method_has_self_argument { - self.has_self.insert(res.def_id()); - } + if self.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument { + self.has_self.insert(def_id); } } Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { From 2cb9207c93a30fa6417a2ae61888d24cfed26a6b Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 30 Jul 2019 23:30:54 +0300 Subject: [PATCH 3/3] resolve: Move some code around Avoid using dummy spans for some external items with available spans --- src/librustc_resolve/build_reduced_graph.rs | 60 +++++++++---------- src/librustc_resolve/diagnostics.rs | 44 +++++++------- src/librustc_resolve/lib.rs | 60 ++++++++++++------- src/librustc_resolve/resolve_imports.rs | 22 +------ ...e-extern-crate-restricted-shadowing.stderr | 8 ++- src/test/ui/imports/glob-shadowing.stderr | 16 +++-- .../local-modularized-tricky-fail-1.stderr | 8 ++- src/test/ui/issues/issue-27033.stderr | 5 ++ 8 files changed, 121 insertions(+), 102 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 5440ae85a59b8..ec74a00f98b60 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -642,6 +642,7 @@ impl<'a> Resolver<'a> { // 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) @@ -651,53 +652,52 @@ impl<'a> Resolver<'a> { def_id, expansion, span); - self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + self.define(parent, ident, TypeNS, (module, vis, span, 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, _) | Res::Def(DefKind::TraitAlias, _) | Res::Def(DefKind::AssocTy, _) - | Res::Def(DefKind::AssocExistential, _) + | Res::Def(DefKind::AssocOpaqueTy, _) | Res::PrimTy(..) - | Res::ToolMod => { - self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); - } + | Res::ToolMod => + self.define(parent, ident, TypeNS, (res, vis, span, 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.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - } - Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => { - self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - - if let Some(struct_def_id) = - self.cstore.def_key(def_id).parent - .map(|index| DefId { krate: def_id.krate, index: index }) { - self.struct_constructors.insert(struct_def_id, (res, vis)); - } + | Res::Def(DefKind::Ctor(..), _) => + self.define(parent, ident, ValueNS, (res, vis, span, expansion)), + Res::Def(DefKind::Macro(..), _) + | Res::NonMacroAttr(..) => + self.define(parent, ident, MacroNS, (res, vis, span, 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.cstore.struct_field_names_untracked(def_id); + self.insert_field_names(def_id, field_names); } Res::Def(DefKind::Method, def_id) => { - self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - if self.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument { self.has_self.insert(def_id); } } - Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { - self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); - - // Record field names for error reporting. - let field_names = self.cstore.struct_field_names_untracked(def_id); - self.insert_field_names(def_id, field_names); - } - Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => { - self.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion)); + Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => { + let parent = self.cstore.def_key(def_id).parent; + if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) { + self.struct_constructors.insert(struct_def_id, (res, vis)); + } } - _ => bug!("unexpected resolution: {:?}", res) + _ => {} } } @@ -837,7 +837,7 @@ impl<'a> Resolver<'a> { if let Some(span) = import_all { let directive = macro_use_directive(span); self.potentially_unused_imports.push(directive); - module.for_each_child(self, |this, ident, ns, binding| if ns == MacroNS { + self.for_each_child(module, |this, ident, ns, binding| if ns == MacroNS { let imported_binding = this.import(binding, directive); this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing); }); diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index 177c51cdb1ebd..87d5a53f7a842 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -65,23 +65,23 @@ fn add_typo_suggestion( false } -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> { + 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> { /// Handles error reporting for `smart_resolve_path_fragment` function. /// Creates base error and amends it with one short label and possibly some longer helps/notes. pub(crate) fn smart_resolve_report_errors( @@ -596,10 +596,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)| { @@ -647,7 +647,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.opt_def_id()) })); @@ -709,7 +709,7 @@ impl<'a> Resolver<'a> { // Items in scope if let RibKind::ModuleRibKind(module) = rib.kind { // Items from this module - add_module_candidates(self, module, &mut names, &filter_fn); + self.add_module_candidates(module, &mut names, &filter_fn); if let ModuleKind::Block(..) = module.kind { // We can see through blocks @@ -737,7 +737,7 @@ impl<'a> Resolver<'a> { })); if let Some(prelude) = self.prelude { - add_module_candidates(self, prelude, &mut names, &filter_fn); + self.add_module_candidates(prelude, &mut names, &filter_fn); } } break; @@ -759,7 +759,7 @@ impl<'a> Resolver<'a> { mod_path, Some(TypeNS), false, span, CrateLint::No ) { if let ModuleOrUniformRoot::Module(module) = module { - add_module_candidates(self, module, &mut names, &filter_fn); + self.add_module_candidates(module, &mut names, &filter_fn); } } } @@ -799,7 +799,7 @@ impl<'a> Resolver<'a> { in_module_is_extern)) = worklist.pop() { // We have to visit module children in deterministic order to avoid // instabilities in reported imports (#43552). - in_module.for_each_child_stable(self, |this, ident, ns, name_binding| { + self.for_each_child_stable(in_module, |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 @@ -911,7 +911,7 @@ impl<'a> Resolver<'a> { // abort if the module is already found if result.is_some() { break; } - in_module.for_each_child_stable(self, |_, ident, _, name_binding| { + self.for_each_child_stable(in_module, |_, 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 @@ -943,7 +943,7 @@ impl<'a> Resolver<'a> { fn collect_enum_variants(&mut self, def_id: DefId) -> Option> { self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| { let mut variants = Vec::new(); - enum_module.for_each_child_stable(self, |_, ident, _, name_binding| { + self.for_each_child_stable(enum_module, |_, 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 a70f7b11ef527..cf28f0ebf43a2 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1221,25 +1221,6 @@ impl<'a> ModuleData<'a> { } } - fn for_each_child(&'a self, resolver: &mut Resolver<'a>, mut f: F) - where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) - { - for (&(ident, ns), name_resolution) in resolver.resolutions(self).borrow().iter() { - name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding)); - } - } - - fn for_each_child_stable(&'a self, resolver: &mut Resolver<'a>, mut f: F) - where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) - { - let resolutions = resolver.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(resolver, ident, ns, binding)); - } - } - fn res(&self) -> Option { match self.kind { ModuleKind::Def(kind, def_id, _) => Some(Res::Def(kind, def_id)), @@ -1904,9 +1885,7 @@ impl<'a> Resolver<'a> { seg.id = self.session.next_node_id(); seg } -} -impl<'a> Resolver<'a> { pub fn new(session: &'a Session, cstore: &'a CStore, krate: &Crate, @@ -2116,6 +2095,43 @@ 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")); + 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 for_each_child(&mut self, module: Module<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + for (&(ident, ns), name_resolution) in self.resolutions(module).borrow().iter() { + name_resolution.borrow().binding.map(|binding| f(self, ident, ns, binding)); + } + } + + fn for_each_child_stable(&mut self, module: Module<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + let resolutions = self.resolutions(module).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(self, ident, ns, binding)); + } + } + 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 { @@ -4531,7 +4547,7 @@ impl<'a> Resolver<'a> { let mut traits = module.traits.borrow_mut(); if traits.is_none() { let mut collected_traits = Vec::new(); - module.for_each_child(self, |_, name, ns, binding| { + self.for_each_child(module, |_, name, ns, binding| { if ns != TypeNS { return } match binding.res() { Res::Def(DefKind::Trait, _) | diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 70bd347dbbca9..93271eb17d80a 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -5,7 +5,7 @@ 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, Segment}; +use crate::{Resolver, Segment}; use crate::{names_to_string, module_to_string}; use crate::{resolve_error, ResolutionError}; use crate::ModuleKind; @@ -36,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; @@ -156,24 +156,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")); - 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()) - } - crate fn resolve_ident_in_module_unadjusted( &mut self, module: ModuleOrUniformRoot<'a>, diff --git a/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr b/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr index 24b1b582d1ea3..1b51295cc8727 100644 --- a/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr +++ b/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr @@ -13,8 +13,7 @@ error[E0659]: `Vec` is ambiguous (macro-expanded name vs less macro-expanded nam LL | Vec::panic!(); | ^^^ ambiguous name | - = note: `Vec` could refer to a struct from prelude -note: `Vec` could also refer to the extern crate imported here +note: `Vec` could refer to the extern crate imported here --> $DIR/extern-prelude-extern-crate-restricted-shadowing.rs:5:9 | LL | extern crate std as Vec; @@ -22,6 +21,11 @@ LL | extern crate std as Vec; ... LL | define_vec!(); | -------------- in this macro invocation +note: `Vec` could also refer to the struct defined here + --> $SRC_DIR/libstd/prelude/v1.rs:LL:COL + | +LL | pub use crate::vec::Vec; + | ^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/imports/glob-shadowing.stderr b/src/test/ui/imports/glob-shadowing.stderr index 4ef446f93c860..60f3c23067aee 100644 --- a/src/test/ui/imports/glob-shadowing.stderr +++ b/src/test/ui/imports/glob-shadowing.stderr @@ -4,14 +4,18 @@ error[E0659]: `env` is ambiguous (glob import vs any other name from outer scope LL | let x = env!("PATH"); | ^^^ ambiguous name | - = note: `env` could refer to a macro from prelude -note: `env` could also refer to the macro imported here +note: `env` could refer to the macro imported here --> $DIR/glob-shadowing.rs:9:9 | LL | use m::*; | ^^^^ = help: consider adding an explicit import of `env` to disambiguate = help: or use `self::env` to refer to this macro unambiguously +note: `env` could also refer to the macro defined here + --> $SRC_DIR/libstd/prelude/v1.rs:LL:COL + | +LL | env, + | ^^^ error[E0659]: `env` is ambiguous (glob import vs any other name from outer scope during import/macro resolution) --> $DIR/glob-shadowing.rs:19:21 @@ -19,13 +23,17 @@ error[E0659]: `env` is ambiguous (glob import vs any other name from outer scope LL | let x = env!("PATH"); | ^^^ ambiguous name | - = note: `env` could refer to a macro from prelude -note: `env` could also refer to the macro imported here +note: `env` could refer to the macro imported here --> $DIR/glob-shadowing.rs:17:13 | LL | use m::*; | ^^^^ = help: consider adding an explicit import of `env` to disambiguate +note: `env` could also refer to the macro defined here + --> $SRC_DIR/libstd/prelude/v1.rs:LL:COL + | +LL | env, + | ^^^ error[E0659]: `fenv` is ambiguous (glob import vs any other name from outer scope during import/macro resolution) --> $DIR/glob-shadowing.rs:29:21 diff --git a/src/test/ui/imports/local-modularized-tricky-fail-1.stderr b/src/test/ui/imports/local-modularized-tricky-fail-1.stderr index 5afdd8889ae7d..283cbe740832d 100644 --- a/src/test/ui/imports/local-modularized-tricky-fail-1.stderr +++ b/src/test/ui/imports/local-modularized-tricky-fail-1.stderr @@ -27,8 +27,7 @@ error[E0659]: `include` is ambiguous (macro-expanded name vs less macro-expanded LL | include!(); | ^^^^^^^ ambiguous name | - = note: `include` could refer to a macro from prelude -note: `include` could also refer to the macro defined here +note: `include` could refer to the macro defined here --> $DIR/local-modularized-tricky-fail-1.rs:17:5 | LL | / macro_rules! include { @@ -39,6 +38,11 @@ LL | | } LL | define_include!(); | ------------------ in this macro invocation = help: use `crate::include` to refer to this macro unambiguously +note: `include` could also refer to the macro defined here + --> $SRC_DIR/libstd/prelude/v1.rs:LL:COL + | +LL | include, + | ^^^^^^^ error[E0659]: `panic` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution) --> $DIR/local-modularized-tricky-fail-1.rs:35:5 diff --git a/src/test/ui/issues/issue-27033.stderr b/src/test/ui/issues/issue-27033.stderr index ab95433228037..b11298d4ab90b 100644 --- a/src/test/ui/issues/issue-27033.stderr +++ b/src/test/ui/issues/issue-27033.stderr @@ -3,6 +3,11 @@ error[E0530]: match bindings cannot shadow unit variants | LL | None @ _ => {} | ^^^^ cannot be named the same as a unit variant + | + ::: $SRC_DIR/libstd/prelude/v1.rs:LL:COL + | +LL | pub use crate::option::Option::{self, Some, None}; + | ---- the unit variant `None` is defined here error[E0530]: match bindings cannot shadow constants --> $DIR/issue-27033.rs:7:9