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: Some module-related refactorings #88872

Merged
merged 6 commits into from
Sep 24, 2021
Merged

resolve: Some module-related refactorings #88872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd58eea
resolve: Do not cache nearest parent mod in `ModuleData`
petrochenkov Sep 13, 2021
6e9adcb
resolve: Cleanup module allocation
petrochenkov Sep 11, 2021
1a23858
resolve: Rename some expansion def scope methods
petrochenkov Sep 11, 2021
a802188
resolve: Use a single common map for local and foreign modules
petrochenkov Sep 11, 2021
ab834e5
resolve: Refactor obtaining `Module` from its `DefId`
petrochenkov Sep 11, 2021
8f035b8
Disable a spuriously failing test
petrochenkov Sep 24, 2021
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
219 changes: 102 additions & 117 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ use crate::def_collector::collect_definitions;
use crate::imports::{Import, ImportKind};
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
use crate::{CrateLint, Determinacy, PathResult, ResolutionError, VisResolutionError};
use crate::{
ExternPreludeEntry, ModuleOrUniformRoot, ParentScope, PerNS, Resolver, ResolverArenas,
};
use crate::{Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, Segment, ToNameBinding};
use crate::{CrateLint, Determinacy, ExternPreludeEntry, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};

use rustc_ast::visit::{self, AssocCtxt, Visitor};
use rustc_ast::{self as ast, AssocItem, AssocItemKind, MetaItemKind, StmtKind};
Expand Down Expand Up @@ -95,100 +93,93 @@ impl<'a> Resolver<'a> {
}

/// Walks up the tree of definitions starting at `def_id`,
/// stopping at the first `DefKind::Mod` encountered
fn nearest_parent_mod(&mut self, def_id: DefId) -> Module<'a> {
let def_key = self.cstore().def_key(def_id);

let mut parent_id = DefId {
krate: def_id.krate,
index: def_key.parent.expect("failed to get parent for module"),
};
// The immediate parent may not be a module
// (e.g. `const _: () = { #[path = "foo.rs"] mod foo; };`)
// Walk up the tree until we hit a module or the crate root.
while parent_id.index != CRATE_DEF_INDEX
&& self.cstore().def_kind(parent_id) != DefKind::Mod
{
let parent_def_key = self.cstore().def_key(parent_id);
parent_id.index = parent_def_key.parent.expect("failed to get parent for module");
/// stopping at the first encountered module.
/// Parent block modules for arbitrary def-ids are not recorded for the local crate,
/// and are not preserved in metadata for foreign crates, so block modules are never
/// returned by this function.
///
/// For the local crate ignoring block modules may be incorrect, so use this method with care.
///
/// For foreign crates block modules can be ignored without introducing observable differences,
/// moreover they has to be ignored right now because they are not kept in metadata.
/// Foreign parent modules are used for resolving names used by foreign macros with def-site
/// hygiene, therefore block module ignorability relies on macros with def-site hygiene and
/// block module parents being unreachable from other crates.
/// Reachable macros with block module parents exist due to `#[macro_export] macro_rules!`,
/// but they cannot use def-site hygiene, so the assumption holds
/// (<https://github.com/rust-lang/rust/pull/77984#issuecomment-712445508>).
fn get_nearest_non_block_module(&mut self, mut def_id: DefId) -> Module<'a> {
loop {
match self.get_module(def_id) {
Some(module) => return module,
None => {
def_id.index =
self.def_key(def_id).parent.expect("non-root `DefId` without parent")
}
}
}
self.get_module(parent_id)
}

pub fn get_module(&mut self, def_id: DefId) -> Module<'a> {
// If this is a local module, it will be in `module_map`, no need to recalculate it.
if let Some(def_id) = def_id.as_local() {
return self.module_map[&def_id];
}
pub fn expect_module(&mut self, def_id: DefId) -> Module<'a> {
self.get_module(def_id).expect("argument `DefId` is not a module")
}

