Skip to content

Commit 7342213

Browse files
borsbors
committed
Auto merge of #88872 - petrochenkov:getmod2, r=cjgillot
resolve: Some module-related refactorings See the individual commits.
2 parents f06f9bb + 8f035b8 commit 7342213

File tree

8 files changed

+178
-193
lines changed

8 files changed

+178
-193
lines changed

Diff for: ‎compiler/rustc_resolve/src/build_reduced_graph.rs

+102-117
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,9 @@ use crate::def_collector::collect_definitions;
99
use crate::imports::{Import, ImportKind};
1010
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
1111
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
12-
use crate::{CrateLint, Determinacy, PathResult, ResolutionError, VisResolutionError};
13-
use crate::{
14-
ExternPreludeEntry, ModuleOrUniformRoot, ParentScope, PerNS, Resolver, ResolverArenas,
15-
};
16-
use crate::{Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, Segment, ToNameBinding};
12+
use crate::{CrateLint, Determinacy, ExternPreludeEntry, Module, ModuleKind, ModuleOrUniformRoot};
13+
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
14+
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};
1715

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

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

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

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

129-
let (name, parent) = if def_id.index == CRATE_DEF_INDEX {
130-
// This is the crate root
131-
(self.cstore().crate_name(def_id.krate), None)
132-
} else {
133-
let def_key = self.cstore().def_key(def_id);
134-
let name = def_key
135-
.disambiguated_data
136-
.data
137-
.get_opt_name()
138-
.expect("given a DefId that wasn't a module");
139-
140-
let parent = Some(self.nearest_parent_mod(def_id));
141-
(name, parent)
142-
};
136+
if !def_id.is_local() {
137+
let def_kind = self.cstore().def_kind(def_id);
138+
match def_kind {
139+
DefKind::Mod | DefKind::Enum | DefKind::Trait => {
140+
let def_key = self.cstore().def_key(def_id);
141+
let parent = def_key.parent.map(|index| {
142+
self.get_nearest_non_block_module(DefId { index, krate: def_id.krate })
143+
});
144+
let name = if def_id.index == CRATE_DEF_INDEX {
145+
self.cstore().crate_name(def_id.krate)
146+
} else {
147+
def_key.disambiguated_data.data.get_opt_name().expect("module without name")
148+
};
143149

144-
// Allocate and return a new module with the information we found
145-
let kind = ModuleKind::Def(DefKind::Mod, def_id, name);
146-
let module = self.arenas.alloc_module(ModuleData::new(
147-
parent,
148-
kind,
149-
def_id,
150-
self.cstore().module_expansion_untracked(def_id, &self.session),
151-
self.cstore().get_span_untracked(def_id, &self.session),
152-
));
153-
self.extern_module_map.insert(def_id, module);
154-
module
150+
let module = self.arenas.new_module(
151+
parent,
152+
ModuleKind::Def(def_kind, def_id, name),
153+
self.cstore().module_expansion_untracked(def_id, &self.session),
154+
self.cstore().get_span_untracked(def_id, &self.session),
155+
// FIXME: Account for `#[no_implicit_prelude]` attributes.
156+
parent.map_or(false, |module| module.no_implicit_prelude),
157+
);
158+
self.module_map.insert(def_id, module);
159+
Some(module)
160+
}
161+
_ => None,
162+
}
163+
} else {
164+
None
165+
}
155166
}
156167

157-
crate fn macro_def_scope(&mut self, expn_id: ExpnId) -> Module<'a> {
158-
let def_id = match expn_id.expn_data().macro_def_id {
159-
Some(def_id) => def_id,
160-
None => {
161-
return expn_id
162-
.as_local()
163-
.and_then(|expn_id| self.ast_transform_scopes.get(&expn_id))
164-
.unwrap_or(&self.graph_root);
165-
}
166-
};
167-
self.macro_def_scope_from_def_id(def_id)
168+
crate fn expn_def_scope(&mut self, expn_id: ExpnId) -> Module<'a> {
169+
match expn_id.expn_data().macro_def_id {
170+
Some(def_id) => self.macro_def_scope(def_id),
171+
None => expn_id
172+
.as_local()
173+
.and_then(|expn_id| self.ast_transform_scopes.get(&expn_id))
174+
.unwrap_or(&self.graph_root),
175+
}
168176
}
169177

