Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve: Populate external modules in more automatic and lazy way #63149

Merged
merged 4 commits into from
Aug 17, 2019
Merged

resolve: Populate external modules in more automatic and lazy way #63149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea81d8c
resolve: Populate external modules in more automatic and lazy way
petrochenkov Jul 29, 2019
fe2524c
resolve: Populate external traits lazily as well
petrochenkov Aug 15, 2019
ab4cc2d
resolve: Move some code around
petrochenkov Aug 15, 2019
5e47cd4
Fix rebase
petrochenkov Aug 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 53 additions & 66 deletions src/librustc_resolve/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -159,33 +160,34 @@ 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> {
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> {
r: &'b mut Resolver<'a>,
parent_scope: ParentScope<'a>,
}

impl<'a> AsMut<Resolver<'a>> 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;
Expand Down Expand Up @@ -603,8 +605,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);
Expand Down Expand Up @@ -879,80 +879,67 @@ 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) => {
| 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,
expansion,
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, _)
| 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));
}
| 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::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::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));

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());
}
}
module.populated.set(true);
}
| Res::Def(DefKind::AssocConst, _)
| 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) => {
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::Method, 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::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)
_ => {}
}
}

fn legacy_import_macro(&mut self,
name: Name,
name: ast::Name,
binding: &'a NameBinding<'a>,
span: Span,
allow_shadowing: bool) {
Expand Down Expand Up @@ -1021,9 +1008,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() {
Expand Down
41 changes: 20 additions & 21 deletions src/librustc_resolve/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,23 @@ crate fn add_typo_suggestion(
false
}

crate fn add_module_candidates(
module: Module<'_>, names: &mut Vec<TypoSuggestion>, filter_fn: &impl Fn(Res) -> bool
) {
for (&(ident, _), resolution) in module.resolutions.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<TypoSuggestion>,
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
Expand Down Expand Up @@ -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);
this.add_module_candidates(root_module, &mut suggestions, filter_fn);
}
Scope::Module(module) => {
add_module_candidates(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)| {
Expand Down Expand Up @@ -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);
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)
}));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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,
Expand All @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/late.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) |
Expand Down
17 changes: 6 additions & 11 deletions src/librustc_resolve/late/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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(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
Expand Down Expand Up @@ -577,7 +576,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
}));

if let Some(prelude) = self.r.prelude {
add_module_candidates(prelude, &mut names, &filter_fn);
self.r.add_module_candidates(prelude, &mut names, &filter_fn);
}
}
break;
Expand All @@ -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(module, &mut names, &filter_fn);
self.r.add_module_candidates(module, &mut names, &filter_fn);
}
}
}
Expand Down Expand Up @@ -717,9 +716,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
Expand Down Expand Up @@ -750,10 +747,8 @@ impl<'a> LateResolutionVisitor<'a, '_> {

fn collect_enum_variants(&mut self, def_id: DefId) -> Option<Vec<Path>> {
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));
Expand Down
Loading