// Cache module resolution
if let Some(&module) = self.extern_module_map.get(&def_id) {
return module;
/// If `def_id` refers to a module (in resolver's sense, i.e. a module item, crate root, enum,
/// or trait), then this function returns that module's resolver representation, otherwise it
/// returns `None`.
/// FIXME: `Module`s for local enums and traits are not currently found.
crate fn get_module(&mut self, def_id: DefId) -> Option<Module<'a>> {
if let module @ Some(..) = self.module_map.get(&def_id) {
return module.copied();
}

let (name, parent) = if def_id.index == CRATE_DEF_INDEX {
// This is the crate root
(self.cstore().crate_name(def_id.krate), None)
} else {
let def_key = self.cstore().def_key(def_id);
let name = def_key
.disambiguated_data
.data
.get_opt_name()
.expect("given a DefId that wasn't a module");

let parent = Some(self.nearest_parent_mod(def_id));
(name, parent)
};
if !def_id.is_local() {
let def_kind = self.cstore().def_kind(def_id);
match def_kind {
DefKind::Mod | DefKind::Enum | DefKind::Trait => {
let def_key = self.cstore().def_key(def_id);
let parent = def_key.parent.map(|index| {
self.get_nearest_non_block_module(DefId { index, krate: def_id.krate })
});
let name = if def_id.index == CRATE_DEF_INDEX {
self.cstore().crate_name(def_id.krate)
} else {
def_key.disambiguated_data.data.get_opt_name().expect("module without name")
};

// Allocate and return a new module with the information we found
let kind = ModuleKind::Def(DefKind::Mod, def_id, name);
let module = self.arenas.alloc_module(ModuleData::new(
parent,
kind,
def_id,
self.cstore().module_expansion_untracked(def_id, &self.session),
self.cstore().get_span_untracked(def_id, &self.session),
));
self.extern_module_map.insert(def_id, module);
module
let module = self.arenas.new_module(
parent,
ModuleKind::Def(def_kind, def_id, name),
self.cstore().module_expansion_untracked(def_id, &self.session),
self.cstore().get_span_untracked(def_id, &self.session),
// FIXME: Account for `#[no_implicit_prelude]` attributes.
parent.map_or(false, |module| module.no_implicit_prelude),
);
self.module_map.insert(def_id, module);
Some(module)
}
_ => None,
}
} else {
None
}
}

crate fn macro_def_scope(&mut self, expn_id: ExpnId) -> Module<'a> {
let def_id = match expn_id.expn_data().macro_def_id {
Some(def_id) => def_id,
None => {
return expn_id
.as_local()
.and_then(|expn_id| self.ast_transform_scopes.get(&expn_id))
.unwrap_or(&self.graph_root);
}
};
self.macro_def_scope_from_def_id(def_id)
crate fn expn_def_scope(&mut self, expn_id: ExpnId) -> Module<'a> {
match expn_id.expn_data().macro_def_id {
Some(def_id) => self.macro_def_scope(def_id),
None => expn_id
.as_local()
.and_then(|expn_id| self.ast_transform_scopes.get(&expn_id))
.unwrap_or(&self.graph_root),
}
}

crate fn macro_def_scope_from_def_id(&mut self, def_id: DefId) -> Module<'a> {
crate fn macro_def_scope(&mut self, def_id: DefId) -> Module<'a> {
if let Some(id) = def_id.as_local() {
self.local_macro_def_scopes[&id]
} else {
// This is not entirely correct - a `macro_rules!` macro may occur
// inside a 'block' module:
//
// ```rust
// const _: () = {
// #[macro_export]
// macro_rules! my_macro {
// () => {};
// }
// `
// We don't record this information for external crates, so
// the module we compute here will be the closest 'mod' item
// (not necesssarily the actual parent of the `macro_rules!`
// macro). `macro_rules!` macros can't use def-site hygiene,
// so this hopefully won't be a problem.
//
// See https://github.com/rust-lang/rust/pull/77984#issuecomment-712445508
self.nearest_parent_mod(def_id)
self.get_nearest_non_block_module(def_id)
}
}

Expand Down Expand Up @@ -274,7 +265,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
self.r.visibilities[&def_id.expect_local()]
}
// Otherwise, the visibility is restricted to the nearest parent `mod` item.
_ => ty::Visibility::Restricted(self.parent_scope.module.nearest_parent_mod),
_ => ty::Visibility::Restricted(self.parent_scope.module.nearest_parent_mod()),
})
}
ast::VisibilityKind::Restricted { ref path, id, .. } => {
Expand Down Expand Up @@ -717,7 +708,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
local_def_id,
);
self.r.extern_crate_map.insert(local_def_id, crate_id);
self.r.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX })
self.r.expect_module(crate_id.as_def_id())
};

let used = self.process_macro_use_imports(item, module);
Expand Down Expand Up @@ -768,21 +759,16 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
}

ItemKind::Mod(..) => {
let module_kind = ModuleKind::Def(DefKind::Mod, def_id, ident.name);
let module = self.r.arenas.alloc_module(ModuleData {
no_implicit_prelude: parent.no_implicit_prelude || {
self.r.session.contains_name(&item.attrs, sym::no_implicit_prelude)
},
..ModuleData::new(
Some(parent),
module_kind,
def_id,
expansion.to_expn_id(),
item.span,
)
});
let module = self.r.arenas.new_module(
Some(parent),
ModuleKind::Def(DefKind::Mod, def_id, ident.name),
expansion.to_expn_id(),
item.span,
parent.no_implicit_prelude
|| self.r.session.contains_name(&item.attrs, sym::no_implicit_prelude),
);
self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion));
self.r.module_map.insert(local_def_id, module);
self.r.module_map.insert(def_id, module);

// Descend into the module.
self.parent_scope.module = module;
Expand Down Expand Up @@ -813,13 +799,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
}