170-
crate fn macro_def_scope_from_def_id(&mut self, def_id: DefId) -> Module<'a> {
178+
crate fn macro_def_scope(&mut self, def_id: DefId) -> Module<'a> {
171179
if let Some(id) = def_id.as_local() {
172180
self.local_macro_def_scopes[&id]
173181
} else {
174-
// This is not entirely correct - a `macro_rules!` macro may occur
175-
// inside a 'block' module:
176-
//
177-
// ```rust
178-
// const _: () = {
179-
// #[macro_export]
180-
// macro_rules! my_macro {
181-
// () => {};
182-
// }
183-
// `
184-
// We don't record this information for external crates, so
185-
// the module we compute here will be the closest 'mod' item
186-
// (not necesssarily the actual parent of the `macro_rules!`
187-
// macro). `macro_rules!` macros can't use def-site hygiene,
188-
// so this hopefully won't be a problem.
189-
//
190-
// See https://github.com/rust-lang/rust/pull/77984#issuecomment-712445508
191-
self.nearest_parent_mod(def_id)
182+
self.get_nearest_non_block_module(def_id)
192183
}
193184
}
194185

@@ -274,7 +265,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
274265
self.r.visibilities[&def_id.expect_local()]
275266
}
276267
// Otherwise, the visibility is restricted to the nearest parent `mod` item.
277-
_ => ty::Visibility::Restricted(self.parent_scope.module.nearest_parent_mod),
268+
_ => ty::Visibility::Restricted(self.parent_scope.module.nearest_parent_mod()),
278269
})
279270
}
280271
ast::VisibilityKind::Restricted { ref path, id, .. } => {
@@ -717,7 +708,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
717708
local_def_id,
718709
);
719710
self.r.extern_crate_map.insert(local_def_id, crate_id);
720-
self.r.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX })
711+
self.r.expect_module(crate_id.as_def_id())
721712
};
722713

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

770761
ItemKind::Mod(..) => {
771-
let module_kind = ModuleKind::Def(DefKind::Mod, def_id, ident.name);
772-
let module = self.r.arenas.alloc_module(ModuleData {
773-
no_implicit_prelude: parent.no_implicit_prelude || {
774-
self.r.session.contains_name(&item.attrs, sym::no_implicit_prelude)
775-
},
776-
..ModuleData::new(
777-
Some(parent),
778-
module_kind,
779-
def_id,
780-
expansion.to_expn_id(),
781-
item.span,
782-
)
783-
});
762+
let module = self.r.arenas.new_module(
763+
Some(parent),
764+
ModuleKind::Def(DefKind::Mod, def_id, ident.name),
765+
expansion.to_expn_id(),
766+
item.span,
767+
parent.no_implicit_prelude
768+
|| self.r.session.contains_name(&item.attrs, sym::no_implicit_prelude),
769+
);
784770
self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion));
785-
self.r.module_map.insert(local_def_id, module);
771+
self.r.module_map.insert(def_id, module);
786772

787773
// Descend into the module.
788774
self.parent_scope.module = module;
@@ -813,13 +799,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
813799
}
814800

815801
ItemKind::Enum(_, _) => {
816-
let module_kind = ModuleKind::Def(DefKind::Enum, def_id, ident.name);
817-
let module = self.r.new_module(
818-
parent,
819-
module_kind,
820-
parent.nearest_parent_mod,
802+
let module = self.r.arenas.new_module(
803+
Some(parent),
804+
ModuleKind::Def(DefKind::Enum, def_id, ident.name),
821805
expansion.to_expn_id(),
822806
item.span,
807+
parent.no_implicit_prelude,
823808
);
824809
self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion));
825810
self.parent_scope.module = module;
@@ -888,13 +873,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
888873