ItemKind::Enum(_, _) => {
let module_kind = ModuleKind::Def(DefKind::Enum, def_id, ident.name);
let module = self.r.new_module(
parent,
module_kind,
parent.nearest_parent_mod,
let module = self.r.arenas.new_module(
Some(parent),
ModuleKind::Def(DefKind::Enum, def_id, ident.name),
expansion.to_expn_id(),
item.span,
parent.no_implicit_prelude,
);
self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion));
self.parent_scope.module = module;
Expand Down Expand Up @@ -888,13 +873,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {

ItemKind::Trait(..) => {
// Add all the items within to a new module.
let module_kind = ModuleKind::Def(DefKind::Trait, def_id, ident.name);
let module = self.r.new_module(
parent,
module_kind,
parent.nearest_parent_mod,
let module = self.r.arenas.new_module(
Some(parent),
ModuleKind::Def(DefKind::Trait, def_id, ident.name),
expansion.to_expn_id(),
item.span,
parent.no_implicit_prelude,
);
self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion));
self.parent_scope.module = module;
Expand Down Expand Up @@ -932,12 +916,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
let parent = self.parent_scope.module;
let expansion = self.parent_scope.expansion;
if self.block_needs_anonymous_module(block) {
let module = self.r.new_module(
parent,
let module = self.r.arenas.new_module(
Some(parent),
ModuleKind::Block(block.id),
parent.nearest_parent_mod,
expansion.to_expn_id(),
block.span,
parent.no_implicit_prelude,
);
self.r.block_map.insert(block.id, module);
self.parent_scope.module = module; // Descend into the block.
Expand All @@ -953,12 +937,13 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
// Record primary definitions.
match res {
Res::Def(kind @ (DefKind::Mod | DefKind::Enum | DefKind::Trait), def_id) => {
let module = self.r.new_module(
parent,
let module = self.r.arenas.new_module(
Some(parent),
ModuleKind::Def(kind, def_id, ident.name),
def_id,
expansion.to_expn_id(),
span,
// FIXME: Account for `#[no_implicit_prelude]` attributes.
parent.no_implicit_prelude,
);
self.r.define(parent, ident, TypeNS, (module, vis, span, expansion));
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,8 +915,7 @@ impl<'a> Resolver<'a> {
continue;
}
if let Some(crate_id) = self.crate_loader.maybe_process_path_extern(ident.name) {
let crate_root =
self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX });
let crate_root = self.expect_module(crate_id.as_def_id());
suggestions.extend(self.lookup_import_candidates_from_module(
lookup_ident,
namespace,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_resolve/src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl<'a> Resolver<'a> {
match ident.span.glob_adjust(module.expansion, glob_import.span) {
Some(Some(def)) => {
tmp_parent_scope =
ParentScope { module: self.macro_def_scope(def), ..*parent_scope };
ParentScope { module: self.expn_def_scope(def), ..*parent_scope };
adjusted_parent_scope = &tmp_parent_scope;
}
Some(None) => {}
Expand Down Expand Up @@ -585,7 +585,7 @@ impl<'a> Resolver<'a> {
for import in module.glob_importers.borrow_mut().iter() {
let mut ident = key.ident;
let scope = match ident.span.reverse_glob_adjust(module.expansion, import.span) {
Some(Some(def)) => self.macro_def_scope(def),
Some(Some(def)) => self.expn_def_scope(def),
Some(None) => import.parent_scope.module,
None => continue,
};
Expand Down Expand Up @@ -1364,7 +1364,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
.collect::<Vec<_>>();
for (mut key, binding) in bindings {
let scope = match key.ident.span.reverse_glob_adjust(module.expansion, import.span) {
Some(Some(def)) => self.r.macro_def_scope(def),
Some(Some(def)) => self.r.expn_def_scope(def),
Some(None) => import.parent_scope.module,
None => continue,
};
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_resolve/src/late.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,9 +799,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
}

fn with_scope<T>(&mut self, id: NodeId, f: impl FnOnce(&mut Self) -> T) -> T {
let id = self.r.local_def_id(id);
let module = self.r.module_map.get(&id).cloned(); // clones a reference
if let Some(module) = module {
if let Some(module) = self.r.get_module(self.r.local_def_id(id).to_def_id()) {
// Move down in the graph.
let orig_module = replace(&mut self.parent_scope.module, module);
self.with_rib(ValueNS, ModuleRibKind(module), |this| {
Expand Down Expand Up @@ -1872,7 +1870,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
if this.should_report_errs() {
let (err, candidates) = this.smart_resolve_report_errors(path, span, source, res);

let def_id = this.parent_scope.module.nearest_parent_mod;
let def_id = this.parent_scope.module.nearest_parent_mod();
let instead = res.is_some();
let suggestion =
if res.is_none() { this.report_missing_type_error(path) } else { None };
Expand Down Expand Up @@ -1940,7 +1938,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {

drop(parent_err);

let def_id = this.parent_scope.module.nearest_parent_mod;
let def_id = this.parent_scope.module.nearest_parent_mod();

if this.should_report_errs() {
this.r.use_injections.push(UseError {
Expand Down
Loading