889874
ItemKind::Trait(..) => {
890875
// Add all the items within to a new module.
891-
let module_kind = ModuleKind::Def(DefKind::Trait, def_id, ident.name);
892-
let module = self.r.new_module(
893-
parent,
894-
module_kind,
895-
parent.nearest_parent_mod,
876+
let module = self.r.arenas.new_module(
877+
Some(parent),
878+
ModuleKind::Def(DefKind::Trait, def_id, ident.name),
896879
expansion.to_expn_id(),
897880
item.span,
881+
parent.no_implicit_prelude,
898882
);
899883
self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion));
900884
self.parent_scope.module = module;
@@ -932,12 +916,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
932916
let parent = self.parent_scope.module;
933917
let expansion = self.parent_scope.expansion;
934918
if self.block_needs_anonymous_module(block) {
935-
let module = self.r.new_module(
936-
parent,
919+
let module = self.r.arenas.new_module(
920+
Some(parent),
937921
ModuleKind::Block(block.id),
938-
parent.nearest_parent_mod,
939922
expansion.to_expn_id(),
940923
block.span,
924+
parent.no_implicit_prelude,
941925
);
942926
self.r.block_map.insert(block.id, module);
943927
self.parent_scope.module = module; // Descend into the block.
@@ -953,12 +937,13 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
953937
// Record primary definitions.
954938
match res {
955939
Res::Def(kind @ (DefKind::Mod | DefKind::Enum | DefKind::Trait), def_id) => {
956-
let module = self.r.new_module(
957-
parent,
940+
let module = self.r.arenas.new_module(
941+
Some(parent),
958942
ModuleKind::Def(kind, def_id, ident.name),
959-
def_id,
960943
expansion.to_expn_id(),
961944
span,
945+
// FIXME: Account for `#[no_implicit_prelude]` attributes.
946+
parent.no_implicit_prelude,
962947
);
963948
self.r.define(parent, ident, TypeNS, (module, vis, span, expansion));
964949
}

Diff for: ‎compiler/rustc_resolve/src/diagnostics.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -915,8 +915,7 @@ impl<'a> Resolver<'a> {
915915
continue;
916916
}
917917
if let Some(crate_id) = self.crate_loader.maybe_process_path_extern(ident.name) {
918-
let crate_root =
919-
self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX });
918+
let crate_root = self.expect_module(crate_id.as_def_id());
920919
suggestions.extend(self.lookup_import_candidates_from_module(
921920
lookup_ident,
922921
namespace,

Diff for: ‎compiler/rustc_resolve/src/imports.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ impl<'a> Resolver<'a> {
427427
match ident.span.glob_adjust(module.expansion, glob_import.span) {
428428
Some(Some(def)) => {
429429
tmp_parent_scope =
430-
ParentScope { module: self.macro_def_scope(def), ..*parent_scope };
430+
ParentScope { module: self.expn_def_scope(def), ..*parent_scope };
431431
adjusted_parent_scope = &tmp_parent_scope;
432432
}
433433
Some(None) => {}
@@ -585,7 +585,7 @@ impl<'a> Resolver<'a> {
585585
for import in module.glob_importers.borrow_mut().iter() {
586586
let mut ident = key.ident;
587587
let scope = match ident.span.reverse_glob_adjust(module.expansion, import.span) {
588-
Some(Some(def)) => self.macro_def_scope(def),
588+
Some(Some(def)) => self.expn_def_scope(def),
589589
Some(None) => import.parent_scope.module,
590590
None => continue,
591591
};
@@ -1364,7 +1364,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
13641364
.collect::<Vec<_>>();
13651365
for (mut key, binding) in bindings {
13661366
let scope = match key.ident.span.reverse_glob_adjust(module.expansion, import.span) {
1367-
Some(Some(def)) => self.r.macro_def_scope(def),
1367+
Some(Some(def)) => self.r.expn_def_scope(def),
13681368
Some(None) => import.parent_scope.module,
13691369
None => continue,
13701370
};

Diff for: ‎compiler/rustc_resolve/src/late.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -799,9 +799,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
799799
}
800800

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

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

19411939
drop(parent_err);
19421940

1943-
let def_id = this.parent_scope.module.nearest_parent_mod;
1941+
let def_id = this.parent_scope.module.nearest_parent_mod();
19441942

19451943
if this.should_report_errs() {
19461944
this.r.use_injections.push(UseError {

0 commit comments

Comments
 (0)